feat(core): Complete the review activity feed and add the decision note (no-changelog) (#36137)

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: kgrhartlage <kai.hartlage@n8n.io>

packages/@n8n/api-types/src/dto/index.ts

@@ -339,7 +339,7 @@ export type * from './workflow-reviews/get-workflow-review-request-detail.dto';
 export {
 	ListWorkflowReviewActivityQueryDto,
 	CreateWorkflowReviewCommentDto,
-	WORKFLOW_REVIEW_COMMENT_MAX_LENGTH,
+	WORKFLOW_REVIEW_TEXT_MAX_LENGTH,
 	type ListWorkflowReviewActivityResponse,
 } from './workflow-reviews/workflow-review-activity.dto';
 

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

@@ -5,11 +5,30 @@ describe('DecideWorkflowReviewRequestDto', () => {
 		test.each([
 			{ name: 'approved decision', request: { decision: 'approved' } },
 			{ name: 'changes_requested decision', request: { decision: 'changes_requested' } },
+			{
+				name: 'approved decision with a note',
+				request: { decision: 'approved', note: 'Ships as is' },
+			},
+			{
+				name: 'changes_requested decision with a note',
+				request: { decision: 'changes_requested', note: 'Please rename the node' },
+			},
 		])('should validate $name', ({ request }) => {
 			const result = DecideWorkflowReviewRequestDto.safeParse(request);
 			expect(result.success).toBe(true);
 			expect(result.data).toMatchObject(request);
 		});
+
+		// 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', () => {
+			const result = DecideWorkflowReviewRequestDto.safeParse({
+				decision: 'approved',
+				note: '  first line\nsecond line  ',
+			});
+
+			expect(result.data?.note).toBe('first line\nsecond line');
+		});
 	});
 
 	describe('Invalid requests', () => {
@@ -34,6 +53,21 @@ describe('DecideWorkflowReviewRequestDto', () => {
 				request: { decision: 1 },
 				expectedErrorPath: ['decision'],
 			},
+			{
+				name: 'a note that is only whitespace',
+				request: { decision: 'changes_requested', note: '   \n  ' },
+				expectedErrorPath: ['note'],
+			},
+			{
+				name: 'a note over the length limit',
+				request: { decision: 'changes_requested', note: 'x'.repeat(10_001) },
+				expectedErrorPath: ['note'],
+			},
+			{
+				name: 'a note containing a control character',
+				request: { decision: 'changes_requested', note: 'oops \x00 here' },
+				expectedErrorPath: ['note'],
+			},
 		])('should fail validation for $name', ({ request, expectedErrorPath }) => {
 			const result = DecideWorkflowReviewRequestDto.safeParse(request);
 			expect(result.success).toBe(false);

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

@@ -1,6 +1,12 @@
+import { reviewTextSchema } from './workflow-review-activity.dto';
 import { workflowReviewRequestDecisionSchema } from '../../workflow-review-request-summary';
 import { Z } from '../../zod-class';
 
 export class DecideWorkflowReviewRequestDto extends Z.class({
 	decision: workflowReviewRequestDecisionSchema.exclude(['pending']),
+	/**
+	 * Required for `changes_requested`, optional for `approved`. The requirement is enforced
+	 * service-side: `Z.class` wraps a plain `z.object` with no `superRefine` hook.
+	 */
+	note: reviewTextSchema.optional(),
 }) {}

packages/@n8n/api-types/src/dto/workflow-reviews/workflow-review-activity.dto.ts

@@ -18,17 +18,20 @@ export interface ListWorkflowReviewActivityResponse {
 	hasMore: boolean;
 }
 
-export const WORKFLOW_REVIEW_COMMENT_MAX_LENGTH = 10_000;
+export const WORKFLOW_REVIEW_TEXT_MAX_LENGTH = 10_000;
+
+/** Shared by the comment body and the decision note: same cap, same control-character rule. */
+export const reviewTextSchema = z
+	.string()
+	.trim()
+	.min(1)
+	.max(WORKFLOW_REVIEW_TEXT_MAX_LENGTH)
+	// NUL cannot be stored in a Postgres text column at all, so it would turn user input
+	// into a 500. The rest of C0 is non-printing junk with no place in a comment body, and
+	// is rejected in the same pass. \n, \r and \t are deliberately allowed through.
+	// eslint-disable-next-line no-control-regex
+	.refine((v) => !/[\x00-\x08\x0B\x0C\x0E-\x1F]/.test(v), 'Contains control characters');
 
 export class CreateWorkflowReviewCommentDto extends Z.class({
-	body: z
-		.string()
-		.trim()
-		.min(1)
-		.max(WORKFLOW_REVIEW_COMMENT_MAX_LENGTH)
-		// NUL cannot be stored in a Postgres text column at all, so it would turn user input
-		// into a 500. The rest of C0 is non-printing junk with no place in a comment body, and
-		// is rejected in the same pass. \n, \r and \t are deliberately allowed through.
-		// eslint-disable-next-line no-control-regex
-		.refine((v) => !/[\x00-\x08\x0B\x0C\x0E-\x1F]/.test(v), 'Body contains control characters'),
+	body: reviewTextSchema,
 }) {}

packages/@n8n/api-types/src/index.ts

@@ -14,7 +14,7 @@ export * from './instance-registry-types';
 export * from './redaction-enforcement';
 export * from './redaction-enforcement-floor';
 export * from './workflow-reviews-policy';
-export type * from './workflow-review-activity';
+export * from './workflow-review-activity';
 export type * from './workflow-review-eligible-reviewer';
 export * from './workflow-review-request-summary';
 export * from './workflow-publish-blocked-details';

packages/@n8n/api-types/src/workflow-review-activity.ts

@@ -1,4 +1,4 @@
-import type { IDataObject } from 'n8n-workflow';
+import { z } from 'zod';
 
 import type { Iso8601DateTimeString } from './datetime';
 import type { WorkflowReviewEligibleReviewer } from './workflow-review-eligible-reviewer';
@@ -22,6 +22,59 @@ export type WorkflowReviewActivityType =
 	 * approval writes `review.approved` instead, never both. */
 	| 'review.closed';
 
+// These are the typeVersion 1 shapes. A typeVersion 2 gets its own schema, its own
+// union member and its own mapper case; never widen one of these in place.
+const workflowReviewClosedReasonSchema = z.enum([
+	'workflow-archived',
+	'workflow-moved',
+	'workflow-deleted',
+]);
+export type WorkflowReviewClosedReason = z.infer<typeof workflowReviewClosedReasonSchema>;
+
+/**
+ * Pairs the version with its workflow: the entry is the immutable record, and the live pin it was
+ * built from is prunable, so a bare version id becomes unresolvable once pruning removes the
+ * history row.
+ */
+const workflowReviewActivityWorkflowVersionSchema = z.object({
+	workflowId: z.string(),
+	workflowVersionId: z.string(),
+});
+export const workflowReviewOpenedActivityDataSchema = z.object({
+	workflowVersions: z.array(workflowReviewActivityWorkflowVersionSchema),
+});
+export type WorkflowReviewOpenedActivityData = z.infer<
+	typeof workflowReviewOpenedActivityDataSchema
+>;
+
+export const workflowReviewDecisionActivityDataSchema = z.object({
+	workflowVersions: z.array(workflowReviewActivityWorkflowVersionSchema),
+	note: z.string().nullable(),
+});
+export type WorkflowReviewDecisionActivityData = z.infer<
+	typeof workflowReviewDecisionActivityDataSchema
+>;
+
+export const workflowReviewVersionUpdatedActivityDataSchema = z.object({
+	/**
+	 * Here rather than in the entity's `workflowId` column: that column's FK cascades, so a
+	 * workflow delete would silently take these entries out of an append-only feed.
+	 */
+	workflowId: z.string(),
+	fromWorkflowVersionId: z.string().nullable(),
+	toWorkflowVersionId: z.string().nullable(),
+});
+export type WorkflowReviewVersionUpdatedActivityData = z.infer<
+	typeof workflowReviewVersionUpdatedActivityDataSchema
+>;
+
+export const workflowReviewClosedActivityDataSchema = z.object({
+	reason: workflowReviewClosedReasonSchema,
+});
+export type WorkflowReviewClosedActivityData = z.infer<
+	typeof workflowReviewClosedActivityDataSchema
+>;
+
 type WorkflowReviewActivityBase = {
 	/** Int in the database, string on the wire. */
 	id: string;
@@ -43,6 +96,8 @@ export type WorkflowReviewActivityMessage = {
 
 /**
  * Total over every type the CHECK constraint allows, so no stored row is ever unmappable.
+ * `data: null` on a non-comment entry means the stored payload did not parse; the renderer
+ * degrades rather than dropping the entry.
  */
 export type WorkflowReviewActivityEntry =
 	| (WorkflowReviewActivityBase & {
@@ -51,6 +106,22 @@ export type WorkflowReviewActivityEntry =
 			messages: WorkflowReviewActivityMessage[];
 	  })
 	| (WorkflowReviewActivityBase & {
-			type: Exclude<WorkflowReviewActivityType, 'comment.created'>;
-			data: IDataObject | null;
+			type: 'review.opened';
+			data: WorkflowReviewOpenedActivityData | null;
+	  })
+	| (WorkflowReviewActivityBase & {
+			type: 'review.changes_requested' | 'review.approved';
+			data: WorkflowReviewDecisionActivityData | null;
+	  })
+	| (WorkflowReviewActivityBase & {
+			type: 'review.version_updated';
+			data: WorkflowReviewVersionUpdatedActivityData | null;
+	  })
+	| (WorkflowReviewActivityBase & {
+			type: 'review.closed';
+			data: WorkflowReviewClosedActivityData | null;
+	  })
+	| (WorkflowReviewActivityBase & {
+			type: 'workflow.published';
+			data: null;
 	  });

packages/@n8n/db/src/entities/workflow-review-activity.ee.ts

@@ -31,8 +31,10 @@ export class WorkflowReviewActivity extends WithCreatedAt {
 	typeVersion: number;
 
 	/**
-	 * Immutable per-type detail. Ids only: a user id stored here would survive user deletion,
-	 * unlike `createdById`, so a type needing another actor gets its own `SET NULL` column.
+	 * Immutable per-type detail: ids, plus a reviewer's note on a decision. Nothing redacts it —
+	 * unlike a comment body, which is nulled on delete, it is frozen once written. No user ids: one
+	 * stored here would survive user deletion, unlike `createdById`, so a type needing another actor
+	 * gets its own `SET NULL` column.
 	 */
 	@JsonColumn({ nullable: true })
 	data: IDataObject | null;
@@ -42,9 +44,13 @@ export class WorkflowReviewActivity extends WithCreatedAt {
 	createdById: string | null;
 
 	/**
-	 * Scopes an entry to one workflow; `null` for review-level entries such as comments. A column
-	 * rather than a key in `data` so the feed can query and filter on it directly, for instance to
-	 * leave out entries about workflows the reader may not open. Nothing writes it yet.
+	 * Scopes an entry to one workflow; `null` for review-level entries such as comments and
+	 * decisions. Nothing writes it yet, deliberately: the FK is `ON DELETE CASCADE`, so setting it
+	 * means accepting that an ordinary workflow delete removes those entries from an append-only
+	 * feed, which is why `review.version_updated` keeps its `workflowId` in `data`. Filtering the
+	 * feed on this column is the precondition for relaxing
+	 * `CreateWorkflowReviewRequestDto.workflows.length(1)`, since a multi-workflow review would
+	 * otherwise show entries about workflows the reader may not open.
 	 */
 	@Column({ type: 'varchar', length: 36, nullable: true })
 	workflowId: string | null;

packages/@n8n/db/src/repositories/index.ts

@@ -83,6 +83,7 @@ export {
 export { WorkflowPublishHistoryRepository } from './workflow-publish-history.repository';
 export {
 	WorkflowReviewRequestRepository,
+	type ClosedUnreviewableRequest,
 	type InboxCursor,
 	type InboxVisibility,
 	type WorkflowReviewRequestForWorkflowRow,

packages/@n8n/db/src/repositories/workflow-review-activity.repository.ts

@@ -1,7 +1,11 @@
-import type { WorkflowReviewActivityType } from '@n8n/api-types';
+import type {
+	WorkflowReviewClosedActivityData,
+	WorkflowReviewDecisionActivityData,
+	WorkflowReviewOpenedActivityData,
+	WorkflowReviewVersionUpdatedActivityData,
+} from '@n8n/api-types';
 import { Service } from '@n8n/di';
 import { DataSource, In, LessThan } from '@n8n/typeorm';
-import type { IDataObject } from 'n8n-workflow';
 
 import { BaseRepository } from './base-repository';
 import { WorkflowReviewActivityComment } from '../entities/workflow-review-activity-comment.ee';
@@ -13,6 +17,22 @@ export type WorkflowReviewActivityFeedEntry = {
 	messages: WorkflowReviewActivityComment[];
 };
 
+/**
+ * Write-side counterpart of `WorkflowReviewActivityEntry`, reusing its data types so the read
+ * and write shapes cannot drift. Repository input only, never serialized, hence not in
+ * `@n8n/api-types`. `changes_requested` and `approved` share a member on purpose, so a
+ * conditional decision type still typechecks at the call site.
+ */
+export type WorkflowReviewActivityPayload =
+	| { type: 'review.opened'; data: WorkflowReviewOpenedActivityData }
+	| { type: 'comment.created' | 'workflow.published'; data: null }
+	| {
+			type: 'review.changes_requested' | 'review.approved';
+			data: WorkflowReviewDecisionActivityData;
+	  }
+	| { type: 'review.version_updated'; data: WorkflowReviewVersionUpdatedActivityData }
+	| { type: 'review.closed'; data: WorkflowReviewClosedActivityData };
+
 @Service()
 export class WorkflowReviewActivityRepository extends BaseRepository<WorkflowReviewActivity> {
 	constructor(dataSource: DataSource, transactionRunner: TransactionRunner) {
@@ -20,11 +40,14 @@ export class WorkflowReviewActivityRepository extends BaseRepository<WorkflowRev
 	}
 
 	async createActivity(
-		input: {
+		input: WorkflowReviewActivityPayload & {
 			workflowReviewRequestId: string;
-			type: WorkflowReviewActivityType;
-			data: IDataObject | null;
 			createdById: string | null;
+			/**
+			 * Scopes the entry to one workflow; omit for review-level entries. No caller sets it
+			 * yet — see the entity, whose FK cascades.
+			 */
+			workflowId?: string;
 		},
 		ctx: OperationContext,
 	): Promise<WorkflowReviewActivity> {

packages/@n8n/db/src/repositories/workflow-review-request.repository.ts

@@ -1,9 +1,12 @@
+import type { WorkflowReviewClosedReason } from '@n8n/api-types';
 import { Service } from '@n8n/di';
+import type { WorkflowSharingRole } from '@n8n/permissions';
 import type { SelectQueryBuilder } from '@n8n/typeorm';
 import { DataSource } from '@n8n/typeorm';
 
 import { BaseRepository } from './base-repository';
 import { SharedWorkflow } from '../entities/shared-workflow';
+import { WorkflowEntity } from '../entities/workflow-entity';
 import { WorkflowReviewRequestAuthor } from '../entities/workflow-review-request-author.ee';
 import { WorkflowReviewRequestReviewer } from '../entities/workflow-review-request-reviewer.ee';
 import { WorkflowReviewRequestWorkflow } from '../entities/workflow-review-request-workflow.ee';
@@ -92,6 +95,44 @@ export type InboxStateCounts = {
 	closed: number;
 };
 
+/** An open request the reconciliation sweep closed, and what made its workflow unreviewable. */
+export type ClosedUnreviewableRequest = {
+	id: string;
+	reason: WorkflowReviewClosedReason;
+};
+
+/** One row per (open request, linked workflow); a request with no link left yields one empty row. */
+type OpenRequestWorkflowRow = {
+	requestId: string;
+	requestProjectId: string;
+	linkedWorkflowId: string | null;
+	/** Raw, so the driver's own boolean: `1` on sqlite and mysql, `true` on postgres. */
+	isArchived: boolean | number | null;
+	owningProjectId: string | null;
+};
+
+/** Most to least definitive: one workflow can be several of these at once. */
+const CLOSE_REASON_PRECEDENCE: WorkflowReviewClosedReason[] = [
+	'workflow-deleted',
+	'workflow-archived',
+	'workflow-moved',
+];
+
+function closeReasonFor(row: OpenRequestWorkflowRow): WorkflowReviewClosedReason | null {
+	// Nothing behind the link: either the request has no link row left, or it points at a
+	// workflow that is gone. Both mean the delete cascade got there first.
+	if (row.linkedWorkflowId === null) return 'workflow-deleted';
+
+	if (row.isArchived) return 'workflow-archived';
+
+	// A workflow with no owning project at all is a broken row, not a move — leave it alone.
+	if (row.owningProjectId !== null && row.owningProjectId !== row.requestProjectId) {
+		return 'workflow-moved';
+	}
+
+	return null;
+}
+
 @Service()
 export class WorkflowReviewRequestRepository extends BaseRepository<WorkflowReviewRequest> {
 	constructor(dataSource: DataSource, transactionRunner: TransactionRunner) {
@@ -139,44 +180,69 @@ export class WorkflowReviewRequestRepository extends BaseRepository<WorkflowRevi
 	}
 
 	/**
-	 * Closes every open request left with no linked workflow, returning the ids closed.
+	 * Closes every open request whose workflow can no longer be reviewed — deleted, archived, or
+	 * moved out of the request's project — reporting each id with the reason that closed it.
 	 *
-	 * A workflow hard delete cascades the link rows away, so an open request that has
-	 * lost its last one covers nothing and can never be acted on again. `create` writes
-	 * the request and its link row in one transaction, so a request is only ever visible
-	 * without links once the workflow behind it is gone — the two steps here cannot see
-	 * a half-written create and so need no lock.
+	 * Matches on the workflow's current state rather than on the mutation that changed it, so it
+	 * catches what the per-mutation hooks cannot: reviews a delete cascade unlinked before a hook
+	 * could find them by workflow id, mutations that skip the hooks entirely, and hooks whose
+	 * close rolled back after their mutation had already committed.
+	 *
+	 * Keys off the ids selected rather than off the state, so the caller must hold the
+	 * review-request lock.
 	 */
-	async closeOrphanedOpenRequests(ctx: OperationContext): Promise<string[]> {
+	async closeUnreviewableOpenRequests(ctx: OperationContext): Promise<ClosedUnreviewableRequest[]> {
 		const openState: WorkflowReviewRequestState = 'open';
 		const closedState: WorkflowReviewRequestState = 'closed';
+		const ownerRole: WorkflowSharingRole = 'workflow:owner';
 		const manager = this.managerFor(ctx);
 
-		const orphans = await manager
+		const rows = await manager
 			.createQueryBuilder(WorkflowReviewRequest, 'review')
-			.select('review.id', 'id')
+			.select('review.id', 'requestId')
+			.addSelect('review.projectId', 'requestProjectId')
+			.addSelect('workflow.id', 'linkedWorkflowId')
+			.addSelect('workflow.isArchived', 'isArchived')
+			.addSelect('shared.projectId', 'owningProjectId')
+			// Left joins throughout: a request with no link, or a link with no workflow, is
+			// precisely the orphan case, and dropping those rows would hide it.
+			.leftJoin(WorkflowReviewRequestWorkflow, 'link', 'link.workflowReviewRequestId = review.id')
+			.leftJoin(WorkflowEntity, 'workflow', 'workflow.id = link.workflowId')
+			.leftJoin(
+				SharedWorkflow,
+				'shared',
+				'shared.workflowId = link.workflowId AND shared.role = :ownerRole',
+				{ ownerRole },
+			)
 			.where('review.state = :openState', { openState })
-			.andWhere((qb) => {
-				const linkedWorkflowExists = qb
-					.subQuery()
-					.select('1')
-					.from(WorkflowReviewRequestWorkflow, 'requestWorkflow')
-					.where('requestWorkflow.workflowReviewRequestId = review.id')
-					.getQuery();
-				return `NOT EXISTS ${linkedWorkflowExists}`;
-			})
-			.getRawMany<{ id: string }>();
+			.getRawMany<OpenRequestWorkflowRow>();
 
-		if (orphans.length === 0) return [];
+		const reasonByRequestId = new Map<string, WorkflowReviewClosedReason>();
+		for (const row of rows) {
+			const reason = closeReasonFor(row);
+			if (reason === null) continue;
+
+			// A request linked to several workflows gets one row each, so keep the most
+			// definitive reason rather than whichever row the database returned last.
+			const current = reasonByRequestId.get(row.requestId);
+			if (
+				current !== undefined &&
+				CLOSE_REASON_PRECEDENCE.indexOf(current) <= CLOSE_REASON_PRECEDENCE.indexOf(reason)
+			) {
+				continue;
+			}
+			reasonByRequestId.set(row.requestId, reason);
+		}
+
+		if (reasonByRequestId.size === 0) return [];
 
-		const ids = orphans.map(({ id }) => id);
 		// A system close has no closing user; the decision stays as-is.
-		await manager.update(WorkflowReviewRequest, ids, {
+		await manager.update(WorkflowReviewRequest, [...reasonByRequestId.keys()], {
 			state: closedState,
 			closedById: null,
 		});
 
-		return ids;
+		return [...reasonByRequestId].map(([id, reason]) => ({ id, reason }));
 	}
 
 	async findById(id: string, ctx: OperationContext): Promise<WorkflowReviewRequest | null> {

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

@@ -352,6 +352,244 @@ describe('Commenting on a review', () => {
 	});
 });
 
+describe('Recording the review lifecycle in the feed', () => {
+	/** Two history versions so the review can be re-pinned from one to the other. */
+	async function createReviewableWorkflow() {
+		const workflow = await createWorkflow({ name: 'Reviewed workflow' }, teamProject);
+		await createWorkflowHistoryItem(workflow.id, { versionId: 'version-1' });
+		await createWorkflowHistoryItem(workflow.id, { versionId: 'version-2' });
+		return workflow;
+	}
+
+	/** Opened by `member`, so `owner` is free to decide on it without an admin override. */
+	async function openReview(workflowId: string, workflowVersionId = 'version-1') {
+		const response = await memberAgent
+			.post('/workflow-review-requests')
+			.send({
+				title: 'Please review',
+				workflows: [{ workflowId, workflowVersionId, workflowVersionName: 'Release candidate' }],
+				reviewerUserIds: [owner.id],
+			})
+			.expect(201);
+		return response.body.data.id as string;
+	}
+
+	const entryTypes = (feed: { data: FeedEntry[] }) => feed.data.map((entry) => entry.type);
+
+	test('shows who opened the review and which version they submitted', async () => {
+		const workflow = await createReviewableWorkflow();
+
+		const requestId = await openReview(workflow.id);
+
+		const feed = await getActivity(memberAgent, requestId);
+		expect(feed.data).toHaveLength(1);
+		expect(feed.data[0]).toMatchObject({
+			type: 'review.opened',
+			typeVersion: 1,
+			createdBy: expect.objectContaining({ id: member.id }),
+		});
+		expect(feed.data[0].data).toEqual({
+			workflowVersions: [{ workflowId: workflow.id, workflowVersionId: 'version-1' }],
+		});
+	});
+
+	test('records the note and the reviewed version when a reviewer requests changes', async () => {
+		const workflow = await createReviewableWorkflow();
+		const requestId = await openReview(workflow.id);
+
+		await ownerAgent
+			.post(`/workflow-review-requests/${requestId}/decision`)
+			.send({ decision: 'changes_requested', note: 'Please rename the node' })
+			.expect(200);
+
+		const feed = await getActivity(ownerAgent, requestId);
+		expect(entryTypes(feed)).toEqual(['review.opened', 'review.changes_requested']);
+		expect(feed.data[1]).toMatchObject({
+			createdBy: expect.objectContaining({ id: owner.id }),
+		});
+		expect(feed.data[1].data).toEqual({
+			workflowVersions: [{ workflowId: workflow.id, workflowVersionId: 'version-1' }],
+			note: 'Please rename the node',
+		});
+	});
+
+	test('refuses to request changes without a note, and records nothing', async () => {
+		const workflow = await createReviewableWorkflow();
+		const requestId = await openReview(workflow.id);
+
+		await ownerAgent
+			.post(`/workflow-review-requests/${requestId}/decision`)
+			.send({ decision: 'changes_requested' })
+			.expect(400);
+
+		expect(entryTypes(await getActivity(ownerAgent, requestId))).toEqual(['review.opened']);
+		expect(await requestRepository.findById(requestId, {})).toMatchObject({
+			state: 'open',
+			decision: 'pending',
+		});
+	});
+
+	test('records the note a reviewer leaves when they approve', async () => {
+		const workflow = await createReviewableWorkflow();
+		const requestId = await openReview(workflow.id);
+
+		await ownerAgent
+			.post(`/workflow-review-requests/${requestId}/decision`)
+			.send({ decision: 'approved', note: 'Ships as is' })
+			.expect(200);
+
+		const feed = await getActivity(ownerAgent, requestId);
+		expect(entryTypes(feed)).toEqual(['review.opened', 'review.approved']);
+		expect(feed.data[1].data).toEqual({
+			workflowVersions: [{ workflowId: workflow.id, workflowVersionId: 'version-1' }],
+			note: 'Ships as is',
+		});
+	});
+
+	test('records an approval left without a note as having none', async () => {
+		const workflow = await createReviewableWorkflow();
+		const requestId = await openReview(workflow.id);
+
+		await ownerAgent
+			.post(`/workflow-review-requests/${requestId}/decision`)
+			.send({ decision: 'approved' })
+			.expect(200);
+
+		const feed = await getActivity(ownerAgent, requestId);
+		// `null`, not a missing key: "no note given" and "the payload did not parse" are
+		// different things on an audit record.
+		expect(feed.data[1].data).toEqual({
+			workflowVersions: [{ workflowId: workflow.id, workflowVersionId: 'version-1' }],
+			note: null,
+		});
+	});
+
+	test('still names the workflow an approved version came from after that workflow is deleted', async () => {
+		const workflow = await createReviewableWorkflow();
+		const requestId = await openReview(workflow.id);
+
+		await ownerAgent
+			.post(`/workflow-review-requests/${requestId}/decision`)
+			.send({ decision: 'approved' })
+			.expect(200);
+
+		await workflowEntityRepository.delete(workflow.id);
+
+		// The pin cascades away with the workflow, so the entry is the only record left of which
+		// version was approved.
+		expect(await workflowRepository.findByRequestId(requestId, {})).toEqual([]);
+
+		const feed = await getActivity(ownerAgent, requestId);
+		expect(feed.data[1].data).toEqual({
+			workflowVersions: [{ workflowId: workflow.id, workflowVersionId: 'version-1' }],
+			note: null,
+		});
+	});
+
+	test('records the version a re-pinned review moved from and to, scoped to that workflow', async () => {
+		const workflow = await createReviewableWorkflow();
+		const requestId = await openReview(workflow.id);
+
+		await memberAgent
+			.post(`/workflow-review-requests/${requestId}/update-version`)
+			.send({
+				workflowId: workflow.id,
+				workflowVersionId: 'version-2',
+				workflowVersionName: 'Second attempt',
+			})
+			.expect(200);
+
+		const feed = await getActivity(memberAgent, requestId);
+		expect(entryTypes(feed)).toEqual(['review.opened', 'review.version_updated']);
+		expect(feed.data[1].data).toEqual({
+			workflowId: workflow.id,
+			fromWorkflowVersionId: 'version-1',
+			toWorkflowVersionId: 'version-2',
+		});
+
+		// The scoping column stays unwritten: its FK cascades, so a workflow delete would take
+		// the entry with it.
+		const rows = await activityRepository.findBy({ type: 'review.version_updated' });
+		expect(rows).toHaveLength(1);
+		expect(rows[0].workflowId).toBeNull();
+	});
+
+	test('keeps a version update in the feed after its workflow is deleted', async () => {
+		const workflow = await createReviewableWorkflow();
+		const requestId = await openReview(workflow.id);
+
+		await memberAgent
+			.post(`/workflow-review-requests/${requestId}/update-version`)
+			.send({
+				workflowId: workflow.id,
+				workflowVersionId: 'version-2',
+				workflowVersionName: 'Second attempt',
+			})
+			.expect(200);
+
+		// Straight from the repository, as a folder-hierarchy cascade does, so nothing but the
+		// database's own cascades decides what survives.
+		await workflowEntityRepository.delete(workflow.id);
+
+		const feed = await getActivity(memberAgent, requestId);
+		expect(entryTypes(feed)).toEqual(['review.opened', 'review.version_updated']);
+		expect(feed.data[1].data).toMatchObject({ workflowId: workflow.id });
+	});
+
+	test('records nothing when a review is re-pinned to the version it already covers', async () => {
+		const workflow = await createReviewableWorkflow();
+		const requestId = await openReview(workflow.id);
+
+		await memberAgent
+			.post(`/workflow-review-requests/${requestId}/update-version`)
+			.send({
+				workflowId: workflow.id,
+				workflowVersionId: 'version-1',
+				workflowVersionName: 'Renamed only',
+			})
+			.expect(200);
+
+		expect(entryTypes(await getActivity(memberAgent, requestId))).toEqual(['review.opened']);
+	});
+
+	test('leaves the review undecided when its feed entry cannot be written', async () => {
+		const workflow = await createReviewableWorkflow();
+		const requestId = await openReview(workflow.id);
+		vi.spyOn(activityRepository, 'createActivity').mockRejectedValueOnce(new Error('write failed'));
+
+		await ownerAgent
+			.post(`/workflow-review-requests/${requestId}/decision`)
+			.send({ decision: 'approved' })
+			.expect(500);
+
+		expect(await requestRepository.findById(requestId, {})).toMatchObject({
+			state: 'open',
+			decision: 'pending',
+		});
+		expect(entryTypes(await getActivity(ownerAgent, requestId))).toEqual(['review.opened']);
+	});
+
+	// Only a stored row can be malformed, so these are seeded past the write union.
+	test.each([
+		['a decision whose payload is not an object', 'review.changes_requested', 'oops'],
+		['a close with a reason this version does not know', 'review.closed', { reason: 'nope' }],
+		['a version update missing its version ids', 'review.version_updated', { from: 1 }],
+		['an opening entry written in an older shape', 'review.opened', { index: 1 }],
+	])('serves %s without its details rather than failing the feed', async (_label, type, data) => {
+		const { request } = await seedReviewInTeamProject(owner);
+		await activityRepository.createActivity(
+			{ workflowReviewRequestId: request.id, type, data, createdById: owner.id } as never,
+			{},
+		);
+
+		const feed = await getActivity(ownerAgent, request.id);
+
+		expect(feed.data).toHaveLength(1);
+		expect(feed.data[0].type).toBe(type);
+		expect(feed.data[0].data).toBeNull();
+	});
+});
+
 describe('Reading the activity feed', () => {
 	/** Non-comment entries, cheap to seed and enough to pin the paging arithmetic. */
 	async function seedEntries(workflowReviewRequestId: string, count: number) {
@@ -361,7 +599,7 @@ describe('Reading the activity feed', () => {
 				{
 					workflowReviewRequestId,
 					type: 'review.opened',
-					data: { index },
+					data: { workflowVersions: [] },
 					createdById: owner.id,
 				},
 				{},
@@ -502,7 +740,10 @@ describe('Reading the activity feed', () => {
 
 	test('shows a non-comment activity entry with its details intact and no messages', async () => {
 		const { request } = await seedReviewInTeamProject(owner);
-		const data = { workflowVersionIds: ['version-pinned'], note: 'needs work' };
+		const data = {
+			workflowVersions: [{ workflowId: 'workflow-reviewed', workflowVersionId: 'version-pinned' }],
+			note: 'needs work',
+		};
 		await activityRepository.createActivity(
 			{
 				workflowReviewRequestId: request.id,

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

@@ -15,6 +15,7 @@ import {
 	SharedWorkflowRepository,
 	UserRepository,
 	WorkflowRepository,
+	WorkflowReviewActivityRepository,
 	WorkflowReviewRequestAuthorRepository,
 	WorkflowReviewRequestRepository,
 	WorkflowReviewRequestWorkflowRepository,
@@ -62,12 +63,14 @@ let ownerAgent: SuperAgentTest;
 let requestRepository: WorkflowReviewRequestRepository;
 let linkRepository: WorkflowReviewRequestWorkflowRepository;
 let authorRepository: WorkflowReviewRequestAuthorRepository;
+let activityRepository: WorkflowReviewActivityRepository;
 
 beforeAll(async () => {
 	await utils.initNodeTypes();
 	requestRepository = Container.get(WorkflowReviewRequestRepository);
 	linkRepository = Container.get(WorkflowReviewRequestWorkflowRepository);
 	authorRepository = Container.get(WorkflowReviewRequestAuthorRepository);
+	activityRepository = Container.get(WorkflowReviewActivityRepository);
 });
 
 beforeEach(async () => {
@@ -75,6 +78,8 @@ beforeEach(async () => {
 	testServer.license.enable('feat:workflowReviews');
 
 	await testDb.truncate([
+		'WorkflowReviewActivityComment',
+		'WorkflowReviewActivity',
 		'WorkflowReviewRequestAuthor',
 		'WorkflowReviewRequestReviewer',
 		'WorkflowReviewRequestWorkflow',
@@ -98,10 +103,18 @@ beforeEach(async () => {
 	ownerAgent = testServer.authAgentFor(owner);
 });
 
+/** Read through the endpoint, so the entries are asserted exactly as a reader sees them. */
+async function getActivityEntries(requestId: string) {
+	const response = await ownerAgent
+		.get(`/workflow-review-requests/${requestId}/activity`)
+		.expect(200);
+	return response.body.data.data as Array<{ type: string; data: { reason?: string } | null }>;
+}
+
 /** Create a workflow owned by `owner` with a pinned history version. */
-async function createReviewableWorkflow() {
+async function createReviewableWorkflow(attributes: { isArchived?: boolean } = {}) {
 	const versionId = uuid();
-	const workflow = await createWorkflow({ versionId }, owner);
+	const workflow = await createWorkflow({ versionId, ...attributes }, owner);
 	await createWorkflowHistoryItem(workflow.id, { versionId });
 	return { workflow, versionId };
 }
@@ -146,6 +159,54 @@ describe('auto-close on workflow archive', () => {
 		expect(closed?.decision).toBe('changes_requested');
 		expect(closed?.closedById).toBeNull();
 		expect(closed?.approvedAt).toBeNull();
+
+		expect(await getActivityEntries(request.id)).toEqual([
+			expect.objectContaining({
+				type: 'review.closed',
+				data: { reason: 'workflow-archived' },
+			}),
+		]);
+	});
+
+	// The close and its explanation share one transaction, so an unwritable entry rolls the
+	// close back — and archiving still succeeds, because cleanup never fails the mutation. The
+	// reconciliation sweep that follows the hook picks the rolled-back review straight back up.
+	test('archives the workflow anyway when the review cannot be explained, and the sweep closes it', async () => {
+		const { workflow, versionId } = await createReviewableWorkflow();
+		const request = await createOpenReview(workflow.id, versionId);
+		vi.spyOn(activityRepository, 'createActivity').mockRejectedValueOnce(new Error('write failed'));
+
+		await ownerAgent.post(`/workflows/${workflow.id}/archive`).expect(200);
+
+		expect((await requestRepository.findById(request.id, {}))?.state).toBe('closed');
+		expect(await getActivityEntries(request.id)).toEqual([
+			expect.objectContaining({ type: 'review.closed', data: { reason: 'workflow-archived' } }),
+		]);
+	});
+
+	// 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 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);
+		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();
+		await ownerAgent.post(`/workflows/${unrelated.workflow.id}/archive`).expect(200);
+
+		const closed = await requestRepository.findById(request.id, {});
+		expect(closed?.state).toBe('closed');
+		expect(closed?.closedById).toBeNull();
+		expect(await getActivityEntries(request.id)).toEqual([
+			expect.objectContaining({ type: 'review.closed', data: { reason: 'workflow-archived' } }),
+		]);
 	});
 
 	test('unarchiving does not reopen the review, and the workflow is no longer publish-blocked', async () => {
@@ -192,6 +253,48 @@ describe('auto-close on workflow transfer', () => {
 		expect(closed?.state).toBe('closed');
 		expect(closed?.decision).toBe('pending');
 		expect(await requestRepository.findOpenRequestForWorkflow(workflow.id, {})).toBeNull();
+
+		expect(await getActivityEntries(request.id)).toEqual([
+			expect.objectContaining({ type: 'review.closed', data: { reason: 'workflow-moved' } }),
+		]);
+	});
+
+	// 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 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,
+		);
+		expect((await requestRepository.findById(request.id, {}))?.state).toBe('open');
+
+		const unrelated = await createReviewableWorkflow();
+		await ownerAgent.post(`/workflows/${unrelated.workflow.id}/archive`).expect(200);
+
+		const closed = await requestRepository.findById(request.id, {});
+		expect(closed?.state).toBe('closed');
+		expect(await linkRepository.findByRequestId(request.id, {})).toHaveLength(1);
+		expect(await getActivityEntries(request.id)).toEqual([
+			expect.objectContaining({ type: 'review.closed', data: { reason: 'workflow-moved' } }),
+		]);
+	});
+
+	test('a review whose workflow is still in its project and unarchived is left open', async () => {
+		const { workflow, versionId } = await createReviewableWorkflow();
+		const request = await createOpenReview(workflow.id, versionId);
+
+		const unrelated = await createReviewableWorkflow();
+		await ownerAgent.post(`/workflows/${unrelated.workflow.id}/archive`).expect(200);
+
+		expect((await requestRepository.findById(request.id, {}))?.state).toBe('open');
 	});
 });
 
@@ -206,6 +309,26 @@ describe('auto-close on workflow hard delete', () => {
 		expect(closed?.state).toBe('closed');
 		// The link row cascaded away with the workflow; the request itself remains, closed.
 		expect(await linkRepository.findByRequestId(request.id, {})).toHaveLength(0);
+
+		// The pre-delete hook and the post-delete sweep both run here, so the count matters
+		// as much as the reason: the review must be explained exactly once.
+		expect(await getActivityEntries(request.id)).toEqual([
+			expect.objectContaining({ type: 'review.closed', data: { reason: 'workflow-deleted' } }),
+		]);
+	});
+
+	// The pre-delete hook shares one transaction with its explanation and rethrows, so an
+	// unwritable entry calls the delete off rather than leaving a closed review unexplained.
+	test('refuses to delete the workflow when the review cannot be explained', async () => {
+		const { workflow, versionId } = await createReviewableWorkflow({ isArchived: true });
+		const request = await createOpenReview(workflow.id, versionId);
+		vi.spyOn(activityRepository, 'createActivity').mockRejectedValueOnce(new Error('write failed'));
+
+		await ownerAgent.delete(`/workflows/${workflow.id}`).expect(500);
+
+		expect(await Container.get(WorkflowRepository).findOneBy({ id: workflow.id })).not.toBeNull();
+		expect((await requestRepository.findById(request.id, {}))?.state).toBe('open');
+		expect(await getActivityEntries(request.id)).toEqual([]);
 	});
 
 	// A review opened after the pre-delete hook ran loses its link row to the cascade and
@@ -220,12 +343,18 @@ describe('auto-close on workflow hard delete', () => {
 		expect((await requestRepository.findById(request.id, {}))?.state).toBe('open');
 		expect(await linkRepository.findByRequestId(request.id, {})).toHaveLength(0);
 
+		// `unrelated` has no review of its own, so the pre-delete hook finds nothing to close
+		// and the sweep is the only thing that can explain the orphaned review.
 		const unrelated = await createReviewableWorkflow();
 		await Container.get(WorkflowService).delete(owner, unrelated.workflow.id, true);
 
 		const closed = await requestRepository.findById(request.id, {});
 		expect(closed?.state).toBe('closed');
 		expect(closed?.closedById).toBeNull();
+
+		expect(await getActivityEntries(request.id)).toEqual([
+			expect.objectContaining({ type: 'review.closed', data: { reason: 'workflow-deleted' } }),
+		]);
 	});
 
 	test('leaves a review whose workflow still exists open', async () => {

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

@@ -1,8 +1,11 @@
 import type { Logger } from '@n8n/backend-common';
+import { DbLock } from '@n8n/db';
 import type {
 	DbLockService,
 	OperationContext,
 	Transaction,
+	WorkflowReviewActivity,
+	WorkflowReviewActivityRepository,
 	WorkflowReviewRequest,
 	WorkflowReviewRequestRepository,
 } from '@n8n/db';
@@ -15,6 +18,7 @@ import { WorkflowReviewAutoCloseService } from '../workflow-review-auto-close.se
 describe('WorkflowReviewAutoCloseService', () => {
 	const logger = mock<Logger>();
 	const requestRepository = mock<WorkflowReviewRequestRepository>();
+	const activityRepository = mock<WorkflowReviewActivityRepository>();
 	const dbLockService = mock<DbLockService>();
 	const collaborationService = mock<CollaborationService>();
 	/** The lock's context. Distinct from the root `{}` so tests can tell the two apart. */
@@ -23,6 +27,7 @@ describe('WorkflowReviewAutoCloseService', () => {
 	const service = new WorkflowReviewAutoCloseService(
 		logger,
 		requestRepository,
+		activityRepository,
 		dbLockService,
 		collaborationService,
 	);
@@ -40,6 +45,7 @@ describe('WorkflowReviewAutoCloseService', () => {
 		vi.resetAllMocks();
 		dbLockService.withLockContext.mockImplementation(async (_id, fn) => await fn(ctx));
 		requestRepository.saveRequest.mockImplementation(async (request) => request);
+		requestRepository.closeUnreviewableOpenRequests.mockResolvedValue([]);
 		collaborationService.broadcastWorkflowReviewStateChanged.mockResolvedValue(undefined);
 	});
 
@@ -132,38 +138,95 @@ describe('WorkflowReviewAutoCloseService', () => {
 		expect(logger.error).toHaveBeenCalled();
 	});
 
-	describe('afterWorkflowsDeleted', () => {
-		it('closes the requests the delete orphaned, outside any lock', async () => {
-			requestRepository.closeOrphanedOpenRequests.mockResolvedValue(['req-9']);
+	describe('reconciliation sweep', () => {
+		it('closes the requests the delete orphaned and explains each of them', async () => {
+			requestRepository.closeUnreviewableOpenRequests.mockResolvedValue([
+				{ id: 'req-9', reason: 'workflow-deleted' },
+			]);
+			activityRepository.createActivity.mockResolvedValue(mock<WorkflowReviewActivity>());
 
 			await service.afterWorkflowsDeleted(['wf-1', 'wf-2']);
 
-			// A single atomic statement pair, so it needs no lock — and must not take one
-			// after a delete, where camping on the create lock would serialize submissions.
-			// The sweep is global, so the batch never reaches the query; it is log context only.
-			expect(requestRepository.closeOrphanedOpenRequests).toHaveBeenCalledExactlyOnceWith({});
-			expect(dbLockService.withLockContext).not.toHaveBeenCalled();
+			// Same lock and same transaction as every other close path: the sweep updates the
+			// requests it selected by id, so two racing sweeps would otherwise explain the same
+			// close twice. The sweep is global, so the batch never reaches the query; it is log
+			// context only.
+			expect(dbLockService.withLockContext).toHaveBeenCalledWith(
+				DbLock.WORKFLOW_REVIEW_REQUEST_CREATE,
+				expect.any(Function),
+			);
+			expect(requestRepository.closeUnreviewableOpenRequests).toHaveBeenCalledExactlyOnceWith(ctx);
+			expect(activityRepository.createActivity).toHaveBeenCalledExactlyOnceWith(
+				{
+					workflowReviewRequestId: 'req-9',
+					type: 'review.closed',
+					data: { reason: 'workflow-deleted' },
+					createdById: null,
+				},
+				ctx,
+			);
 			expect(logger.info).toHaveBeenCalledExactlyOnceWith(
 				expect.any(String),
 				expect.objectContaining({
-					reason: 'workflow-deleted',
 					workflowIds: ['wf-1', 'wf-2'],
-					workflowReviewRequestIds: ['req-9'],
+					closedRequests: [{ id: 'req-9', reason: 'workflow-deleted' }],
 				}),
 			);
 		});
 
-		it('stays quiet when the delete orphaned nothing', async () => {
-			requestRepository.closeOrphanedOpenRequests.mockResolvedValue([]);
+		// The reason is per request, not per mutation: one sweep can find a review left by an
+		// archive next to one left by a move.
+		it('explains each request with the reason the sweep reported for it', async () => {
+			requestRepository.closeUnreviewableOpenRequests.mockResolvedValue([
+				{ id: 'req-1', reason: 'workflow-archived' },
+				{ id: 'req-2', reason: 'workflow-moved' },
+			]);
+			activityRepository.createActivity.mockResolvedValue(mock<WorkflowReviewActivity>());
+
+			await service.afterWorkflowsDeleted(['wf-1']);
+
+			expect(activityRepository.createActivity).toHaveBeenCalledWith(
+				expect.objectContaining({
+					workflowReviewRequestId: 'req-1',
+					data: { reason: 'workflow-archived' },
+				}),
+				ctx,
+			);
+			expect(activityRepository.createActivity).toHaveBeenCalledWith(
+				expect.objectContaining({
+					workflowReviewRequestId: 'req-2',
+					data: { reason: 'workflow-moved' },
+				}),
+				ctx,
+			);
+		});
+
+		it('stays quiet when nothing is left unreviewable', async () => {
+			requestRepository.closeUnreviewableOpenRequests.mockResolvedValue([]);
 
 			await service.afterWorkflowsDeleted(['wf-1']);
 
 			expect(logger.info).not.toHaveBeenCalled();
 		});
 
+		// The close and its explanation share one transaction, so an unwritable entry rolls the
+		// close back and the next sweep picks the review up again. The delete already committed,
+		// so there is nothing left to fail.
+		it('leaves a review it cannot explain to the next sweep', async () => {
+			requestRepository.closeUnreviewableOpenRequests.mockResolvedValue([
+				{ id: 'req-9', reason: 'workflow-deleted' },
+			]);
+			activityRepository.createActivity.mockRejectedValue(new Error('db down'));
+
+			await expect(service.afterWorkflowsDeleted(['wf-1'])).resolves.toBeUndefined();
+
+			expect(logger.error).toHaveBeenCalled();
+			expect(logger.info).not.toHaveBeenCalled();
+		});
+
 		// The delete already committed, so there is nothing left to abort.
 		it('swallows repository errors, unlike the pre-delete hook', async () => {
-			requestRepository.closeOrphanedOpenRequests.mockRejectedValue(new Error('db down'));
+			requestRepository.closeUnreviewableOpenRequests.mockRejectedValue(new Error('db down'));
 
 			await expect(service.afterWorkflowsDeleted(['wf-1', 'wf-2'])).resolves.toBeUndefined();
 
@@ -172,6 +235,37 @@ describe('WorkflowReviewAutoCloseService', () => {
 				expect.objectContaining({ workflowIds: ['wf-1', 'wf-2'] }),
 			);
 		});
+
+		// An archive or a move whose close rolled back stays committed, so the sweep has to run
+		// there too — the targeted close is done with the review by then.
+		it('runs after the targeted close on archive, and again on transfer', async () => {
+			requestRepository.findOpenRequestsForWorkflows.mockResolvedValue([]);
+
+			await service.afterWorkflowArchived('wf-1');
+			expect(requestRepository.closeUnreviewableOpenRequests).toHaveBeenCalledExactlyOnceWith(ctx);
+
+			await service.afterWorkflowsTransferred(['wf-2']);
+			expect(requestRepository.closeUnreviewableOpenRequests).toHaveBeenCalledTimes(2);
+		});
+
+		// A close that rolled back is exactly what the sweep is there to repair, so a throwing
+		// targeted close must not skip it.
+		it('still runs when the targeted close on archive failed', async () => {
+			requestRepository.findOpenRequestsForWorkflows.mockRejectedValue(new Error('db down'));
+
+			await expect(service.afterWorkflowArchived('wf-1')).resolves.toBeUndefined();
+
+			expect(requestRepository.closeUnreviewableOpenRequests).toHaveBeenCalledExactlyOnceWith(ctx);
+		});
+
+		// The pre-delete hook aborts the delete instead, so there is nothing to reconcile.
+		it('does not run before a delete', async () => {
+			requestRepository.findOpenRequestsForWorkflows.mockResolvedValue([]);
+
+			await service.beforeWorkflowDeleted('wf-1');
+
+			expect(requestRepository.closeUnreviewableOpenRequests).not.toHaveBeenCalled();
+		});
 	});
 
 	it('a failed broadcast is only warned about, never thrown', async () => {

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

@@ -2,6 +2,7 @@ import type { DecideWorkflowReviewRequestDto } from '@n8n/api-types';
 import type { LicenseState, Logger } from '@n8n/backend-common';
 import type {
 	DbLockService,
+	Project,
 	ProjectRelationRepository,
 	SharedWorkflowRepository,
 	User,
@@ -12,6 +13,7 @@ import type {
 	WorkflowReviewRequest,
 	WorkflowReviewRequestAuthorRepository,
 	WorkflowReviewRequestRepository,
+	WorkflowReviewActivityRepository,
 	WorkflowReviewRequestReviewerRepository,
 	WorkflowReviewRequestWorkflow,
 	WorkflowReviewRequestWorkflowRepository,
@@ -23,6 +25,7 @@ import { DbLock } 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';
@@ -46,19 +49,24 @@ const requesterUser = mock<User>({
 const requestId = 'req-1';
 const projectId = 'proj-1';
 const approveDto: DecideWorkflowReviewRequestDto = { decision: 'approved' };
-const requestChangesDto: DecideWorkflowReviewRequestDto = { decision: 'changes_requested' };
+const requestChangesDto: DecideWorkflowReviewRequestDto = {
+	decision: 'changes_requested',
+	note: 'Please rename the node',
+};
 
 describe('WorkflowReviewRequestService.decide', () => {
 	const workflowReviewPolicyService = mock<WorkflowReviewPolicyService>();
 	const workflowFinderService = mock<WorkflowFinderService>();
 	const workflowHistoryService = mock<WorkflowHistoryService>();
 	const workflowHistoryRepository = mock<WorkflowHistoryRepository>();
+	const workflowEntityRepository = mock<WorkflowRepository>();
 	const sharedWorkflowRepository = mock<SharedWorkflowRepository>();
 	const publishHistoryRepository = mock<WorkflowPublishHistoryRepository>();
 	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 projectRelationRepository = mock<ProjectRelationRepository>();
 	const roleService = mock<RoleService>();
@@ -76,13 +84,14 @@ describe('WorkflowReviewRequestService.decide', () => {
 		workflowFinderService,
 		workflowHistoryService,
 		workflowHistoryRepository,
-		mock<WorkflowRepository>(),
+		workflowEntityRepository,
 		sharedWorkflowRepository,
 		publishHistoryRepository,
 		requestRepository,
 		workflowRepository,
 		authorRepository,
 		reviewerRepository,
+		activityRepository,
 		userRepository,
 		// Real service over the same mocks, so the override assertions below
 		// exercise the actual eligibility logic decide() shares with the read side.
@@ -125,6 +134,10 @@ describe('WorkflowReviewRequestService.decide', () => {
 		workflowFinderService.findWorkflowForUser.mockResolvedValue(
 			mock<WorkflowEntity>({ isArchived: false }),
 		);
+		workflowEntityRepository.findArchivedState.mockResolvedValue({ isArchived: false });
+		sharedWorkflowRepository.getWorkflowOwningProject.mockResolvedValue(
+			mock<Project>({ id: projectId }),
+		);
 		authorRepository.isAuthor.mockResolvedValue(false);
 		reviewerRepository.isReviewer.mockResolvedValue(true);
 		projectRelationRepository.getAccessibleProjectsByRoles.mockResolvedValue([]);
@@ -231,6 +244,18 @@ describe('WorkflowReviewRequestService.decide', () => {
 			expect(dbLockService.withLockContext).not.toHaveBeenCalled();
 		});
 
+		// The missing note is a payload problem, and an author is not entitled to hear about it:
+		// they may not decide at all, whatever they sent.
+		it('tells an author they may not decide even when their note is missing too', async () => {
+			mockSuccessfulDecidePath();
+			authorRepository.isAuthor.mockResolvedValue(true);
+			projectRelationRepository.getAccessibleProjectsByRoles.mockResolvedValue([]);
+
+			await expect(
+				service.decide(memberUser(), requestId, { decision: 'changes_requested' }),
+			).rejects.toThrow(ForbiddenError);
+		});
+
 		it('rejects a caller who became an author while waiting for the lock', async () => {
 			mockSuccessfulDecidePath();
 			// Not an author before the lock, but a version sync won the lock first and
@@ -392,6 +417,27 @@ describe('WorkflowReviewRequestService.decide', () => {
 		expect(collaborationService.broadcastWorkflowReviewStateChanged).not.toHaveBeenCalled();
 	});
 
+	it('refuses to approve a workflow archived while the decision waited for the lock', async () => {
+		mockSuccessfulDecidePath();
+		// The pre-lock lookups still see a live workflow; only the in-lock re-read
+		// sees the archive that committed while this decision queued.
+		workflowEntityRepository.findArchivedState.mockResolvedValue({ isArchived: true });
+
+		const decision = service.decide(memberUser(), requestId, approveDto);
+		await expect(decision).rejects.toThrow(BadRequestError);
+		// The reviewer is told their review was refused, not that the workflow
+		// "cannot be submitted for review" — that is the author's action, not theirs.
+		await expect(decision).rejects.toThrow(
+			"The workflow 'wf-1' is archived and cannot be reviewed",
+		);
+
+		// Nothing may reach the activity feed: an approval entry here would durably
+		// assert a decision on a workflow that had already left the reviewable state.
+		expect(activityRepository.createActivity).not.toHaveBeenCalled();
+		expect(requestRepository.saveRequest).not.toHaveBeenCalled();
+		expect(workflowService.activateWorkflow).not.toHaveBeenCalled();
+	});
+
 	it('reports and publishes the version re-pinned by a concurrent sync that won the lock', async () => {
 		mockSuccessfulDecidePath();
 		workflowRepository.findByRequestId
@@ -549,6 +595,15 @@ describe('WorkflowReviewRequestService.decide', () => {
 				status: 'failed',
 				message: 'The reviewed workflow version no longer exists',
 			});
+			// A `[null]` here is rejected on read, which would take the reviewer's note down
+			// with it and leave an approval nobody can account for.
+			expect(activityRepository.createActivity).toHaveBeenCalledWith(
+				expect.objectContaining({
+					type: 'review.approved',
+					data: { workflowVersions: [], note: null },
+				}),
+				ctx,
+			);
 		});
 	});
 

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

@@ -18,6 +18,7 @@ import type {
 	WorkflowReviewRequestAuthorRepository,
 	WorkflowReviewRequestRepository,
 	WorkflowReviewRequestForWorkflowRow,
+	WorkflowReviewActivityRepository,
 	WorkflowReviewRequestReviewerRepository,
 	WorkflowReviewRequestWorkflowRepository,
 	WorkflowRepository,
@@ -71,6 +72,7 @@ describe('WorkflowReviewRequestService', () => {
 	const workflowRepository = mock<WorkflowReviewRequestWorkflowRepository>();
 	const authorRepository = mock<WorkflowReviewRequestAuthorRepository>();
 	const reviewerRepository = mock<WorkflowReviewRequestReviewerRepository>();
+	const activityRepository = mock<WorkflowReviewActivityRepository>();
 	const userRepository = mock<UserRepository>();
 	const eligibilityService = mock<WorkflowReviewEligibilityService>();
 	const roleService = mock<RoleService>();
@@ -95,6 +97,7 @@ describe('WorkflowReviewRequestService', () => {
 		workflowRepository,
 		authorRepository,
 		reviewerRepository,
+		activityRepository,
 		userRepository,
 		eligibilityService,
 		roleService,
@@ -264,7 +267,11 @@ describe('WorkflowReviewRequestService', () => {
 			mockSuccessfulCreatePath();
 			workflowEntityRepository.findArchivedState.mockResolvedValue({ isArchived: true });
 
-			await expect(service.create(user, dto)).rejects.toThrow(BadRequestError);
+			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();

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

@@ -3,6 +3,7 @@ import type { LicenseState, Logger } from '@n8n/backend-common';
 import { DbLock } from '@n8n/db';
 import type {
 	DbLockService,
+	Project,
 	SharedWorkflowRepository,
 	User,
 	UserRepository,
@@ -13,6 +14,7 @@ import type {
 	WorkflowReviewRequest,
 	WorkflowReviewRequestAuthorRepository,
 	WorkflowReviewRequestRepository,
+	WorkflowReviewActivityRepository,
 	WorkflowReviewRequestReviewerRepository,
 	WorkflowReviewRequestWorkflow,
 	WorkflowReviewRequestWorkflowRepository,
@@ -40,6 +42,7 @@ import { WorkflowReviewRequestService } from '../workflow-review-request.service
 const user = mock<User>({ id: 'user-1' });
 
 const requestId = 'req-1';
+const projectId = 'proj-1';
 const dto: UpdateWorkflowReviewRequestVersionDto = {
 	workflowId: 'wf-1',
 	workflowVersionId: 'ver-2',
@@ -51,12 +54,14 @@ describe('WorkflowReviewRequestService.updateVersion', () => {
 	const workflowFinderService = mock<WorkflowFinderService>();
 	const workflowHistoryService = mock<WorkflowHistoryService>();
 	const workflowHistoryRepository = mock<WorkflowHistoryRepository>();
+	const workflowEntityRepository = mock<WorkflowRepository>();
 	const sharedWorkflowRepository = mock<SharedWorkflowRepository>();
 	const publishHistoryRepository = mock<WorkflowPublishHistoryRepository>();
 	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 eligibilityService = mock<WorkflowReviewEligibilityService>();
 	const roleService = mock<RoleService>();
@@ -74,13 +79,14 @@ describe('WorkflowReviewRequestService.updateVersion', () => {
 		workflowFinderService,
 		workflowHistoryService,
 		workflowHistoryRepository,
-		mock<WorkflowRepository>(),
+		workflowEntityRepository,
 		sharedWorkflowRepository,
 		publishHistoryRepository,
 		requestRepository,
 		workflowRepository,
 		authorRepository,
 		reviewerRepository,
+		activityRepository,
 		userRepository,
 		eligibilityService,
 		roleService,
@@ -92,6 +98,7 @@ describe('WorkflowReviewRequestService.updateVersion', () => {
 	const openRequest = (overrides: Partial<WorkflowReviewRequest> = {}) =>
 		mock<WorkflowReviewRequest>({
 			id: requestId,
+			projectId,
 			state: 'open',
 			decision: 'pending',
 			description: null,
@@ -112,6 +119,10 @@ describe('WorkflowReviewRequestService.updateVersion', () => {
 		workflowFinderService.findWorkflowForUser.mockResolvedValue(
 			mock<WorkflowEntity>({ isArchived: false }),
 		);
+		workflowEntityRepository.findArchivedState.mockResolvedValue({ isArchived: false });
+		sharedWorkflowRepository.getWorkflowOwningProject.mockResolvedValue(
+			mock<Project>({ id: projectId }),
+		);
 		workflowHistoryService.findVersion.mockResolvedValue(mock());
 		workflowHistoryRepository.updateVersionMetadata.mockResolvedValue(1);
 		requestRepository.saveRequest.mockImplementation(async (request) => request);
@@ -303,6 +314,25 @@ describe('WorkflowReviewRequestService.updateVersion', () => {
 		expect(collaborationService.broadcastWorkflowReviewStateChanged).not.toHaveBeenCalled();
 	});
 
+	it('refuses to re-pin a workflow archived while the update waited for the lock', async () => {
+		mockSuccessfulUpdatePath();
+		// The pre-lock lookups still see a live workflow; only the in-lock re-read
+		// sees the archive that committed while this update queued.
+		workflowEntityRepository.findArchivedState.mockResolvedValue({ isArchived: true });
+
+		const update = service.updateVersion(user, requestId, dto);
+		await expect(update).rejects.toThrow(BadRequestError);
+		await expect(update).rejects.toThrow(
+			"The workflow 'wf-1' is archived and cannot be submitted as a new review version",
+		);
+
+		// Nothing may reach the activity feed: a version_updated entry here would
+		// durably assert a re-pin on a workflow that can no longer be reviewed.
+		expect(activityRepository.createActivity).not.toHaveBeenCalled();
+		expect(workflowRepository.updateWorkflowVersion).not.toHaveBeenCalled();
+		expect(requestRepository.saveRequest).not.toHaveBeenCalled();
+	});
+
 	it('throws NotFoundError when the request disappears between check and lock', async () => {
 		mockSuccessfulUpdatePath();
 		requestRepository.findById.mockResolvedValueOnce(openRequest()).mockResolvedValueOnce(null);

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

@@ -81,6 +81,8 @@ beforeEach(async () => {
 	testServer.license.enable('feat:workflowReviews');
 
 	await testDb.truncate([
+		'WorkflowReviewActivityComment',
+		'WorkflowReviewActivity',
 		'WorkflowReviewRequestAuthor',
 		'WorkflowReviewRequestReviewer',
 		'WorkflowReviewRequestWorkflow',
@@ -1535,7 +1537,7 @@ describe('POST /workflow-review-requests/:workflowReviewRequestId/decision', ()
 
 		const response = await memberAgent
 			.post(`/workflow-review-requests/${request.id}/decision`)
-			.send({ decision: 'changes_requested' })
+			.send({ decision: 'changes_requested', note: 'Please rename the node' })
 			.expect(200);
 
 		expect(response.body.data).toMatchObject({
@@ -1576,7 +1578,7 @@ describe('POST /workflow-review-requests/:workflowReviewRequestId/decision', ()
 
 		await memberAgent
 			.post(`/workflow-review-requests/${request.id}/decision`)
-			.send({ decision: 'changes_requested' })
+			.send({ decision: 'changes_requested', note: 'Please rename the node' })
 			.expect(200);
 
 		expect(await requestRepository.findById(request.id, {})).toMatchObject({
@@ -1665,7 +1667,7 @@ describe('POST /workflow-review-requests/:workflowReviewRequestId/decision', ()
 
 		await viewerAgent
 			.post(`/workflow-review-requests/${request.id}/decision`)
-			.send({ decision: 'changes_requested' })
+			.send({ decision: 'changes_requested', note: 'Please rename the node' })
 			.expect(200);
 
 		expect(await requestRepository.findById(request.id, {})).toMatchObject({

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

@@ -0,0 +1,49 @@
+import type { Logger } from '@n8n/backend-common';
+import type { WorkflowReviewActivity } from '@n8n/db';
+import { mock } from 'vitest-mock-extended';
+
+import { toActivityEntry } from '../workflow-review.mapper';
+
+describe('toActivityEntry', () => {
+	const logger = mock<Logger>();
+
+	// The stored `type` is CHECK-constrained, so only a downgrade to a version that knows fewer
+	// types can produce this row — which is why it cannot be reached through the endpoints.
+	it('serves an entry of a type this version does not know rather than dropping it', () => {
+		const row = mock<WorkflowReviewActivity>({
+			id: 7,
+			typeVersion: 1,
+			type: 'review.something_newer' as WorkflowReviewActivity['type'],
+			data: { anything: true },
+			createdById: null,
+			createdAt: new Date('2026-07-20T10:00:00.000Z'),
+		});
+
+		expect(toActivityEntry(row, [], new Map(), logger)).toEqual({
+			id: '7',
+			typeVersion: 1,
+			type: 'review.something_newer',
+			data: null,
+			createdBy: null,
+			createdAt: '2026-07-20T10:00:00.000Z',
+		});
+	});
+
+	// Same reason: only a downgrade writes a payload version this code does not know, and its
+	// shape may happen to satisfy the version 1 schema while meaning something else.
+	it('does not read a newer payload version as if it were the one it knows', () => {
+		const row = mock<WorkflowReviewActivity>({
+			id: 8,
+			typeVersion: 2,
+			type: 'review.approved',
+			data: {
+				workflowVersions: [{ workflowId: 'wf-1', workflowVersionId: 'ver-1' }],
+				note: 'Ship it',
+			},
+			createdById: null,
+			createdAt: new Date('2026-07-20T10:00:00.000Z'),
+		});
+
+		expect(toActivityEntry(row, [], new Map(), logger).data).toBeNull();
+	});
+});

packages/cli/src/modules/workflow-reviews.ee/workflow-review-activity.service.ts

@@ -5,6 +5,7 @@ import type {
 	WorkflowReviewActivityEntry,
 	WorkflowReviewEligibleReviewer,
 } from '@n8n/api-types';
+import { Logger } from '@n8n/backend-common';
 import {
 	TransactionRunner,
 	UserRepository,
@@ -33,6 +34,7 @@ export class WorkflowReviewActivityService {
 		private readonly activityCommentRepository: WorkflowReviewActivityCommentRepository,
 		private readonly userRepository: UserRepository,
 		private readonly txRunner: TransactionRunner,
+		private readonly logger: Logger,
 	) {}
 
 	async listActivity(
@@ -96,7 +98,12 @@ export class WorkflowReviewActivityService {
 			return { activity, message };
 		});
 
-		return toActivityEntry(activity, [message], new Map([[user.id, toEligibleReviewer(user)]]));
+		return toActivityEntry(
+			activity,
+			[message],
+			new Map([[user.id, toEligibleReviewer(user)]]),
+			this.logger,
+		);
 	}
 
 	private async hydrate(
@@ -109,7 +116,9 @@ export class WorkflowReviewActivityService {
 			]),
 		);
 
-		return entries.map((entry) => toActivityEntry(entry.activity, entry.messages, usersById));
+		return entries.map((entry) =>
+			toActivityEntry(entry.activity, entry.messages, usersById, this.logger),
+		);
 	}
 
 	private async hydrateAuthors(

packages/cli/src/modules/workflow-reviews.ee/workflow-review-auto-close.service.ts

@@ -1,12 +1,16 @@
+import type { WorkflowReviewClosedReason } from '@n8n/api-types';
 import { Logger } from '@n8n/backend-common';
-import { DbLock, DbLockService, WorkflowReviewRequestRepository } from '@n8n/db';
+import {
+	DbLock,
+	DbLockService,
+	WorkflowReviewActivityRepository,
+	WorkflowReviewRequestRepository,
+} from '@n8n/db';
 import { Service } from '@n8n/di';
 
 import { CollaborationService } from '@/collaboration/collaboration.service';
 import type { WorkflowMutationHooks } from '@/workflows/workflow-mutation-hooks-proxy.service';
 
-type AutoCloseReason = 'workflow-archived' | 'workflow-moved' | 'workflow-deleted';
-
 /**
  * Closes open review requests when their workflow stops being reviewable:
  * archived, moved to another project, or deleted.
@@ -21,16 +25,19 @@ export class WorkflowReviewAutoCloseService implements WorkflowMutationHooks {
 	constructor(
 		private readonly logger: Logger,
 		private readonly workflowReviewRequestRepository: WorkflowReviewRequestRepository,
+		private readonly activityRepository: WorkflowReviewActivityRepository,
 		private readonly dbLockService: DbLockService,
 		private readonly collaborationService: CollaborationService,
 	) {}
 
 	async afterWorkflowArchived(workflowId: string): Promise<void> {
 		await this.closeOpenRequestsForWorkflows([workflowId], 'workflow-archived');
+		await this.reconcileUnreviewableOpenRequests([workflowId]);
 	}
 
 	async afterWorkflowsTransferred(workflowIds: string[]): Promise<void> {
 		await this.closeOpenRequestsForWorkflows(workflowIds, 'workflow-moved');
+		await this.reconcileUnreviewableOpenRequests(workflowIds);
 	}
 
 	async beforeWorkflowDeleted(workflowId: string): Promise<void> {
@@ -41,38 +48,71 @@ export class WorkflowReviewAutoCloseService implements WorkflowMutationHooks {
 		});
 	}
 
-	/**
-	 * Closes reviews whose workflow is gone: the cascade took their link rows, so they
-	 * can only be found by "open with nothing linked", not by workflow id — the sweep is
-	 * global and one call covers the whole batch. Catches reviews opened after the
-	 * pre-delete hooks ran, and any left by a delete that skips the hooks.
-	 */
 	async afterWorkflowsDeleted(workflowIds: string[]): Promise<void> {
+		await this.reconcileUnreviewableOpenRequests(workflowIds);
+	}
+
+	/**
+	 * Closes every open review whose workflow is no longer reviewable, whatever left it that way.
+	 * The sweep is global — one call covers the whole batch, and the ids are log context only.
+	 *
+	 * The per-mutation close above cannot cover two cases. A delete cascades the link rows away,
+	 * so the reviews it strands are findable only as "open with nothing linked", never by workflow
+	 * id. And an archive or a move whose close rolled back stays committed, leaving a review its
+	 * own hook has already finished with — so the sweep runs after those hooks too, in its own
+	 * transaction, and repairs the strand on that mutation or on the next one.
+	 *
+	 * No broadcast: the reviews it closes are a rare inconsistency rather than a live edit, and
+	 * viewers heal on the next focus/reconnect refetch.
+	 */
+	private async reconcileUnreviewableOpenRequests(workflowIds: string[]): Promise<void> {
 		try {
-			const closedRequestIds = await this.workflowReviewRequestRepository.closeOrphanedOpenRequests(
-				{},
+			const closedRequests = await this.dbLockService.withLockContext(
+				DbLock.WORKFLOW_REVIEW_REQUEST_CREATE,
+				async (ctx) => {
+					const requests =
+						await this.workflowReviewRequestRepository.closeUnreviewableOpenRequests(ctx);
+
+					// Under the same lock and in the same transaction as every other close path: the
+					// sweep selects the requests and then updates them by id, so two sweeps racing
+					// across that gap would both explain the same close and both overwrite whatever a
+					// concurrent approval wrote. A failed entry rolls the close back and the next
+					// sweep closes it again, which is what the sweep is for.
+					for (const { id, reason } of requests) {
+						await this.activityRepository.createActivity(
+							{
+								workflowReviewRequestId: id,
+								type: 'review.closed',
+								data: { reason },
+								createdById: null,
+							},
+							ctx,
+						);
+					}
+
+					return requests;
+				},
 			);
 
-			if (closedRequestIds.length === 0) return;
+			if (closedRequests.length === 0) return;
 
-			this.logger.info('Closed workflow review request(s) left without a workflow', {
-				reason: 'workflow-deleted',
+			this.logger.info('Closed open workflow review request(s) left on unreviewable workflows', {
 				workflowIds,
-				workflowReviewRequestIds: closedRequestIds,
+				closedRequests,
 			});
 		} catch (error) {
-			// The delete has already committed; failing it now would be worse than a
+			// The mutation has already committed; failing it now would be worse than a
 			// request that stays open until the next sweep closes it.
-			this.logger.error('Failed to close workflow review request(s) left without a workflow', {
-				workflowIds,
-				error,
-			});
+			this.logger.error(
+				'Failed to close open workflow review request(s) left on unreviewable workflows',
+				{ workflowIds, error },
+			);
 		}
 	}
 
 	private async closeOpenRequestsForWorkflows(
 		workflowIds: string[],
-		reason: AutoCloseReason,
+		reason: WorkflowReviewClosedReason,
 		options: { rethrow?: boolean } = {},
 	): Promise<void> {
 		try {
@@ -93,6 +133,21 @@ export class WorkflowReviewAutoCloseService implements WorkflowMutationHooks {
 						// A system close has no closing user; the decision stays as-is.
 						request.closedById = null;
 						await this.workflowReviewRequestRepository.saveRequest(request, ctx);
+
+						// In the transaction on purpose: a review closed without an explanation is
+						// worse than a close that rolls back. Before a delete the rollback calls the
+						// delete off; for an archive or a move the mutation stands and the review
+						// stays open until the reconciliation sweep closes it again.
+						await this.activityRepository.createActivity(
+							{
+								workflowReviewRequestId: request.id,
+								type: 'review.closed',
+								data: { reason },
+								createdById: null,
+							},
+							ctx,
+						);
+
 						for (const linkedWorkflowId of linkedWorkflowIds) affected.add(linkedWorkflowId);
 					}
 
@@ -121,8 +176,9 @@ export class WorkflowReviewAutoCloseService implements WorkflowMutationHooks {
 				workflowIds,
 				error,
 			});
-			// Cleanup must never fail a mutation that already committed; only the
-			// pre-delete hook is still in a position to call its mutation off.
+			// Cleanup does not fail a mutation that already committed — archive and move end
+			// here, having rolled the close back. Only the pre-delete hook is still in a
+			// position to call its mutation off.
 			if (options.rethrow) throw error;
 		}
 	}

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

@@ -24,6 +24,7 @@ import {
 	WorkflowReviewRequestAuthorRepository,
 	WorkflowReviewRequestRepository,
 	WorkflowRepository,
+	WorkflowReviewActivityRepository,
 	WorkflowReviewRequestReviewerRepository,
 	WorkflowReviewRequestWorkflowRepository,
 	type OperationContext,
@@ -58,6 +59,15 @@ function normalizeReviewDescription(description: string | undefined): string | n
 	return description.trim() || null;
 }
 
+/** What the caller was trying to do, so a refusal names that action rather than another. */
+type BlockedAction = 'submit' | 'review' | 'update';
+
+const BLOCKED_ACTION_TEXT: Record<BlockedAction, string> = {
+	submit: 'submitted for review',
+	review: 'reviewed',
+	update: 'submitted as a new review version',
+};
+
 /**
  * The workflow-scoped review request lifecycle: listing a workflow's reviews,
  * resolving its eligible reviewers, opening a review, re-pinning it to a new
@@ -79,6 +89,7 @@ export class WorkflowReviewRequestService {
 		private readonly workflowReviewRequestWorkflowRepository: WorkflowReviewRequestWorkflowRepository,
 		private readonly workflowReviewRequestAuthorRepository: WorkflowReviewRequestAuthorRepository,
 		private readonly workflowReviewRequestReviewerRepository: WorkflowReviewRequestReviewerRepository,
+		private readonly activityRepository: WorkflowReviewActivityRepository,
 		private readonly userRepository: UserRepository,
 		private readonly eligibilityService: WorkflowReviewEligibilityService,
 		private readonly roleService: RoleService,
@@ -289,7 +300,7 @@ export class WorkflowReviewRequestService {
 	}
 
 	/**
-	 * Re-asserts under the lock what `create`'s pre-lock checks established: the
+	 * Re-asserts under the lock what the caller's pre-lock checks established: the
 	 * workflow still exists, is unarchived, and still belongs to the same project.
 	 * Access is not re-checked — the races this guards against are archive and
 	 * transfer, both of which these two reads cover.
@@ -302,7 +313,9 @@ export class WorkflowReviewRequestService {
 		workflowId: string,
 		expectedProjectId: string,
 		ctx: OperationContext,
+		action: BlockedAction,
 	): Promise<void> {
+		const blockedAction = BLOCKED_ACTION_TEXT[action];
 		const workflow = await this.workflowRepository.findArchivedState(workflowId, ctx);
 		if (!workflow) {
 			throw new NotFoundError('Could not find workflow');
@@ -310,14 +323,14 @@ export class WorkflowReviewRequestService {
 
 		if (workflow.isArchived) {
 			throw new BadRequestError(
-				`The workflow '${workflowId}' is archived and cannot be submitted for review`,
+				`The workflow '${workflowId}' is archived and cannot be ${blockedAction}`,
 			);
 		}
 
 		const project = await this.sharedWorkflowRepository.getWorkflowOwningProject(workflowId, ctx);
 		if (project?.id !== expectedProjectId) {
 			throw new ConflictError(
-				`The workflow '${workflowId}' moved to another project and cannot be submitted for review here`,
+				`The workflow '${workflowId}' moved to another project and cannot be ${blockedAction} here`,
 				'Retry from the project that now owns the workflow',
 			);
 		}
@@ -384,7 +397,7 @@ export class WorkflowReviewRequestService {
 			async (ctx) => {
 				// without this, a create that lost the race opens a review on an archived
 				// or moved workflow.
-				await this.assertWorkflowStillReviewable(workflowId, project.id, ctx);
+				await this.assertWorkflowStillReviewable(workflowId, project.id, ctx, 'submit');
 
 				const existing = await this.workflowReviewRequestRepository.findOpenRequestForWorkflow(
 					workflowId,
@@ -430,6 +443,19 @@ export class WorkflowReviewRequestService {
 					ctx,
 				);
 
+				// Records the opening version: the pin only ever holds the current one, and
+				// pruning spares it only while the review is open, so a re-pin or a close
+				// leaves the original prunable and its `SET NULL` erases the last trace.
+				await this.activityRepository.createActivity(
+					{
+						workflowReviewRequestId: created.id,
+						type: 'review.opened',
+						data: { workflowVersions: [{ workflowId, workflowVersionId }] },
+						createdById: user.id,
+					},
+					ctx,
+				);
+
 				return created;
 			},
 		);
@@ -547,6 +573,16 @@ export class WorkflowReviewRequestService {
 				if (!currentRow) {
 					throw new NotFoundError('Could not find review request');
 				}
+
+				// Archive and transfer run in after hooks, so they commit before queueing
+				// on this lock — the pre-lock check can already be stale here.
+				await this.assertWorkflowStillReviewable(
+					currentRow.workflowId,
+					current.projectId,
+					ctx,
+					'update',
+				);
+
 				if (currentRow.workflowVersionId === dto.workflowVersionId) {
 					// A concurrent sync won the lock and already re-pinned this version
 					// but our rename or re-description can still be pending — apply it
@@ -572,6 +608,9 @@ export class WorkflowReviewRequestService {
 					return { request: saved, changed: true };
 				}
 
+				// Captured before the update.
+				const fromWorkflowVersionId = currentRow.workflowVersionId;
+
 				await this.workflowReviewRequestWorkflowRepository.updateWorkflowVersion(
 					{
 						workflowReviewRequestId,
@@ -601,6 +640,20 @@ export class WorkflowReviewRequestService {
 					ctx,
 				);
 
+				await this.activityRepository.createActivity(
+					{
+						workflowReviewRequestId,
+						type: 'review.version_updated',
+						data: {
+							workflowId: dto.workflowId,
+							fromWorkflowVersionId,
+							toWorkflowVersionId: dto.workflowVersionId,
+						},
+						createdById: user.id,
+					},
+					ctx,
+				);
+
 				return { request: saved, changed: true };
 			},
 		);
@@ -680,6 +733,13 @@ export class WorkflowReviewRequestService {
 				? await this.resolveRequesterPublishability(request.createdById, workflowRow.workflowId)
 				: null;
 
+		// After the lifecycle and permission checks: a state or permission error outranks a
+		// payload error, so an author keeps being told they may not decide at all rather than
+		// that their note was the problem.
+		if (dto.decision === 'changes_requested' && !dto.note) {
+			throw new BadRequestError('A note is required when requesting changes');
+		}
+
 		const { request: saved, pinnedVersionId } = await this.dbLockService.withLockContext(
 			DbLock.WORKFLOW_REVIEW_REQUEST_CREATE,
 			async (ctx) => {
@@ -719,6 +779,17 @@ export class WorkflowReviewRequestService {
 					throw new NotFoundError('Could not find review request');
 				}
 
+				// Archive and transfer run in after hooks, so they commit before queueing
+				// on this lock — the pre-lock check can already be stale here.
+				// 'reviewed' covers both decisions: an approval and a change request
+				// are equally refused here.
+				await this.assertWorkflowStillReviewable(
+					currentRow.workflowId,
+					current.projectId,
+					ctx,
+					'review',
+				);
+
 				current.decision = dto.decision;
 				current.updatedById = user.id;
 				if (dto.decision === 'approved') {
@@ -730,6 +801,25 @@ export class WorkflowReviewRequestService {
 				}
 
 				const savedRequest = await this.workflowReviewRequestRepository.saveRequest(current, ctx);
+
+				await this.activityRepository.createActivity(
+					{
+						workflowReviewRequestId,
+						type: dto.decision === 'approved' ? 'review.approved' : 'review.changes_requested',
+						data: {
+							// A pruned pin drops out rather than landing as a null in the array.
+							workflowVersions: currentRows.flatMap((row) =>
+								row.workflowVersionId === null
+									? []
+									: [{ workflowId: row.workflowId, workflowVersionId: row.workflowVersionId }],
+							),
+							note: dto.note ?? null,
+						},
+						createdById: user.id,
+					},
+					ctx,
+				);
+
 				return { request: savedRequest, pinnedVersionId: currentRow.workflowVersionId };
 			},
 		);

packages/cli/src/modules/workflow-reviews.ee/workflow-review.mapper.ts

@@ -1,5 +1,14 @@
-import type { WorkflowReviewActivityEntry, WorkflowReviewEligibleReviewer } from '@n8n/api-types';
+import {
+	workflowReviewClosedActivityDataSchema,
+	workflowReviewDecisionActivityDataSchema,
+	workflowReviewOpenedActivityDataSchema,
+	workflowReviewVersionUpdatedActivityDataSchema,
+	type WorkflowReviewActivityEntry,
+	type WorkflowReviewEligibleReviewer,
+} from '@n8n/api-types';
+import type { Logger } from '@n8n/backend-common';
 import type { User, WorkflowReviewActivity, WorkflowReviewActivityComment } from '@n8n/db';
+import type { z } from 'zod';
 
 /**
  * Project a user onto the boundary shape the review endpoints are allowed to expose.
@@ -15,10 +24,36 @@ export function toEligibleReviewer(user: User): WorkflowReviewEligibleReviewer {
 	};
 }
 
+/**
+ * A stored payload the schema rejects becomes `null` rather than an exception: one bad row must
+ * not take down the whole feed page.
+ */
+function parsePayload<T>(
+	schema: z.ZodType<T>,
+	row: WorkflowReviewActivity,
+	logger: Logger,
+): T | null {
+	// These schemas describe typeVersion 1 only; a later version gets its own schema and case.
+	if (row.typeVersion === 1) {
+		const parsed = schema.safeParse(row.data);
+		if (parsed.success) {
+			return parsed.data;
+		}
+	}
+
+	logger.warn('Failed to parse workflow review activity payload', {
+		activityId: row.id,
+		type: row.type,
+		typeVersion: row.typeVersion,
+	});
+	return null;
+}
+
 export function toActivityEntry(
 	row: WorkflowReviewActivity,
 	messages: WorkflowReviewActivityComment[],
 	usersById: Map<string, WorkflowReviewEligibleReviewer>,
+	logger: Logger,
 ): WorkflowReviewActivityEntry {
 	const createdBy = row.createdById ? (usersById.get(row.createdById) ?? null) : null;
 	const base = {
@@ -28,23 +63,53 @@ export function toActivityEntry(
 		createdAt: row.createdAt.toISOString(),
 	};
 
-	if (row.type === 'comment.created') {
-		return {
-			...base,
-			type: row.type,
-			data: null,
-			messages: messages.map((message) => ({
-				id: String(message.id),
-				// Never send the body of a deleted comment, even if the row still holds one, so a
-				// delete path that only sets `deletedAt` cannot leak it.
-				body: message.deletedAt ? null : message.body,
-				createdBy: message.createdById ? (usersById.get(message.createdById) ?? null) : null,
-				createdAt: message.createdAt.toISOString(),
-				updatedAt: message.updatedAt?.toISOString() ?? null,
-				deletedAt: message.deletedAt?.toISOString() ?? null,
-			})),
-		};
+	switch (row.type) {
+		case 'comment.created':
+			return {
+				...base,
+				type: row.type,
+				data: null,
+				messages: messages.map((message) => ({
+					id: String(message.id),
+					// Never send the body of a deleted comment, even if the row still holds one, so a
+					// delete path that only sets `deletedAt` cannot leak it.
+					body: message.deletedAt ? null : message.body,
+					createdBy: message.createdById ? (usersById.get(message.createdById) ?? null) : null,
+					createdAt: message.createdAt.toISOString(),
+					updatedAt: message.updatedAt?.toISOString() ?? null,
+					deletedAt: message.deletedAt?.toISOString() ?? null,
+				})),
+			};
+		case 'review.opened':
+			return {
+				...base,
+				type: row.type,
+				data: parsePayload(workflowReviewOpenedActivityDataSchema, row, logger),
+			};
+		case 'review.changes_requested':
+		case 'review.approved':
+			return {
+				...base,
+				type: row.type,
+				data: parsePayload(workflowReviewDecisionActivityDataSchema, row, logger),
+			};
+		case 'review.version_updated':
+			return {
+				...base,
+				type: row.type,
+				data: parsePayload(workflowReviewVersionUpdatedActivityDataSchema, row, logger),
+			};
+		case 'review.closed':
+			return {
+				...base,
+				type: row.type,
+				data: parsePayload(workflowReviewClosedActivityDataSchema, row, logger),
+			};
+		case 'workflow.published':
+			return { ...base, type: row.type, data: null };
 	}
 
-	return { ...base, type: row.type, data: row.data };
+	// Unreachable per the union; a row whose type is outside it (e.g. after a version
+	// downgrade) must still render as an unknown entry rather than crash the feed.
+	return { ...base, type: row.type, data: null };
 }

packages/frontend/@n8n/i18n/src/locales/en.json

@@ -5485,13 +5485,19 @@
 	"workflowReviews.decision.approved": "Approved",
 	"workflowReviews.detail.tabs.activity": "Activity",
 	"workflowReviews.detail.tabs.changes": "Changes",
+	"workflowReviews.detail.activity.description": "Description",
 	"workflowReviews.detail.activity.noDescription": "No description provided.",
 	"workflowReviews.detail.activity.listLabel": "Review activity",
-	"workflowReviews.detail.activity.empty.heading": "No activity yet",
-	"workflowReviews.detail.activity.empty.description": "Comments and review activity will appear here.",
 	"workflowReviews.detail.activity.comment.deleted": "This comment was deleted.",
 	"workflowReviews.detail.activity.unknownEntry": "This activity entry can't be displayed.",
 	"workflowReviews.detail.activity.unknownAuthor": "Deleted user",
+	"workflowReviews.detail.activity.opened": "Review submitted",
+	"workflowReviews.detail.activity.changesRequested": "Requested changes",
+	"workflowReviews.detail.activity.approved": "Approved submission",
+	"workflowReviews.detail.activity.versionUpdated": "New workflow version submitted",
+	"workflowReviews.detail.activity.closed.archived": "Review closed because the workflow was archived",
+	"workflowReviews.detail.activity.closed.moved": "Review closed because the workflow moved to another project",
+	"workflowReviews.detail.activity.closed.deleted": "Review closed because the workflow was deleted",
 	"workflowReviews.detail.activity.composer.label": "Add a comment",
 	"workflowReviews.detail.activity.composer.placeholder": "Leave a comment...",
 	"workflowReviews.detail.activity.composer.notAllowed": "You don't have permission to comment on this review.",
@@ -5515,7 +5521,11 @@
 	"workflowReviews.changes.firstPublish.sourceEmpty": "This workflow hasn't been published before.",
 	"workflowReviews.detail.notFound.title": "Review not found",
 	"workflowReviews.detail.notFound.body": "This review doesn't exist or you don't have access to it.",
-	"workflowReviews.detail.decision.approve": "Approve",
+	"workflowReviews.detail.decision.trigger": "Review",
+	"workflowReviews.detail.decision.note.label": "Review note",
+	"workflowReviews.detail.decision.note.required": "Add a note to request changes",
+	"workflowReviews.detail.decision.comment": "Comment",
+	"workflowReviews.detail.decision.approveAndPublish": "Approve and publish",
 	"workflowReviews.detail.decision.requestChanges": "Request changes",
 	"workflowReviews.detail.decision.ineligible.author": "You contributed a version to this review.",
 	"workflowReviews.emptyState.body.open": "There are no open reviews right now.",

packages/frontend/editor-ui/src/features/workflow-reviews/components/WorkflowReviewActivityFeed.test.ts

@@ -8,10 +8,11 @@ import { useReviewActivityStore } from '../reviewActivity.store';
 import WorkflowReviewActivityFeed from './WorkflowReviewActivityFeed.vue';
 
 /**
- * Scroll anchoring and the first-load scroll to bottom are deliberately untested:
- * jsdom has no layout, so `scrollHeight` is always 0 and `getBoundingClientRect()`
- * all zeroes. A test would have to stub those getters and then re-compute the
- * production arithmetic from its own constants. Do not backfill a fake one.
+ * The scroll anchoring arithmetic is deliberately untested: jsdom has no layout, so
+ * `getBoundingClientRect()` is all zeroes and a test would have to stub those getters and then
+ * re-compute the production arithmetic from its own constants. Do not backfill a fake one.
+ * Which of the two behaviours a changed list picks — hold the reading position, or jump to the
+ * newest entry — is testable, and is what the reload cases below assert.
  */
 const observer = vi.hoisted(() => ({
 	observe: vi.fn(),
@@ -27,7 +28,9 @@ vi.mock('@/app/composables/useIntersectionObserver', () => ({
 
 const renderComponent = createComponentRenderer(WorkflowReviewActivityFeed);
 
-function makeEntry(overrides: Partial<WorkflowReviewActivityEntry> = {}) {
+type CommentEntry = Extract<WorkflowReviewActivityEntry, { type: 'comment.created' }>;
+
+function makeComment(overrides: Partial<CommentEntry> = {}): WorkflowReviewActivityEntry {
 	return {
 		id: '1',
 		type: 'comment.created',
@@ -46,9 +49,17 @@ function makeEntry(overrides: Partial<WorkflowReviewActivityEntry> = {}) {
 			},
 		],
 		...overrides,
-	} as WorkflowReviewActivityEntry;
+	};
 }
 
+/** Every non-comment entry shares these fields; `type` and `data` are per row. */
+const systemEntry = {
+	id: '1',
+	typeVersion: 1,
+	createdBy: null,
+	createdAt: '2024-01-01T00:00:00.000Z',
+};
+
 describe('WorkflowReviewActivityFeed', () => {
 	let store: ReturnType<typeof mockedStore<typeof useReviewActivityStore>>;
 
@@ -66,7 +77,7 @@ describe('WorkflowReviewActivityFeed', () => {
 	});
 
 	it('shows one entry per activity in the feed', async () => {
-		store.entries = Array.from({ length: 3 }, (_, index) => makeEntry({ id: String(index) }));
+		store.entries = Array.from({ length: 3 }, (_, index) => makeComment({ id: String(index) }));
 
 		const { queryAllByTestId } = renderComponent();
 		await nextTick();
@@ -83,12 +94,6 @@ describe('WorkflowReviewActivityFeed', () => {
 		expect(queryByTestId('workflow-review-activity-empty')).not.toBeInTheDocument();
 	});
 
-	it('tells the viewer there is no activity yet', () => {
-		const { getByTestId } = renderComponent();
-
-		expect(getByTestId('workflow-review-activity-empty')).toHaveTextContent('No activity yet');
-	});
-
 	it('shows an error row when the first page failed', () => {
 		store.error = new Error('boom');
 
@@ -112,7 +117,7 @@ describe('WorkflowReviewActivityFeed', () => {
 
 	it('still reaches the earlier activity after posting onto a feed that failed to load', () => {
 		store.error = new Error('boom');
-		store.entries = [makeEntry()];
+		store.entries = [makeComment()];
 
 		const { getByTestId } = renderComponent();
 		getByTestId('workflow-review-activity-load-more-retry').click();
@@ -122,7 +127,7 @@ describe('WorkflowReviewActivityFeed', () => {
 	});
 
 	it('shows progress while refetching a feed that already has entries', async () => {
-		store.entries = [makeEntry()];
+		store.entries = [makeComment()];
 		store.loading = true;
 
 		const { container } = renderComponent();
@@ -132,7 +137,7 @@ describe('WorkflowReviewActivityFeed', () => {
 	});
 
 	it('keeps a loaded feed and offers a retry when load-more failed', async () => {
-		store.entries = [makeEntry()];
+		store.entries = [makeComment()];
 		store.hasMore = true;
 		// A load-more failure always has a cursor to resume from.
 		store.nextCursor = 'cursor-1';
@@ -161,7 +166,7 @@ describe('WorkflowReviewActivityFeed', () => {
 	});
 
 	it('shows a placeholder for an activity type this frontend does not know', async () => {
-		store.entries = [makeEntry({ typeVersion: 2 })];
+		store.entries = [makeComment({ typeVersion: 2 })];
 
 		const { getByTestId } = renderComponent();
 		await nextTick();
@@ -172,7 +177,7 @@ describe('WorkflowReviewActivityFeed', () => {
 	});
 
 	it('loads older entries when the viewer scrolls to the top of the feed', async () => {
-		store.entries = [makeEntry()];
+		store.entries = [makeComment()];
 		store.hasMore = true;
 
 		const { getByTestId } = renderComponent();
@@ -189,7 +194,7 @@ describe('WorkflowReviewActivityFeed', () => {
 	});
 
 	it('stops loading once the oldest entry is on screen', async () => {
-		store.entries = [makeEntry()];
+		store.entries = [makeComment()];
 		store.hasMore = false;
 
 		renderComponent();
@@ -199,8 +204,47 @@ describe('WorkflowReviewActivityFeed', () => {
 		expect(observer.observe).not.toHaveBeenCalled();
 	});
 
+	/**
+	 * A reload keeps only the entries newer than the page it fetched, so its first entry
+	 * differs without anything having been prepended. `scrollHeight` is stubbed because jsdom
+	 * has no layout; the assertion is on the feed landing at the bottom, not on any arithmetic.
+	 */
+	async function reloadFeedAndReturnContainer(getByTestId: (id: string) => HTMLElement) {
+		const container = getByTestId('workflow-review-activity-feed');
+		Object.defineProperty(container, 'scrollHeight', { value: 500, configurable: true });
+		container.scrollTop = 120;
+
+		store.entries = [makeComment({ id: '2' }), makeComment({ id: '3' })];
+		await nextTick();
+
+		return container;
+	}
+
+	it('shows the entry a decision just added to a feed the viewer had paged back through', async () => {
+		store.entries = [makeComment({ id: '1' }), makeComment({ id: '2' })];
+
+		const { getByTestId } = renderComponent();
+		await nextTick();
+
+		expect((await reloadFeedAndReturnContainer(getByTestId)).scrollTop).toBe(500);
+	});
+
+	// `loadMore` returns early while another page is in flight, leaving behind the anchor it
+	// captured — which by the next reload points at an entry no longer on screen.
+	it('shows the newest entry after a reload that followed a load-more that never ran', async () => {
+		store.entries = [makeComment({ id: '1' }), makeComment({ id: '2' })];
+		store.hasMore = true;
+		store.loadingMore = true;
+
+		const { getByTestId } = renderComponent();
+		await nextTick();
+		observer.onIntersect();
+
+		expect((await reloadFeedAndReturnContainer(getByTestId)).scrollTop).toBe(500);
+	});
+
 	it('does not request the same older page twice', async () => {
-		store.entries = [makeEntry()];
+		store.entries = [makeComment()];
 		store.hasMore = true;
 		store.loadingMore = true;
 
@@ -210,4 +254,214 @@ describe('WorkflowReviewActivityFeed', () => {
 
 		expect(observer.observe).not.toHaveBeenCalled();
 	});
+
+	describe('review lifecycle entries', () => {
+		it.each<[string, WorkflowReviewActivityEntry, { testId: string; text: string; note?: string }]>(
+			[
+				[
+					'the review being submitted',
+					{
+						...systemEntry,
+						type: 'review.opened',
+						data: { workflowVersions: [{ workflowId: 'wf-1', workflowVersionId: 'version-1' }] },
+					},
+					{ testId: 'workflow-review-activity-opened', text: 'Review submitted' },
+				],
+				[
+					'requested changes with the note the reviewer left',
+					{
+						...systemEntry,
+						type: 'review.changes_requested',
+						data: {
+							workflowVersions: [{ workflowId: 'wf-1', workflowVersionId: 'version-1' }],
+							note: 'Please add retries',
+						},
+					},
+					{
+						testId: 'workflow-review-activity-changes-requested',
+						text: 'Requested changes',
+						note: 'Please add retries',
+					},
+				],
+				[
+					'an approval with the note the reviewer left',
+					{
+						...systemEntry,
+						type: 'review.approved',
+						data: {
+							workflowVersions: [{ workflowId: 'wf-1', workflowVersionId: 'version-1' }],
+							note: 'Ship it',
+						},
+					},
+					{
+						testId: 'workflow-review-activity-approved',
+						text: 'Approved submission',
+						note: 'Ship it',
+					},
+				],
+				[
+					'an approval given without a note',
+					{
+						...systemEntry,
+						type: 'review.approved',
+						data: {
+							workflowVersions: [{ workflowId: 'wf-1', workflowVersionId: 'version-1' }],
+							note: null,
+						},
+					},
+					{ testId: 'workflow-review-activity-approved', text: 'Approved submission' },
+				],
+				[
+					'an approval whose stored payload can no longer be read',
+					{ ...systemEntry, type: 'review.approved', data: null },
+					{ testId: 'workflow-review-activity-approved', text: 'Approved submission' },
+				],
+				[
+					'the reviewed version being updated',
+					{
+						...systemEntry,
+						type: 'review.version_updated',
+						data: {
+							workflowId: 'wf-1',
+							fromWorkflowVersionId: 'version-1',
+							toWorkflowVersionId: 'version-2',
+						},
+					},
+					{
+						testId: 'workflow-review-activity-version-updated',
+						text: 'New workflow version submitted',
+					},
+				],
+				[
+					'a review closed by archiving its workflow',
+					{ ...systemEntry, type: 'review.closed', data: { reason: 'workflow-archived' } },
+					{
+						testId: 'workflow-review-activity-closed',
+						text: 'Review closed because the workflow was archived',
+					},
+				],
+				[
+					'a review closed by moving its workflow',
+					{ ...systemEntry, type: 'review.closed', data: { reason: 'workflow-moved' } },
+					{
+						testId: 'workflow-review-activity-closed',
+						text: 'Review closed because the workflow moved to another project',
+					},
+				],
+				[
+					'a review closed by deleting its workflow',
+					{ ...systemEntry, type: 'review.closed', data: { reason: 'workflow-deleted' } },
+					{
+						testId: 'workflow-review-activity-closed',
+						text: 'Review closed because the workflow was deleted',
+					},
+				],
+			],
+		)('shows %s', async (_label, entry, expected) => {
+			store.entries = [entry];
+
+			const { getByTestId, queryByTestId } = renderComponent();
+			await nextTick();
+
+			expect(getByTestId(expected.testId)).toHaveTextContent(expected.text);
+			if (expected.note) {
+				expect(getByTestId('workflow-review-activity-note')).toHaveTextContent(expected.note);
+			} else {
+				expect(queryByTestId('workflow-review-activity-note')).not.toBeInTheDocument();
+			}
+		});
+
+		// The only entry whose payload the sentence cannot do without: the close reason *is* the
+		// sentence, so there is nothing left to render.
+		it('shows a placeholder for a closed review that no longer says why', async () => {
+			store.entries = [{ ...systemEntry, type: 'review.closed', data: null }];
+
+			const { getByTestId, queryByTestId } = renderComponent();
+			await nextTick();
+
+			expect(getByTestId('workflow-review-activity-unknown')).toHaveTextContent(
+				"This activity entry can't be displayed.",
+			);
+			expect(queryByTestId('workflow-review-activity-closed')).not.toBeInTheDocument();
+		});
+
+		// Losing the note must not cost the actor and the verb too, which are known from the type.
+		it('keeps a change request whose note can no longer be read', async () => {
+			store.entries = [{ ...systemEntry, type: 'review.changes_requested', data: null }];
+
+			const { getByTestId, queryByTestId } = renderComponent();
+			await nextTick();
+
+			expect(getByTestId('workflow-review-activity-changes-requested')).toHaveTextContent(
+				'Requested changes',
+			);
+			expect(queryByTestId('workflow-review-activity-note')).not.toBeInTheDocument();
+			expect(queryByTestId('workflow-review-activity-unknown')).not.toBeInTheDocument();
+		});
+
+		// No component is registered for this type yet, which is why the registry stays partial.
+		it('shows a placeholder for an entry type no renderer covers yet', async () => {
+			store.entries = [{ ...systemEntry, type: 'workflow.published', data: null }];
+
+			const { getByTestId } = renderComponent();
+			await nextTick();
+
+			expect(getByTestId('workflow-review-activity-unknown')).toBeInTheDocument();
+		});
+
+		it('names the reviewer who acted', async () => {
+			store.entries = [
+				{
+					...systemEntry,
+					type: 'review.approved',
+					data: {
+						workflowVersions: [{ workflowId: 'wf-1', workflowVersionId: 'version-1' }],
+						note: null,
+					},
+					createdBy: {
+						id: 'user-1',
+						email: 'ada@example.com',
+						firstName: 'Ada',
+						lastName: 'Lovelace',
+					},
+				},
+			];
+
+			const { getByTestId } = renderComponent();
+			await nextTick();
+
+			expect(getByTestId('workflow-review-activity-actor')).toHaveTextContent('Ada Lovelace');
+		});
+
+		// The actor is missing for two different reasons, and a deleted approver must not
+		// read as a system event.
+		it('names a deleted reviewer as the actor of their decision', async () => {
+			store.entries = [
+				{
+					...systemEntry,
+					type: 'review.approved',
+					data: {
+						workflowVersions: [{ workflowId: 'wf-1', workflowVersionId: 'version-1' }],
+						note: null,
+					},
+				},
+			];
+
+			const { getByTestId } = renderComponent();
+			await nextTick();
+
+			expect(getByTestId('workflow-review-activity-actor')).toHaveTextContent('Deleted user');
+		});
+
+		it('shows no actor at all for a review the system closed', async () => {
+			store.entries = [
+				{ ...systemEntry, type: 'review.closed', data: { reason: 'workflow-moved' } },
+			];
+
+			const { queryByTestId } = renderComponent();
+			await nextTick();
+
+			expect(queryByTestId('workflow-review-activity-actor')).not.toBeInTheDocument();
+		});
+	});
 });

packages/frontend/editor-ui/src/features/workflow-reviews/components/WorkflowReviewActivityFeed.vue

@@ -1,5 +1,5 @@
 <script setup lang="ts">
-import { N8nButton, N8nEmptyState, N8nLoading, N8nText } from '@n8n/design-system';
+import { N8nButton, N8nLoading, N8nText } from '@n8n/design-system';
 import { useI18n } from '@n8n/i18n';
 import { storeToRefs } from 'pinia';
 import { onMounted, ref, watch } from 'vue';
@@ -52,6 +52,12 @@ watch(
 watch(
 	entries,
 	(next, previous) => {
+		// Good for this update only, and only while its element is still in the list: `loadMore`
+		// bails out on a cursor it has already spent, leaving an anchor behind, and a refetch
+		// drops the entries an older anchor points at.
+		const anchor = prependAnchor?.element.isConnected === true ? prependAnchor : null;
+		prependAnchor = null;
+
 		if (next.length === 0) return;
 		if (!previous || previous.length === 0) {
 			scrollToBottom();
@@ -59,11 +65,13 @@ watch(
 		}
 		if (next[0]?.id !== previous[0]?.id) {
 			const container = scrollContainer.value;
-			if (container && prependAnchor) {
-				container.scrollTop +=
-					prependAnchor.element.getBoundingClientRect().top - prependAnchor.top;
+			// With no anchor the list was replaced rather than prepended to — a refetch keeps
+			// only what is newer than the page it got — so the newest entry is what to show.
+			if (!container || !anchor) {
+				scrollToBottom();
+				return;
 			}
-			prependAnchor = null;
+			container.scrollTop += anchor.element.getBoundingClientRect().top - anchor.top;
 			return;
 		}
 		if (next.at(-1)?.id !== previous.at(-1)?.id) scrollToBottom();
@@ -91,6 +99,9 @@ onMounted(() => {
 
 <template>
 	<div ref="scrollContainer" :class="$style.feed" data-test-id="workflow-review-activity-feed">
+		<div v-if="$slots.header" :class="$style.header">
+			<slot name="header" />
+		</div>
 		<N8nLoading v-if="loading && entries.length === 0" :loading="true" :rows="3" />
 		<div
 			v-else-if="error && entries.length === 0"
@@ -109,12 +120,6 @@ onMounted(() => {
 				{{ i18n.baseText('generic.retry') }}
 			</N8nButton>
 		</div>
-		<div v-else-if="entries.length === 0" data-test-id="workflow-review-activity-empty">
-			<N8nEmptyState
-				:heading="i18n.baseText('workflowReviews.detail.activity.empty.heading')"
-				:description="i18n.baseText('workflowReviews.detail.activity.empty.description')"
-			/>
-		</div>
 		<template v-else>
 			<div
 				v-if="initialScrollApplied && hasMore && !error"
@@ -138,6 +143,9 @@ onMounted(() => {
 					{{ i18n.baseText('generic.retry') }}
 				</N8nButton>
 			</div>
+			<N8nText bold color="text-light" size="medium" :class="$style.header">{{
+				i18n.baseText('workflowReviews.detail.tabs.activity')
+			}}</N8nText>
 			<div
 				ref="list"
 				role="list"
@@ -148,6 +156,7 @@ onMounted(() => {
 					v-for="entry in entries"
 					:key="entry.id"
 					role="listitem"
+					:class="$style.item"
 					data-test-id="workflow-review-activity-entry"
 				>
 					<component :is="resolveActivityComponent(entry)" :entry="entry" />
@@ -164,7 +173,9 @@ onMounted(() => {
 	flex: 1;
 	min-height: 0;
 	overflow: auto;
-	padding-block: var(--spacing--2xs) var(--spacing--sm);
+	padding-block: var(--spacing--5xs) var(--spacing--sm);
+	/* Keeps the cards off the scrollbar that appears here when feed overflows */
+	padding-inline-end: var(--spacing--2xs);
 }
 
 /* The detail body stacks and takes over scrolling here, so the feed must bound itself or its
@@ -175,10 +186,44 @@ onMounted(() => {
 	}
 }
 
+/* Same inset the list gives its entries, so a card here starts on the avatar column. */
+.header {
+	padding-inline: var(--spacing--sm);
+	padding-bottom: var(--spacing--sm);
+}
+
 .list {
+	/* The rail below spans this gap, so both read it from here. */
+	--review-activity--gap: var(--spacing--md);
+	/* Every entry leads with an `xxsmall` avatar (`N8nAvatar/avatarSizes.ts`), and a boxed
+		entry's negative margin and padding cancel out, so all avatars share this column. The
+		rail below is centred on it. */
+	--review-activity--avatar-size: 16px;
+	--review-activity--line-height: 20px;
+
 	display: flex;
 	flex-direction: column;
-	gap: var(--spacing--md);
+	gap: var(--review-activity--gap);
+	/* Entries sit inset; a boxed entry cancels this to reach the panel edge. */
+	padding-inline: var(--spacing--sm);
+}
+
+.item {
+	position: relative;
+}
+
+/* Threads the entries into one timeline. Drawn in the gap above each entry rather than
+	inside it, because the line belongs to the space between two entries and only the list
+	knows that gap. The first entry has nothing above it to join. */
+.item:not(:first-child)::before {
+	content: '';
+	position: absolute;
+	/* Floats clear of both neighbours instead of butting into them, as the design does. The
+		row is taller than its avatar, so the same inset reads as a wider gap at the top. */
+	bottom: calc(100% + var(--spacing--5xs));
+	height: calc(var(--review-activity--gap) - 2 * var(--spacing--5xs));
+	left: calc(var(--review-activity--avatar-size) / 2);
+	border-left: var(--border);
 }
 
 .errorRow {

packages/frontend/editor-ui/src/features/workflow-reviews/components/WorkflowReviewCommentComposer.test.ts

@@ -1,4 +1,4 @@
-import { WORKFLOW_REVIEW_COMMENT_MAX_LENGTH } from '@n8n/api-types';
+import { WORKFLOW_REVIEW_TEXT_MAX_LENGTH } from '@n8n/api-types';
 import { createTestingPinia } from '@pinia/testing';
 import userEvent from '@testing-library/user-event';
 import { waitFor } from '@testing-library/vue';
@@ -29,7 +29,7 @@ describe('WorkflowReviewCommentComposer', () => {
 		store = mockedStore(useReviewActivityStore);
 		store.posting = false;
 		store.draft = '';
-		store.postComment.mockResolvedValue(undefined);
+		store.postComment.mockResolvedValue(true);
 	});
 
 	it('keeps a half-typed comment when the composer is unmounted and shown again', async () => {
@@ -70,12 +70,12 @@ describe('WorkflowReviewCommentComposer', () => {
 		const textarea = getByRole('textbox');
 
 		textarea.focus();
-		await userEvent.paste('x'.repeat(WORKFLOW_REVIEW_COMMENT_MAX_LENGTH));
+		await userEvent.paste('x'.repeat(WORKFLOW_REVIEW_TEXT_MAX_LENGTH));
 		await waitFor(() => expect(getByTestId('send-message-button')).not.toBeDisabled());
 		// Shift+Enter inserts the newline itself, so `maxlength` on the textarea does not stop it
 		await userEvent.keyboard('{Shift>}{Enter}{/Shift}');
 
-		expect(textarea).toHaveValue(`${'x'.repeat(WORKFLOW_REVIEW_COMMENT_MAX_LENGTH)}\n`);
+		expect(textarea).toHaveValue(`${'x'.repeat(WORKFLOW_REVIEW_TEXT_MAX_LENGTH)}\n`);
 		await waitFor(() => expect(getByTestId('send-message-button')).toBeDisabled());
 	});
 
@@ -94,8 +94,8 @@ describe('WorkflowReviewCommentComposer', () => {
 		let resolvePost!: () => void;
 		store.postComment.mockImplementation(
 			async () =>
-				await new Promise<void>((resolve) => {
-					resolvePost = resolve;
+				await new Promise<boolean>((resolve) => {
+					resolvePost = () => resolve(true);
 				}),
 		);
 		const { getByTestId, getByRole } = renderComponent({ props: { canComment: true } });
@@ -112,6 +112,27 @@ describe('WorkflowReviewCommentComposer', () => {
 		expect(textarea).toHaveValue('Nice work and the next one');
 	});
 
+	it('keeps the draft when the post it was waiting on belonged to another review', async () => {
+		let resolvePost!: () => void;
+		store.postComment.mockImplementation(
+			async () =>
+				await new Promise<boolean>((resolve) => {
+					// Stale: the viewer moved to another review while this was in flight.
+					resolvePost = () => resolve(false);
+				}),
+		);
+		const { getByTestId, getByRole } = renderComponent({ props: { canComment: true } });
+		const textarea = getByRole('textbox');
+
+		await userEvent.type(textarea, 'Nice work');
+		await userEvent.click(getByTestId('send-message-button'));
+		resolvePost();
+		await new Promise(setImmediate);
+
+		// Identical text, so only the stale result can keep it from being cleared.
+		expect(textarea).toHaveValue('Nice work');
+	});
+
 	it('keeps the draft and surfaces an error when posting fails', async () => {
 		store.postComment.mockRejectedValue(new Error('boom'));
 		const { getByTestId, getByRole } = renderComponent({ props: { canComment: true } });

packages/frontend/editor-ui/src/features/workflow-reviews/components/WorkflowReviewCommentComposer.vue

@@ -1,5 +1,5 @@
 <script setup lang="ts">
-import { WORKFLOW_REVIEW_COMMENT_MAX_LENGTH } from '@n8n/api-types';
+import { WORKFLOW_REVIEW_TEXT_MAX_LENGTH } from '@n8n/api-types';
 import { N8nChatInput } from '@n8n/design-system';
 import { useI18n } from '@n8n/i18n';
 import { useToast } from '@n8n/composables/useToast';
@@ -21,7 +21,7 @@ const submitDisabled = computed(
 	() =>
 		posting.value ||
 		draft.value.trim().length === 0 ||
-		draft.value.length > WORKFLOW_REVIEW_COMMENT_MAX_LENGTH ||
+		draft.value.length > WORKFLOW_REVIEW_TEXT_MAX_LENGTH ||
 		!props.canComment,
 );
 
@@ -31,9 +31,11 @@ async function onSubmit() {
 	if (!body) return;
 
 	try {
-		await store.postComment(body);
+		// A stale post belongs to the review the viewer left, so its completion must not
+		// touch the draft they are typing now, even if the two strings happen to match.
+		const posted = await store.postComment(body);
 		// Don't clear text typed while the post was in flight.
-		if (draft.value === submitted) draft.value = '';
+		if (posted && draft.value === submitted) draft.value = '';
 	} catch (error) {
 		showError(error, i18n.baseText('workflowReviews.detail.activity.error.post'));
 	}
@@ -49,7 +51,7 @@ async function onSubmit() {
 		</span>
 		<N8nChatInput
 			v-model="draft"
-			:max-length="WORKFLOW_REVIEW_COMMENT_MAX_LENGTH"
+			:max-length="WORKFLOW_REVIEW_TEXT_MAX_LENGTH"
 			:placeholder="i18n.baseText('workflowReviews.detail.activity.composer.placeholder')"
 			refocus-after-send
 			:disabled="!canComment"
@@ -65,10 +67,8 @@ async function onSubmit() {
 .composer {
 	display: block;
 	flex-shrink: 0;
-	border-top: var(--border);
-	/* The panel around this clips overflow, and the input draws its focus ring outside its own
-	   box, so without clearance on these three sides the ring is cut off. */
-	padding: var(--spacing--sm) var(--spacing--3xs) var(--spacing--3xs);
+	/* Prevent focus-ring clipping and vertically align with feed comments */
+	padding: var(--spacing--lg) var(--spacing--2xs) var(--spacing--5xs) var(--spacing--5xs);
 }
 
 .srOnly {

packages/frontend/editor-ui/src/features/workflow-reviews/components/WorkflowReviewDecisionPopover.test.ts

@@ -0,0 +1,272 @@
+import { useToast } from '@n8n/composables/useToast';
+import { createTestingPinia } from '@pinia/testing';
+import userEvent from '@testing-library/user-event';
+import { createComponentRenderer } from '@/__tests__/render';
+import { mockedStore, waitAllPromises } from '@/__tests__/utils';
+
+import { useReviewActivityStore } from '../reviewActivity.store';
+import WorkflowReviewDecisionPopover from './WorkflowReviewDecisionPopover.vue';
+
+vi.mock('@n8n/composables/useToast', () => ({
+	useToast: vi.fn(),
+}));
+
+/**
+ * Reka UI popovers do not open in jsdom, so the four primitives N8nPopover imports are
+ * stubbed to render inline. Copied rather than imported: the design system keeps its own
+ * set private. Reka's trigger machinery goes with them, hence the open button standing in
+ * for it — everything after that is the component's own `v-model:open`.
+ */
+const rekaStubs = {
+	PopoverRoot: {
+		props: ['open'],
+		emits: ['update:open'],
+		template: `
+			<div :data-open="open" data-test-id="popover-root">
+				<button data-test-id="open-popover" @click="$emit('update:open', true)" />
+				<slot />
+			</div>`,
+	},
+	PopoverTrigger: { template: '<div><slot /></div>' },
+	PopoverPortal: { template: '<div><slot /></div>' },
+	PopoverContent: { template: '<div><slot /></div>' },
+};
+
+// The real tooltip renders its content in a portal on hover, which jsdom cannot exercise;
+// expose the bindings as attributes instead.
+const tooltipStub = {
+	N8nTooltip: {
+		props: ['disabled', 'content'],
+		template:
+			'<div data-test-id="decision-tooltip" :data-disabled="disabled" :data-content="content"><slot /></div>',
+	},
+};
+
+const renderComponent = createComponentRenderer(WorkflowReviewDecisionPopover, {
+	global: { stubs: { ...rekaStubs, ...tooltipStub } },
+	props: {
+		deciding: false,
+		viewerCanDecide: true,
+		viewerCanComment: true,
+		ineligibilityHint: '',
+	},
+});
+
+function tooltipOf(button: HTMLElement) {
+	const tooltip = button.closest('[data-test-id="decision-tooltip"]');
+	if (!tooltip) throw new Error('button is not wrapped in a tooltip');
+	return tooltip;
+}
+
+const showError = vi.fn();
+
+describe('WorkflowReviewDecisionPopover', () => {
+	let store: ReturnType<typeof mockedStore<typeof useReviewActivityStore>>;
+
+	beforeEach(() => {
+		createTestingPinia();
+		showError.mockReset();
+		vi.mocked(useToast).mockReturnValue({ showError } as unknown as ReturnType<typeof useToast>);
+		store = mockedStore(useReviewActivityStore);
+		store.decisionNote = '';
+		store.posting = false;
+		store.postComment.mockResolvedValue(true);
+	});
+
+	it('needs a note before the reviewer can request changes or comment', async () => {
+		const { getByTestId } = renderComponent();
+
+		const requestChanges = getByTestId('workflow-review-decision-request-changes-button');
+		expect(requestChanges).toBeDisabled();
+		expect(getByTestId('workflow-review-decision-comment-button')).toBeDisabled();
+		expect(tooltipOf(requestChanges)).toHaveAttribute(
+			'data-content',
+			'Add a note to request changes',
+		);
+		expect(tooltipOf(requestChanges)).toHaveAttribute('data-disabled', 'false');
+
+		await userEvent.type(getByTestId('workflow-review-decision-note'), 'Please add retries');
+
+		expect(requestChanges).not.toBeDisabled();
+		expect(getByTestId('workflow-review-decision-comment-button')).not.toBeDisabled();
+		expect(tooltipOf(requestChanges)).toHaveAttribute('data-disabled', 'true');
+	});
+
+	it.each<[string, { viewerCanComment?: boolean; deciding?: boolean }, boolean]>([
+		['a viewer who is not allowed to comment', { viewerCanComment: false }, false],
+		['a comment that is already being posted', {}, true],
+		['a reviewer whose decision is in flight', { deciding: true }, false],
+	])('offers no Comment button to %s, note or not', async (_label, props, posting) => {
+		store.posting = posting;
+
+		const { getByTestId } = renderComponent({ props });
+
+		await userEvent.type(getByTestId('workflow-review-decision-note'), 'Just a thought');
+
+		expect(getByTestId('workflow-review-decision-comment-button')).toBeDisabled();
+	});
+
+	// Posting a comment leaves its text in the box on purpose, so a decision taken meanwhile
+	// would send the very same text a second time as its note.
+	it.each<[string, { posting: boolean; deciding: boolean }]>([
+		['a comment is still being posted', { posting: true, deciding: false }],
+		['a decision is already in flight', { posting: false, deciding: true }],
+	])('offers no decision while %s', async (_label, { posting, deciding }) => {
+		store.posting = posting;
+
+		const { getByTestId } = renderComponent({ props: { deciding } });
+
+		await userEvent.type(getByTestId('workflow-review-decision-note'), 'Please add retries');
+
+		expect(getByTestId('workflow-review-decision-request-changes-button')).toBeDisabled();
+		expect(getByTestId('workflow-review-decision-approve-button')).toBeDisabled();
+	});
+
+	it('submits a change request with the note', async () => {
+		const { getByTestId, emitted } = renderComponent();
+
+		await userEvent.type(getByTestId('workflow-review-decision-note'), 'Please add retries');
+		await userEvent.click(getByTestId('workflow-review-decision-request-changes-button'));
+
+		expect(emitted('decide')).toEqual([
+			[{ decision: 'changes_requested', note: 'Please add retries' }],
+		]);
+	});
+
+	it('submits an approval with the note', async () => {
+		const { getByTestId, emitted } = renderComponent();
+
+		await userEvent.type(getByTestId('workflow-review-decision-note'), 'Ship it');
+		await userEvent.click(getByTestId('workflow-review-decision-approve-button'));
+
+		expect(emitted('decide')).toEqual([[{ decision: 'approved', note: 'Ship it' }]]);
+	});
+
+	it('approves without a note rather than sending an empty one', async () => {
+		const { getByTestId, emitted } = renderComponent();
+
+		await userEvent.click(getByTestId('workflow-review-decision-approve-button'));
+
+		expect(emitted('decide')).toEqual([[{ decision: 'approved' }]]);
+	});
+
+	it('clears its open state once a decision is submitted', async () => {
+		const { getByTestId } = renderComponent();
+
+		await userEvent.click(getByTestId('open-popover'));
+		expect(getByTestId('popover-root')).toHaveAttribute('data-open', 'true');
+
+		await userEvent.click(getByTestId('workflow-review-decision-approve-button'));
+
+		expect(getByTestId('popover-root')).toHaveAttribute('data-open', 'false');
+	});
+
+	it('posts the note as a comment and shows the viewer where it landed', async () => {
+		const { getByTestId, emitted } = renderComponent();
+
+		await userEvent.type(getByTestId('workflow-review-decision-note'), 'Just a thought');
+		await userEvent.click(getByTestId('workflow-review-decision-comment-button'));
+
+		expect(store.postComment).toHaveBeenCalledWith('Just a thought');
+		expect(emitted('comment-posted')).toHaveLength(1);
+		expect(store.clearDecisionNote).toHaveBeenCalledWith('Just a thought');
+		expect(emitted('decide')).toBeUndefined();
+	});
+
+	// The note is only dropped after the post comes back, by which time the viewer may be
+	// typing the next one. Whether that text is theirs to drop is the store's rule.
+	it('drops the note it posted, not whatever the viewer typed meanwhile', async () => {
+		let resolvePost!: (landed: boolean) => void;
+		store.postComment.mockReturnValue(
+			new Promise((resolve) => {
+				resolvePost = resolve;
+			}),
+		);
+
+		const { getByTestId, emitted } = renderComponent();
+
+		await userEvent.type(getByTestId('workflow-review-decision-note'), 'Just a thought');
+		await userEvent.click(getByTestId('workflow-review-decision-comment-button'));
+
+		store.decisionNote = 'and one more thing';
+		resolvePost(true);
+		await waitAllPromises();
+
+		expect(store.clearDecisionNote).toHaveBeenCalledWith('Just a thought');
+		expect(emitted('comment-posted')).toHaveLength(1);
+	});
+
+	// The comment is written either way, but it says nothing about the review now on screen.
+	it('leaves the review the viewer moved to alone when the comment lands late', async () => {
+		store.postComment.mockResolvedValue(false);
+
+		const { getByTestId, emitted } = renderComponent();
+
+		await userEvent.type(getByTestId('workflow-review-decision-note'), 'Just a thought');
+		await userEvent.click(getByTestId('workflow-review-decision-comment-button'));
+
+		expect(store.clearDecisionNote).not.toHaveBeenCalled();
+		expect(emitted('comment-posted')).toBeUndefined();
+	});
+
+	it('keeps the note when the comment could not be posted', async () => {
+		const error = new Error('nope');
+		store.postComment.mockRejectedValueOnce(error);
+
+		const { getByTestId, emitted } = renderComponent();
+
+		await userEvent.type(getByTestId('workflow-review-decision-note'), 'Just a thought');
+		await userEvent.click(getByTestId('workflow-review-decision-comment-button'));
+
+		expect(showError).toHaveBeenCalledWith(error, 'Could not post comment');
+		expect(store.clearDecisionNote).not.toHaveBeenCalled();
+		expect(getByTestId('workflow-review-decision-note')).toHaveValue('Just a thought');
+		expect(emitted('comment-posted')).toBeUndefined();
+	});
+
+	// The draft has to outlive both the popover closing and a switch to another review,
+	// so the input is bound to the store rather than to a local ref.
+	it('shows the note the review already has and writes edits back', async () => {
+		store.decisionNote = 'started earlier';
+
+		const { getByTestId } = renderComponent();
+
+		expect(getByTestId('workflow-review-decision-note')).toHaveValue('started earlier');
+
+		await userEvent.type(getByTestId('workflow-review-decision-note'), ' and continued');
+
+		expect(store.decisionNote).toBe('started earlier and continued');
+	});
+
+	it('says why the viewer cannot decide instead of just blocking them', () => {
+		const { getByTestId } = renderComponent({
+			props: {
+				viewerCanDecide: false,
+				ineligibilityHint: 'You contributed a version to this review.',
+			},
+		});
+
+		const trigger = getByTestId('workflow-review-decision-trigger');
+		expect(trigger).toBeDisabled();
+		expect(tooltipOf(trigger)).toHaveAttribute(
+			'data-content',
+			'You contributed a version to this review.',
+		);
+	});
+
+	// A wrapper around the trigger takes the popover's own trigger props with it: the click,
+	// the expanded state a screen reader announces, and focus coming back on close.
+	it('hands a reviewer who may decide the trigger itself rather than a wrapper', () => {
+		const { getByTestId } = renderComponent();
+
+		expect(
+			getByTestId('workflow-review-decision-trigger').closest('[data-test-id="decision-tooltip"]'),
+		).toBeNull();
+	});
+
+	it('keeps the reviewer out while their decision is in flight', () => {
+		const { getByTestId } = renderComponent({ props: { deciding: true } });
+
+		expect(getByTestId('workflow-review-decision-trigger')).toBeDisabled();
+	});
+});

packages/frontend/editor-ui/src/features/workflow-reviews/components/WorkflowReviewDecisionPopover.vue

@@ -0,0 +1,196 @@
+<script setup lang="ts">
+import { WORKFLOW_REVIEW_TEXT_MAX_LENGTH } from '@n8n/api-types';
+import { useToast } from '@n8n/composables/useToast';
+import { N8nButton, N8nIcon, N8nInput, N8nPopover, N8nTooltip } from '@n8n/design-system';
+import { useI18n } from '@n8n/i18n';
+import { storeToRefs } from 'pinia';
+import { computed, ref, useId } from 'vue';
+
+import { useReviewActivityStore } from '../reviewActivity.store';
+import type { WorkflowReviewDecisionInput } from '../workflowReviews.api';
+
+const props = defineProps<{
+	deciding: boolean;
+	viewerCanDecide: boolean;
+	viewerCanComment: boolean;
+	/** Why the viewer may not decide; empty when they may. */
+	ineligibilityHint: string;
+}>();
+
+const emit = defineEmits<{
+	decide: [input: WorkflowReviewDecisionInput];
+	'comment-posted': [];
+}>();
+
+const i18n = useI18n();
+const { showError } = useToast();
+const store = useReviewActivityStore();
+const { posting, decisionNote } = storeToRefs(store);
+
+const isOpen = ref(false);
+const noteInputId = useId();
+
+const note = computed(() => decisionNote.value.trim());
+
+// Every viewer who may decide may also comment, and a viewer who may not decide cannot open
+// the popover at all, so the last term only guards against those two rules drifting apart.
+const commentDisabled = computed(
+	() => posting.value || props.deciding || note.value.length === 0 || !props.viewerCanComment,
+);
+
+// A comment in flight leaves its text in the box on purpose, so without this the same text
+// goes out twice: once as the comment, once as the decision note.
+const decisionDisabled = computed(() => props.deciding || posting.value);
+
+function submitDecision(input: WorkflowReviewDecisionInput) {
+	isOpen.value = false;
+	emit('decide', input);
+}
+
+function onRequestChanges() {
+	submitDecision({ decision: 'changes_requested', note: note.value });
+}
+
+function onApprove() {
+	// The key is omitted rather than sent empty: the DTO rejects `''`.
+	submitDecision(
+		note.value ? { decision: 'approved', note: note.value } : { decision: 'approved' },
+	);
+}
+
+async function onComment() {
+	const body = note.value;
+
+	isOpen.value = false;
+	try {
+		// A comment that landed after the viewer moved on says nothing about the review they
+		// are reading now, so neither its note nor its tab may be touched.
+		if (!(await store.postComment(body))) return;
+
+		store.clearDecisionNote(body);
+		// The trigger sits outside the tab panel, so a comment posted from the Changes tab
+		// would otherwise land nowhere the viewer can see.
+		emit('comment-posted');
+	} catch (error) {
+		showError(error, i18n.baseText('workflowReviews.detail.activity.error.post'));
+	}
+}
+</script>
+
+<template>
+	<!-- 480px deviates from Figma's 440px on purpose: the three icon+label buttons do not fit
+		in 440px, and N8nPopover sets a fixed width, so the row would spill rather than wrap. -->
+	<N8nPopover
+		v-model:open="isOpen"
+		side="bottom"
+		align="end"
+		width="480px"
+		:enable-scrolling="false"
+		:content-class="$style.popover"
+	>
+		<template #trigger>
+			<!-- Wrapped only while the viewer may not decide: N8nTooltip does not forward attrs
+				on this branch, so the trigger props land on the span — which is what makes the
+				hint fire over a natively disabled button, and this trigger cannot open anything
+				anyway. An enabled trigger has to stay the bare button, or it loses `aria-expanded`
+				and the focus it gets back on close. -->
+			<span v-if="ineligibilityHint">
+				<N8nTooltip :content="ineligibilityHint" :show-after="300">
+					<N8nButton
+						size="small"
+						:disabled="deciding || !viewerCanDecide"
+						data-test-id="workflow-review-decision-trigger"
+					>
+						{{ i18n.baseText('workflowReviews.detail.decision.trigger') }}
+						<N8nIcon icon="chevron-down" />
+					</N8nButton>
+				</N8nTooltip>
+			</span>
+			<N8nButton
+				v-else
+				size="small"
+				:disabled="deciding || !viewerCanDecide"
+				data-test-id="workflow-review-decision-trigger"
+			>
+				{{ i18n.baseText('workflowReviews.detail.decision.trigger') }}
+				<N8nIcon icon="chevron-down" />
+			</N8nButton>
+		</template>
+		<template #content>
+			<div :class="$style.content" data-test-id="workflow-review-decision-popover">
+				<N8nInput
+					:id="noteInputId"
+					:aria-label="i18n.baseText('workflowReviews.detail.decision.note.label')"
+					v-model="decisionNote"
+					type="textarea"
+					:rows="3"
+					:maxlength="WORKFLOW_REVIEW_TEXT_MAX_LENGTH"
+					:placeholder="i18n.baseText('workflowReviews.detail.activity.composer.placeholder')"
+					data-test-id="workflow-review-decision-note"
+				/>
+				<div :class="$style.actions">
+					<N8nButton
+						variant="outline"
+						size="small"
+						:disabled="commentDisabled"
+						data-test-id="workflow-review-decision-comment-button"
+						@click="onComment"
+					>
+						<template #icon>
+							<N8nIcon icon="message-square" />
+						</template>
+						{{ i18n.baseText('workflowReviews.detail.decision.comment') }}
+					</N8nButton>
+					<N8nTooltip
+						:disabled="!!note"
+						:content="i18n.baseText('workflowReviews.detail.decision.note.required')"
+						:show-after="300"
+					>
+						<N8nButton
+							variant="outline"
+							size="small"
+							:disabled="!note || decisionDisabled"
+							data-test-id="workflow-review-decision-request-changes-button"
+							@click="onRequestChanges"
+						>
+							<template #icon>
+								<N8nIcon icon="refresh-cw" />
+							</template>
+							{{ i18n.baseText('workflowReviews.detail.decision.requestChanges') }}
+						</N8nButton>
+					</N8nTooltip>
+					<N8nButton
+						variant="outline"
+						size="small"
+						:disabled="decisionDisabled"
+						data-test-id="workflow-review-decision-approve-button"
+						@click="onApprove"
+					>
+						<template #icon>
+							<N8nIcon icon="check" />
+						</template>
+						{{ i18n.baseText('workflowReviews.detail.decision.approveAndPublish') }}
+					</N8nButton>
+				</div>
+			</div>
+		</template>
+	</N8nPopover>
+</template>
+
+<style lang="scss" module>
+.popover {
+	padding: var(--spacing--xs);
+}
+
+.content {
+	display: flex;
+	flex-direction: column;
+	gap: var(--spacing--xs);
+}
+
+.actions {
+	display: flex;
+	justify-content: flex-end;
+	gap: var(--spacing--2xs);
+}
+</style>

packages/frontend/editor-ui/src/features/workflow-reviews/components/WorkflowReviewDetailMetadata.vue

@@ -38,13 +38,13 @@ const statusSummary = computed(() => {
 	<aside :class="$style.metadata" data-test-id="workflow-review-detail-metadata">
 		<N8nCard :class="$style.card" data-test-id="workflow-review-detail-status-card">
 			<template #header>
-				<N8nText bold color="text-light" size="small">
+				<N8nText bold color="text-light" size="medium">
 					{{ i18n.baseText('workflowReviews.detail.metadata.status') }}
 				</N8nText>
 			</template>
 			<div :class="$style.status">
 				<WorkflowReviewStatusDot :state="review.state" :decision="review.decision" />
-				<N8nText size="small">{{ statusSummary }}</N8nText>
+				<N8nText size="medium">{{ statusSummary }}</N8nText>
 			</div>
 		</N8nCard>
 
@@ -53,7 +53,7 @@ const statusSummary = computed(() => {
 			data-test-id="workflow-review-detail-people-card"
 		>
 			<div :class="$style.section">
-				<N8nText bold color="text-light" size="small">
+				<N8nText bold color="text-light" size="medium">
 					{{ i18n.baseText('workflowReviews.detail.metadata.requestedBy') }}
 				</N8nText>
 				<div v-if="review.requester" :class="$style.person">
@@ -62,12 +62,12 @@ const statusSummary = computed(() => {
 						:last-name="review.requester.lastName"
 						size="xsmall"
 					/>
-					<N8nText size="small">{{ formatUserDisplayName(review.requester) }}</N8nText>
+					<N8nText size="medium">{{ formatUserDisplayName(review.requester) }}</N8nText>
 				</div>
 				<N8nText
 					v-else
 					color="text-light"
-					size="small"
+					size="medium"
 					data-test-id="workflow-review-detail-requester-deleted"
 				>
 					{{ i18n.baseText('workflowReviews.detail.metadata.requesterDeleted') }}
@@ -79,17 +79,17 @@ const statusSummary = computed(() => {
 				:class="$style.section"
 				data-test-id="workflow-review-detail-other-authors"
 			>
-				<N8nText bold color="text-light" size="small">
+				<N8nText bold color="text-light" size="medium">
 					{{ i18n.baseText('workflowReviews.detail.metadata.otherAuthors') }}
 				</N8nText>
 				<div v-for="author in otherAuthors" :key="author.id" :class="$style.person">
 					<N8nAvatar :first-name="author.firstName" :last-name="author.lastName" size="xsmall" />
-					<N8nText size="small">{{ formatUserDisplayName(author) }}</N8nText>
+					<N8nText size="medium">{{ formatUserDisplayName(author) }}</N8nText>
 				</div>
 			</div>
 
 			<div :class="$style.section">
-				<N8nText bold color="text-light" size="small">
+				<N8nText bold color="text-light" size="medium">
 					{{ i18n.baseText('workflowReviews.detail.metadata.reviewers') }}
 				</N8nText>
 				<div v-for="reviewer in review.reviewers" :key="reviewer.id" :class="$style.person">
@@ -98,12 +98,12 @@ const statusSummary = computed(() => {
 						:last-name="reviewer.lastName"
 						size="xsmall"
 					/>
-					<N8nText size="small">{{ formatUserDisplayName(reviewer) }}</N8nText>
+					<N8nText size="medium">{{ formatUserDisplayName(reviewer) }}</N8nText>
 				</div>
 				<N8nText
 					v-if="review.reviewers.length === 0"
 					color="text-light"
-					size="small"
+					size="medium"
 					data-test-id="workflow-review-detail-no-reviewers"
 				>
 					{{ i18n.baseText('workflowReviews.detail.metadata.noReviewers') }}
@@ -117,7 +117,7 @@ const statusSummary = computed(() => {
 			data-test-id="workflow-review-detail-changes-card"
 		>
 			<template #header>
-				<N8nText bold color="text-light" size="small">
+				<N8nText bold color="text-light" size="medium">
 					{{ i18n.baseText('workflowReviews.detail.metadata.workflow') }}
 				</N8nText>
 			</template>
@@ -127,11 +127,11 @@ const statusSummary = computed(() => {
 					:key="workflow.workflowId"
 					:to="{ name: VIEWS.WORKFLOW, params: { workflowId: workflow.workflowId } }"
 					theme="text"
-					size="small"
+					size="medium"
 					:class="$style.workflow"
 					data-test-id="workflow-review-detail-workflow-link"
 				>
-					<N8nIcon icon="workflow" size="small" :class="$style.workflowIcon" />
+					<N8nIcon icon="workflow" size="medium" :class="$style.workflowIcon" />
 					<span :class="$style.workflowName">{{ workflow.workflowName }}</span>
 				</N8nLink>
 			</div>
@@ -146,6 +146,7 @@ const statusSummary = computed(() => {
 	flex-direction: column;
 	gap: var(--spacing--2xs);
 	min-width: 14rem;
+	padding-top: var(--spacing--5xs);
 }
 
 .card {

packages/frontend/editor-ui/src/features/workflow-reviews/components/WorkflowReviewDetailTabs.decisionNote.test.ts

@@ -0,0 +1,92 @@
+import type { WorkflowReviewRequestDetail } from '@n8n/api-types';
+import { createTestingPinia } from '@pinia/testing';
+import userEvent from '@testing-library/user-event';
+import { createComponentRenderer } from '@/__tests__/render';
+
+import { useReviewActivityStore } from '../reviewActivity.store';
+import WorkflowReviewDetailTabs from './WorkflowReviewDetailTabs.vue';
+
+/**
+ * The real decision popover is kept, so this pins the two facts the draft note depends on:
+ * it lives in the store, and its trigger sits outside the tab panel that unmounts.
+ * Reka UI popovers do not open in jsdom, so its primitives render inline instead.
+ */
+const rekaStubs = {
+	PopoverRoot: { template: '<div><slot /></div>' },
+	PopoverTrigger: { template: '<div><slot /></div>' },
+	PopoverPortal: { template: '<div><slot /></div>' },
+	PopoverContent: { template: '<div><slot /></div>' },
+};
+
+const childStubs = {
+	WorkflowReviewActivityFeed: { template: '<div><slot name="header" /></div>' },
+	WorkflowReviewChangesSection: { template: '<div />' },
+	WorkflowReviewCommentComposer: { template: '<div />' },
+	WorkflowReviewDetailMetadata: { template: '<aside />' },
+};
+
+const renderComponent = createComponentRenderer(WorkflowReviewDetailTabs, {
+	global: { stubs: { ...rekaStubs, ...childStubs } },
+});
+
+const review: WorkflowReviewRequestDetail = {
+	id: 'req-1',
+	projectId: 'proj-1',
+	title: 'Needs review',
+	workflowName: 'My workflow',
+	workflowVersionId: null,
+	requester: null,
+	authors: [],
+	reviewers: [],
+	decision: 'pending',
+	state: 'open',
+	createdAt: '2024-01-01T00:00:00.000Z',
+	updatedAt: '2024-01-01T00:00:00.000Z',
+	description: null,
+	workflows: [],
+	viewerCanDecide: true,
+	viewerDecisionIneligibilityReason: null,
+	viewerCanComment: true,
+};
+
+describe('WorkflowReviewDetailTabs decision note', () => {
+	beforeEach(() => {
+		createTestingPinia();
+	});
+
+	it('keeps a half-written decision note across a tab switch', async () => {
+		const { getByTestId, rerender } = renderComponent({
+			props: { review, tab: 'activity', deciding: false },
+		});
+
+		await userEvent.type(getByTestId('workflow-review-decision-note'), 'Half a thought');
+
+		await rerender({ tab: 'changes' });
+		await rerender({ tab: 'activity' });
+
+		expect(getByTestId('workflow-review-decision-note')).toHaveValue('Half a thought');
+	});
+
+	// The two facts that make the note survive: it is typed into the store, and the popover
+	// sits outside the panel the tab switch unmounts. Asserted separately because either one
+	// alone still reads back the note above.
+	it('holds the note in the store rather than in the popover', async () => {
+		const { getByTestId } = renderComponent({
+			props: { review, tab: 'activity', deciding: false },
+		});
+
+		await userEvent.type(getByTestId('workflow-review-decision-note'), 'Half a thought');
+
+		expect(useReviewActivityStore().decisionNote).toBe('Half a thought');
+	});
+
+	it('keeps the decision popover out of the tab panel', () => {
+		const { getByTestId } = renderComponent({
+			props: { review, tab: 'activity', deciding: false },
+		});
+
+		expect(getByTestId('workflow-review-activity-panel')).not.toContainElement(
+			getByTestId('workflow-review-decision-note'),
+		);
+	});
+});

packages/frontend/editor-ui/src/features/workflow-reviews/components/WorkflowReviewDetailTabs.test.ts

@@ -19,7 +19,7 @@ vi.mock('./WorkflowReviewChangesSection.vue', () => ({
 vi.mock('./WorkflowReviewActivityFeed.vue', () => ({
 	default: {
 		name: 'WorkflowReviewActivityFeed',
-		template: '<div data-test-id="workflow-review-activity-feed" />',
+		template: '<div data-test-id="workflow-review-activity-feed"><slot name="header" /></div>',
 	},
 }));
 
@@ -39,36 +39,24 @@ vi.mock('./WorkflowReviewDetailMetadata.vue', () => ({
 	},
 }));
 
-const renderComponent = createComponentRenderer(WorkflowReviewDetailTabs, {
-	global: {
-		stubs: {
-			N8nTooltip: {
-				props: ['disabled', 'content'],
-				template: `
-					<div data-test-id="workflow-review-decision-tooltip" :data-disabled="disabled" :data-content="content">
-						<slot />
-					</div>`,
-			},
-		},
+// The real popover cannot open in jsdom (Reka UI), so expose what this component passes
+// down as attributes and let a button stand in for the comment it reports back.
+vi.mock('./WorkflowReviewDecisionPopover.vue', () => ({
+	default: {
+		name: 'WorkflowReviewDecisionPopover',
+		props: ['deciding', 'viewerCanDecide', 'viewerCanComment', 'ineligibilityHint'],
+		template: `
+			<div
+				data-test-id="workflow-review-decision-popover"
+				:data-can-decide="viewerCanDecide"
+				:data-ineligibility-hint="ineligibilityHint"
+			>
+				<button data-test-id="emit-comment-posted" @click="$emit('comment-posted')" />
+			</div>`,
 	},
-});
+}));
 
-function decisionButtons(getByTestId: (id: string) => HTMLElement) {
-	return [
-		getByTestId('workflow-review-approve-button'),
-		getByTestId('workflow-review-request-changes-button'),
-	];
-}
-
-/**
- * The tab bar renders a tooltip per tab, which the N8nTooltip stub matches too,
- * so walk up from the button rather than querying the test id globally.
- */
-function decisionTooltip(button: HTMLElement) {
-	const tooltip = button.closest('[data-test-id="workflow-review-decision-tooltip"]');
-	if (!tooltip) throw new Error('decision button is not wrapped in a tooltip');
-	return tooltip;
-}
+const renderComponent = createComponentRenderer(WorkflowReviewDetailTabs);
 
 function makeInboxItem(overrides: Partial<WorkflowReviewInboxItem> = {}): WorkflowReviewInboxItem {
 	return {
@@ -152,7 +140,9 @@ describe('WorkflowReviewDetailTabs', () => {
 			expect(getByTestId('workflow-review-no-description')).toBeInTheDocument();
 		});
 
-		it('renders the feed and the composer below the description', () => {
+		// The description has to sit inside the feed's scroll container for the two to scroll
+		// together, and the composer has to stay outside it to keep its place at the bottom.
+		it('scrolls the description with the feed and keeps the composer below both', () => {
 			const { getByTestId } = renderComponent({
 				props: {
 					review: makeDetail({ description: 'Adds retry logic' }),
@@ -161,19 +151,12 @@ describe('WorkflowReviewDetailTabs', () => {
 				},
 			});
 
-			const panel = getByTestId('workflow-review-activity-panel');
-			const order = [
-				'workflow-review-description',
-				'workflow-review-activity-feed',
-				'workflow-review-comment-composer',
-			].map((testId) => {
-				const element = panel.querySelector(`[data-test-id="${testId}"]`);
-				if (!element) throw new Error(`${testId} is not in the activity panel`);
-				return element;
-			});
+			const feed = getByTestId('workflow-review-activity-feed');
+			const description = getByTestId('workflow-review-description');
+			const composer = getByTestId('workflow-review-comment-composer');
 
-			expect(order[0].compareDocumentPosition(order[1])).toBe(Node.DOCUMENT_POSITION_FOLLOWING);
-			expect(order[1].compareDocumentPosition(order[2])).toBe(Node.DOCUMENT_POSITION_FOLLOWING);
+			expect(feed).toContainElement(description);
+			expect(feed.compareDocumentPosition(composer)).toBe(Node.DOCUMENT_POSITION_FOLLOWING);
 		});
 
 		it.each([
@@ -272,18 +255,19 @@ describe('WorkflowReviewDetailTabs', () => {
 	});
 
 	describe('decision actions', () => {
-		it('emits decisions from the action buttons', () => {
+		// The trigger lives outside the tab panel, so a comment posted from the Changes tab
+		// would otherwise succeed with nothing to show for it.
+		it('switches to the activity tab once a comment was posted from the popover', () => {
 			const { getByTestId, emitted } = renderComponent({
-				props: { review: makeDetail(), tab: 'activity', deciding: false },
+				props: { review: makeDetail(), tab: 'changes', deciding: false },
 			});
 
-			getByTestId('workflow-review-approve-button').click();
-			getByTestId('workflow-review-request-changes-button').click();
+			getByTestId('emit-comment-posted').click();
 
-			expect(emitted('decide')).toEqual([['approved'], ['changes_requested']]);
+			expect(emitted('update:tab')).toEqual([['activity']]);
 		});
 
-		it('hides the action buttons for a closed review', () => {
+		it('offers no decision on a closed review', () => {
 			const { queryByTestId } = renderComponent({
 				props: {
 					review: makeDetail({ state: 'closed', decision: 'approved' }),
@@ -292,41 +276,28 @@ describe('WorkflowReviewDetailTabs', () => {
 				},
 			});
 
-			expect(queryByTestId('workflow-review-approve-button')).not.toBeInTheDocument();
-			expect(queryByTestId('workflow-review-request-changes-button')).not.toBeInTheDocument();
+			expect(queryByTestId('workflow-review-decision-popover')).not.toBeInTheDocument();
 		});
 
-		it('hides the action buttons on a review whose detail payload never loaded', () => {
+		it('offers no decision on a review whose detail payload never loaded', () => {
 			const { queryByTestId } = renderComponent({
 				props: { review: makeInboxItem(), tab: 'activity', deciding: false },
 			});
 
-			expect(queryByTestId('workflow-review-approve-button')).not.toBeInTheDocument();
-			expect(queryByTestId('workflow-review-request-changes-button')).not.toBeInTheDocument();
+			expect(queryByTestId('workflow-review-decision-popover')).not.toBeInTheDocument();
 		});
 
-		it('disables the action buttons while deciding', () => {
-			const { getByTestId } = renderComponent({
-				props: { review: makeDetail(), tab: 'activity', deciding: true },
-			});
-
-			expect(getByTestId('workflow-review-approve-button')).toBeDisabled();
-			expect(getByTestId('workflow-review-request-changes-button')).toBeDisabled();
-		});
-
-		// Each button carries its own tooltip, so both must agree.
-		it('keeps the buttons enabled and the tooltips off when the viewer can decide', () => {
+		it('leaves the decision actions open with no hint when the viewer can decide', () => {
 			const { getByTestId } = renderComponent({
 				props: { review: makeDetail(), tab: 'activity', deciding: false },
 			});
 
-			for (const button of decisionButtons(getByTestId)) {
-				expect(button).not.toBeDisabled();
-				expect(decisionTooltip(button)).toHaveAttribute('data-disabled', 'true');
-			}
+			const popover = getByTestId('workflow-review-decision-popover');
+			expect(popover).toHaveAttribute('data-can-decide', 'true');
+			expect(popover).toHaveAttribute('data-ineligibility-hint', '');
 		});
 
-		it('disables the buttons and says why when the viewer contributed a version', () => {
+		it('says the viewer contributed a version when that is why they cannot decide', () => {
 			const { getByTestId } = renderComponent({
 				props: {
 					review: makeDetail({
@@ -338,14 +309,12 @@ describe('WorkflowReviewDetailTabs', () => {
 				},
 			});
 
-			for (const button of decisionButtons(getByTestId)) {
-				expect(button).toBeDisabled();
-				expect(decisionTooltip(button)).toHaveAttribute('data-disabled', 'false');
-				expect(decisionTooltip(button)).toHaveAttribute(
-					'data-content',
-					'You contributed a version to this review.',
-				);
-			}
+			const popover = getByTestId('workflow-review-decision-popover');
+			expect(popover).toHaveAttribute('data-can-decide', 'false');
+			expect(popover).toHaveAttribute(
+				'data-ineligibility-hint',
+				'You contributed a version to this review.',
+			);
 		});
 
 		it('falls back to the generic permission hint for any other reason', () => {
@@ -360,30 +329,12 @@ describe('WorkflowReviewDetailTabs', () => {
 				},
 			});
 
-			for (const button of decisionButtons(getByTestId)) {
-				expect(button).toBeDisabled();
-				expect(decisionTooltip(button)).toHaveAttribute(
-					'data-content',
-					'Missing permissions to perform this action',
-				);
-			}
-		});
-
-		it('does not emit a decision for an ineligible viewer', () => {
-			const { getByTestId, emitted } = renderComponent({
-				props: {
-					review: makeDetail({
-						viewerCanDecide: false,
-						viewerDecisionIneligibilityReason: 'author',
-					}),
-					tab: 'activity',
-					deciding: false,
-				},
-			});
-
-			getByTestId('workflow-review-approve-button').click();
-
-			expect(emitted('decide')).toBeUndefined();
+			const popover = getByTestId('workflow-review-decision-popover');
+			expect(popover).toHaveAttribute('data-can-decide', 'false');
+			expect(popover).toHaveAttribute(
+				'data-ineligibility-hint',
+				'Missing permissions to perform this action',
+			);
 		});
 	});
 });

packages/frontend/editor-ui/src/features/workflow-reviews/components/WorkflowReviewDetailTabs.vue

@@ -1,6 +1,6 @@
 <script setup lang="ts">
 import type { WorkflowReviewInboxItem, WorkflowReviewRequestDetail } from '@n8n/api-types';
-import { N8nButton, N8nCallout, N8nTabs, N8nText, N8nTooltip } from '@n8n/design-system';
+import { N8nCallout, N8nTabs, N8nText } from '@n8n/design-system';
 import { useI18n } from '@n8n/i18n';
 import { computed } from 'vue';
 
@@ -8,6 +8,7 @@ import type { WorkflowReviewDecisionInput } from '../workflowReviews.api';
 import WorkflowReviewActivityFeed from './WorkflowReviewActivityFeed.vue';
 import WorkflowReviewChangesSection from './WorkflowReviewChangesSection.vue';
 import WorkflowReviewCommentComposer from './WorkflowReviewCommentComposer.vue';
+import WorkflowReviewDecisionPopover from './WorkflowReviewDecisionPopover.vue';
 import WorkflowReviewDetailMetadata from './WorkflowReviewDetailMetadata.vue';
 
 export type WorkflowReviewDetailTab = 'activity' | 'changes';
@@ -20,7 +21,7 @@ const props = defineProps<{
 
 const emit = defineEmits<{
 	'update:tab': [tab: WorkflowReviewDetailTab];
-	decide: [decision: WorkflowReviewDecisionInput];
+	decide: [input: WorkflowReviewDecisionInput];
 }>();
 
 const i18n = useI18n();
@@ -67,27 +68,14 @@ const tabOptions = computed(() => [
 			<!-- Gated on `detail`, not `review`: eligibility only arrives with the
 				detail payload, so the list item alone can't say who may decide. -->
 			<div v-if="detail?.state === 'open'" :class="$style.decisionActions">
-				<N8nTooltip :disabled="!ineligibilityHint" :content="ineligibilityHint" :show-after="300">
-					<N8nButton
-						size="small"
-						:disabled="deciding || !viewerCanDecide"
-						data-test-id="workflow-review-approve-button"
-						@click="emit('decide', 'approved')"
-					>
-						{{ i18n.baseText('workflowReviews.detail.decision.approve') }}
-					</N8nButton>
-				</N8nTooltip>
-				<N8nTooltip :disabled="!ineligibilityHint" :content="ineligibilityHint" :show-after="300">
-					<N8nButton
-						size="small"
-						type="secondary"
-						:disabled="deciding || !viewerCanDecide"
-						data-test-id="workflow-review-request-changes-button"
-						@click="emit('decide', 'changes_requested')"
-					>
-						{{ i18n.baseText('workflowReviews.detail.decision.requestChanges') }}
-					</N8nButton>
-				</N8nTooltip>
+				<WorkflowReviewDecisionPopover
+					:deciding="deciding"
+					:viewer-can-decide="viewerCanDecide"
+					:viewer-can-comment="viewerCanComment"
+					:ineligibility-hint="ineligibilityHint"
+					@decide="emit('decide', $event)"
+					@comment-posted="emit('update:tab', 'activity')"
+				/>
 			</div>
 		</div>
 
@@ -97,27 +85,35 @@ const tabOptions = computed(() => [
 				:class="$style.activityPanel"
 				data-test-id="workflow-review-activity-panel"
 			>
-				<div :class="$style.activityHeader">
-					<N8nText
-						v-if="detail?.description"
-						color="text-base"
-						size="medium"
-						:class="$style.description"
-						data-test-id="workflow-review-description"
-					>
-						{{ detail.description }}
-					</N8nText>
-					<N8nText
-						v-else
-						color="text-light"
-						size="medium"
-						data-test-id="workflow-review-no-description"
-					>
-						{{ i18n.baseText('workflowReviews.detail.activity.noDescription') }}
-					</N8nText>
-				</div>
-
-				<WorkflowReviewActivityFeed :key="review.id" />
+				<WorkflowReviewActivityFeed :key="review.id">
+					<template #header>
+						<!-- Carded and labelled so the review's own words are not mistaken for the
+							first entry of the feed below it. -->
+						<div :class="$style.descriptionCard">
+							<N8nText tag="h3" bold color="text-light" size="medium">
+								{{ i18n.baseText('workflowReviews.detail.activity.description') }}
+							</N8nText>
+							<N8nText
+								v-if="detail?.description"
+								color="text-base"
+								size="medium"
+								:class="$style.description"
+								data-test-id="workflow-review-description"
+							>
+								{{ detail.description }}
+							</N8nText>
+							<N8nText
+								v-else
+								color="text-light"
+								size="medium"
+								:class="$style.noDescription"
+								data-test-id="workflow-review-no-description"
+							>
+								{{ i18n.baseText('workflowReviews.detail.activity.noDescription') }}
+							</N8nText>
+						</div>
+					</template>
+				</WorkflowReviewActivityFeed>
 
 				<WorkflowReviewCommentComposer :can-comment="viewerCanComment" />
 			</div>
@@ -162,6 +158,8 @@ const tabOptions = computed(() => [
 </template>
 
 <style module lang="scss">
+@use './activity-card' as *;
+
 .container {
 	display: flex;
 	flex-direction: column;
@@ -198,28 +196,34 @@ const tabOptions = computed(() => [
 	flex: 1;
 	min-height: 0;
 	overflow: hidden;
+	max-width: var(--review-activity--max-width, 48rem);
+	margin-inline-end: auto;
 }
 
-/* Capped so a long description cannot push the composer off screen. */
-.activityHeader {
-	flex-shrink: 0;
-	max-height: 30%;
-	overflow: auto;
-	padding-bottom: var(--spacing--sm);
+.descriptionCard {
+	@include activity-card;
+
+	display: flex;
+	flex-direction: column;
+	gap: var(--spacing--4xs);
 }
 
 .callout {
 	max-width: var(--review-callout--max-width, 34rem);
 }
 
+/* `pre-wrap` alone does not break a pasted URL, which is the one thing that could scroll the
+	card sideways. */
 .description {
 	white-space: pre-wrap;
+	overflow-wrap: anywhere;
+}
+
+.noDescription {
+	font-style: italic;
 }
 
 .decisionActions {
-	display: flex;
-	align-items: center;
-	gap: var(--spacing--2xs);
 	flex-shrink: 0;
 }
 
@@ -237,6 +241,8 @@ const tabOptions = computed(() => [
 	.activityPanel {
 		flex: 0 0 auto;
 		overflow: visible;
+		max-width: none;
+		margin-inline-end: 0;
 	}
 }
 </style>

packages/frontend/editor-ui/src/features/workflow-reviews/components/WorkflowReviewRequestsSidebar.vue

@@ -314,7 +314,7 @@ function onListBackgroundClick() {
 	min-width: 0;
 	max-width: var(--review-sidebar--width);
 	height: 100%;
-	border-right: var(--border-width) solid var(--color--foreground--tint-1);
+	border-right: var(--border-width) solid var(--border-color);
 }
 
 .columnTitle {

packages/frontend/editor-ui/src/features/workflow-reviews/components/_activity-card.scss

@@ -0,0 +1,36 @@
+// Shared by the feed's entry cards and the description card above them.
+
+/* The negative margin cancels the list's inset so every avatar sits in one column, boxed or
+	not; the border comes out of the padding so carded text starts on that same column. */
+@mixin activity-card {
+	margin-inline: calc(-1 * var(--spacing--sm));
+	padding: var(--spacing--xs) calc(var(--spacing--sm) - var(--border-width, 1px));
+	border: var(--border);
+	border-radius: var(--radius--2xs);
+}
+
+@mixin activity-row {
+	display: flex;
+	align-items: flex-start;
+	gap: var(--spacing--2xs);
+}
+
+/* An avatar is shorter than a line of text, so the row's `flex-start` hangs it above the name.
+	Half the leading puts it back on that line — and only that line, so it stays put when a note
+	or a wrapped headline makes the row taller. Every entry kind must include this; without it
+	the avatar reads as sitting too high. */
+@mixin activity-avatar {
+	flex-shrink: 0;
+	margin-block-start: calc(
+		(var(--review-activity--line-height) - var(--review-activity--avatar-size)) / 2
+	);
+}
+
+/* The actor, the sentence and its timestamp read as one line, so every entry kind spaces
+	them by a word's worth. */
+@mixin activity-headline {
+	display: flex;
+	align-items: baseline;
+	gap: var(--spacing--4xs);
+	flex-wrap: wrap;
+}

packages/frontend/editor-ui/src/features/workflow-reviews/components/activity-entries/WorkflowReviewActivityComment.vue

@@ -5,7 +5,7 @@ import { useI18n } from '@n8n/i18n';
 
 import TimeAgo from '@/app/components/TimeAgo.vue';
 
-import { formatUserDisplayName } from '../../workflowReviews.utils';
+import { formatActorName } from '../../workflowReviews.utils';
 
 defineProps<{
 	entry: Extract<WorkflowReviewActivityEntry, { type: 'comment.created' }>;
@@ -14,9 +14,10 @@ defineProps<{
 const i18n = useI18n();
 
 function authorName(message: WorkflowReviewActivityMessage): string {
-	return message.createdBy
-		? formatUserDisplayName(message.createdBy)
-		: i18n.baseText('workflowReviews.detail.activity.unknownAuthor');
+	return formatActorName(
+		message.createdBy,
+		i18n.baseText('workflowReviews.detail.activity.unknownAuthor'),
+	);
 }
 </script>
 
@@ -24,7 +25,8 @@ function authorName(message: WorkflowReviewActivityMessage): string {
 	<div :class="$style.entry">
 		<div v-for="message in entry.messages" :key="message.id" :class="$style.message">
 			<N8nAvatar
-				size="small"
+				size="xxsmall"
+				:class="$style.avatar"
 				:first-name="message.createdBy?.firstName"
 				:last-name="message.createdBy?.lastName"
 			/>
@@ -38,10 +40,11 @@ function authorName(message: WorkflowReviewActivityMessage): string {
 					>
 						{{ authorName(message) }}
 					</N8nText>
-					<N8nText size="xsmall" color="text-light">
+					<N8nText size="small" color="text-light">
 						<time
 							:datetime="message.createdAt"
 							data-test-id="workflow-review-activity-comment-time"
+							:class="$style.timeStamp"
 						>
 							<TimeAgo :date="message.createdAt" />
 						</time>
@@ -71,16 +74,23 @@ function authorName(message: WorkflowReviewActivityMessage): string {
 </template>
 
 <style lang="scss" module>
+@use '../activity-card' as *;
+
+/* A comment is prose, so it gets the same card as a decision that carries a note. */
 .entry {
+	@include activity-card;
+
 	display: flex;
 	flex-direction: column;
 	gap: var(--spacing--xs);
 }
 
 .message {
-	display: flex;
-	align-items: flex-start;
-	gap: var(--spacing--2xs);
+	@include activity-row;
+}
+
+.avatar {
+	@include activity-avatar;
 }
 
 .content {
@@ -91,14 +101,15 @@ function authorName(message: WorkflowReviewActivityMessage): string {
 }
 
 .header {
-	display: flex;
-	align-items: baseline;
-	gap: var(--spacing--2xs);
+	@include activity-headline;
 }
 
-/* Figma asks for 20px on 14px text; no line-height token gives that ratio. */
 .line {
-	line-height: 20px;
+	line-height: var(--review-activity--line-height);
+}
+
+.timeStamp {
+	padding-left: var(--spacing--3xs);
 }
 
 .body {

packages/frontend/editor-ui/src/features/workflow-reviews/components/activity-entries/WorkflowReviewActivityEventEntry.vue

@@ -0,0 +1,203 @@
+<script setup lang="ts">
+import type { WorkflowReviewActivityEntry, WorkflowReviewClosedReason } from '@n8n/api-types';
+import { N8nAvatar, N8nText } from '@n8n/design-system';
+import { type BaseTextKey, useI18n } from '@n8n/i18n';
+import { computed } from 'vue';
+
+import TimeAgo from '@/app/components/TimeAgo.vue';
+
+import { formatActorName } from '../../workflowReviews.utils';
+import WorkflowReviewActivityFallback from './WorkflowReviewActivityFallback.vue';
+
+const props = defineProps<{
+	entry: Extract<
+		WorkflowReviewActivityEntry,
+		{
+			type:
+				| 'review.opened'
+				| 'review.changes_requested'
+				| 'review.approved'
+				| 'review.version_updated'
+				| 'review.closed';
+		}
+	>;
+}>();
+
+const i18n = useI18n();
+
+const closedReasonKeys: Record<WorkflowReviewClosedReason, BaseTextKey> = {
+	'workflow-archived': 'workflowReviews.detail.activity.closed.archived',
+	'workflow-moved': 'workflowReviews.detail.activity.closed.moved',
+	'workflow-deleted': 'workflowReviews.detail.activity.closed.deleted',
+};
+
+/**
+ * Everything that varies by type, so a new type is one case rather than several. `null` only for
+ * `review.closed`, whose sentence *is* the stored reason and so has nothing to fall back to. The
+ * rest say what happened from the type alone, and an unreadable payload costs them only the note.
+ *
+ * `namesActor` follows the type, not a missing `createdBy`: that is also null for a decision
+ * whose author was deleted, which must still read as that person's decision.
+ */
+const content = computed<{
+	text: string;
+	note: string | null;
+	testId: string;
+	namesActor: boolean;
+} | null>(() => {
+	const entry = props.entry;
+	switch (entry.type) {
+		case 'review.opened':
+			return {
+				text: i18n.baseText('workflowReviews.detail.activity.opened'),
+				note: null,
+				testId: 'workflow-review-activity-opened',
+				namesActor: true,
+			};
+		case 'review.changes_requested':
+			return {
+				text: i18n.baseText('workflowReviews.detail.activity.changesRequested'),
+				note: entry.data?.note ?? null,
+				testId: 'workflow-review-activity-changes-requested',
+				namesActor: true,
+			};
+		case 'review.approved':
+			return {
+				text: i18n.baseText('workflowReviews.detail.activity.approved'),
+				note: entry.data?.note ?? null,
+				testId: 'workflow-review-activity-approved',
+				namesActor: true,
+			};
+		case 'review.version_updated':
+			return {
+				text: i18n.baseText('workflowReviews.detail.activity.versionUpdated'),
+				note: null,
+				testId: 'workflow-review-activity-version-updated',
+				namesActor: true,
+			};
+		case 'review.closed':
+			if (!entry.data) return null;
+			return {
+				text: i18n.baseText(closedReasonKeys[entry.data.reason]),
+				note: null,
+				testId: 'workflow-review-activity-closed',
+				namesActor: false,
+			};
+	}
+});
+
+const actorName = computed(() =>
+	formatActorName(
+		props.entry.createdBy,
+		i18n.baseText('workflowReviews.detail.activity.unknownAuthor'),
+	),
+);
+</script>
+
+<template>
+	<div v-if="content" :class="[$style.entry, content.note && $style.boxed]">
+		<N8nAvatar
+			v-if="content.namesActor"
+			size="xxsmall"
+			:class="$style.avatar"
+			:first-name="entry.createdBy?.firstName"
+			:last-name="entry.createdBy?.lastName"
+		/>
+		<!-- Holds the avatar's column open, so an actorless sentence stays in line with its
+			neighbours and the timeline above it does not end over its first word. -->
+		<div v-else :class="$style.avatarSpacer" />
+		<!-- Column beside the avatar, as in the comment entry, so a note lines up with the
+			sentence above it instead of starting back at the avatar. -->
+		<div :class="$style.content">
+			<div :class="$style.headline">
+				<N8nText
+					v-if="content.namesActor"
+					size="medium"
+					color="text-base"
+					:class="$style.line"
+					data-test-id="workflow-review-activity-actor"
+				>
+					{{ actorName }}
+					<!-- Inside the name so it disappears with it, rather than leaving a dangling
+						separator on the actorless entries. -->
+					<span aria-hidden="true" :class="$style.separator">|</span>
+				</N8nText>
+				<N8nText
+					size="medium"
+					color="text-base"
+					:class="$style.line"
+					:data-test-id="content.testId"
+				>
+					{{ content.text }}
+				</N8nText>
+				<N8nText size="small" color="text-light" :class="[$style.line, $style.timeStamp]">
+					<time :datetime="entry.createdAt">
+						<TimeAgo :date="entry.createdAt" />
+					</time>
+				</N8nText>
+			</div>
+			<N8nText
+				v-if="content.note"
+				size="medium"
+				color="text-light"
+				:class="[$style.body, $style.line]"
+				data-test-id="workflow-review-activity-note"
+			>
+				{{ content.note }}
+			</N8nText>
+		</div>
+	</div>
+	<WorkflowReviewActivityFallback v-else />
+</template>
+
+<style lang="scss" module>
+@use '../activity-card' as *;
+
+.entry {
+	@include activity-row;
+}
+
+.avatar {
+	@include activity-avatar;
+}
+
+.avatarSpacer {
+	flex-shrink: 0;
+	width: var(--review-activity--avatar-size);
+}
+
+.content {
+	display: flex;
+	flex-direction: column;
+	gap: var(--spacing--3xs);
+	min-width: 0;
+}
+
+/* The separator carries its own wider spacing. */
+.headline {
+	@include activity-headline;
+}
+
+.separator {
+	margin-inline: var(--spacing--3xs);
+}
+
+.timeStamp {
+	padding-left: var(--spacing--3xs);
+}
+
+/* A decision that carries a note sits in a card, as a comment always does. */
+.boxed {
+	@include activity-card;
+}
+
+/* The same line the avatar beside it is centred on, so the two cannot drift apart. */
+.line {
+	line-height: var(--review-activity--line-height);
+}
+
+.body {
+	white-space: pre-wrap;
+	overflow-wrap: anywhere;
+}
+</style>

packages/frontend/editor-ui/src/features/workflow-reviews/components/activityEntryRegistry.ts

@@ -3,11 +3,19 @@ import type { Component } from 'vue';
 
 import WorkflowReviewActivityComment from './activity-entries/WorkflowReviewActivityComment.vue';
 import WorkflowReviewActivityFallback from './activity-entries/WorkflowReviewActivityFallback.vue';
+import WorkflowReviewActivityEventEntry from './activity-entries/WorkflowReviewActivityEventEntry.vue';
 
+// Stays partial: `workflow.published` has no writer yet, and falling back for it is the
+// designed behaviour — a total map would force a component for an entry that cannot appear.
 type ActivityEntryRegistry = Partial<Record<WorkflowReviewActivityType, Record<number, Component>>>;
 
 const registry: ActivityEntryRegistry = {
 	'comment.created': { 1: WorkflowReviewActivityComment },
+	'review.opened': { 1: WorkflowReviewActivityEventEntry },
+	'review.changes_requested': { 1: WorkflowReviewActivityEventEntry },
+	'review.approved': { 1: WorkflowReviewActivityEventEntry },
+	'review.version_updated': { 1: WorkflowReviewActivityEventEntry },
+	'review.closed': { 1: WorkflowReviewActivityEventEntry },
 };
 
 export function resolveActivityComponent(entry: WorkflowReviewActivityEntry): Component {

packages/frontend/editor-ui/src/features/workflow-reviews/reviewActivity.store.test.ts

@@ -102,7 +102,7 @@ describe('useReviewActivityStore', () => {
 		const store = useReviewActivityStore();
 		await store.fetchFeed('req-1');
 
-		await store.postComment('hi');
+		await expect(store.postComment('hi')).resolves.toBe(true);
 
 		expect(store.entries.map((entry) => entry.id)).toEqual(['3', '4', '5']);
 
@@ -237,6 +237,7 @@ describe('useReviewActivityStore', () => {
 
 		const pending = store.fetchFeed('req-1');
 		store.draft = 'for req-1 only';
+		store.decisionNote = 'for req-1 only';
 		store.reset();
 		resolveFeed(makePage(['1']));
 		await pending;
@@ -245,6 +246,7 @@ describe('useReviewActivityStore', () => {
 		expect(store.currentReviewId).toBeNull();
 		// Leaving the view must not carry a draft into whatever review is opened next.
 		expect(store.draft).toBe('');
+		expect(store.decisionNote).toBe('');
 	});
 
 	it('does not leave the next review stuck in a sending state', async () => {
@@ -361,20 +363,44 @@ describe('useReviewActivityStore', () => {
 		vi.mocked(workflowReviewsApi.fetchWorkflowReviewActivity).mockResolvedValue(makePage(['2']));
 		await store.fetchFeed('req-2');
 		resolvePost(makeEntry('9'));
-		await pending;
 
+		// Reported to the caller too: clearing a draft or switching tabs on the back of this
+		// post would hit the review the viewer is reading now.
+		await expect(pending).resolves.toBe(false);
 		expect(store.entries.map((entry) => entry.id)).toEqual(['2']);
 	});
 
-	it('drops the draft when the viewer moves to another review', async () => {
+	it('drops the comment and decision drafts when the viewer moves to another review', async () => {
 		vi.mocked(workflowReviewsApi.fetchWorkflowReviewActivity).mockResolvedValue(makePage(['1']));
 		const store = useReviewActivityStore();
 		await store.fetchFeed('req-1');
 		store.draft = 'for req-1 only';
+		store.decisionNote = 'for req-1 only';
 
 		await store.fetchFeed('req-2');
 
 		expect(store.draft).toBe('');
+		expect(store.decisionNote).toBe('');
+
+		// The composer is on screen at the same time, so dropping the note must leave the
+		// comment the viewer is writing alone.
+		store.draft = 'half a comment';
+		store.decisionNote = 'needs work';
+		store.clearDecisionNote('needs work');
+		expect(store.draft).toBe('half a comment');
+	});
+
+	// Both the decision and the comment paths clear the note after awaiting a request, by
+	// which time the viewer may have typed the next one.
+	it('keeps a note typed while the submitted one was still in flight', () => {
+		const store = useReviewActivityStore();
+		store.decisionNote = 'and one more thing';
+
+		store.clearDecisionNote('needs work');
+		expect(store.decisionNote).toBe('and one more thing');
+
+		store.clearDecisionNote('and one more thing');
+		expect(store.decisionNote).toBe('');
 	});
 
 	it('reports a failed comment to the composer, not as a feed error', async () => {

packages/frontend/editor-ui/src/features/workflow-reviews/reviewActivity.store.ts

@@ -27,6 +27,10 @@ export const useReviewActivityStore = defineStore('workflowReviewActivity', () =
 	// Held here, not in the composer: switching to the Changes tab unmounts it, and a
 	// half-typed comment must survive that.
 	const draft = ref('');
+	// Held here, not in the popover: the popover is not keyed on the review, so a local ref
+	// would carry review A's note into review B. This copy goes with the feed when the
+	// selection changes.
+	const decisionNote = ref('');
 
 	let feedRequestSeq = 0;
 	let postSeq = 0;
@@ -48,6 +52,7 @@ export const useReviewActivityStore = defineStore('workflowReviewActivity', () =
 			entries.value = [];
 			posting.value = false;
 			draft.value = '';
+			decisionNote.value = '';
 		}
 
 		try {
@@ -110,7 +115,11 @@ export const useReviewActivityStore = defineStore('workflowReviewActivity', () =
 		}
 	}
 
-	async function postComment(body: string) {
+	/**
+	 * Resolves `false` when the viewer moved to another review while the post was in flight:
+	 * the comment was written, but nothing about it belongs on the review they are reading now.
+	 */
+	async function postComment(body: string): Promise<boolean> {
 		const reviewId = currentReviewId.value;
 		// Thrown, not swallowed: a silent return would read as success to the caller.
 		if (!reviewId) throw new Error('Cannot post a comment without a selected review');
@@ -119,10 +128,11 @@ export const useReviewActivityStore = defineStore('workflowReviewActivity', () =
 		posting.value = true;
 		try {
 			const entry = await createWorkflowReviewComment(rootStore.restApiContext, reviewId, { body });
-			if (currentReviewId.value !== reviewId) return;
+			if (currentReviewId.value !== reviewId) return false;
 
 			// A feed refetch that raced this post may already carry the comment.
 			entries.value = [...entries.value.filter((existing) => existing.id !== entry.id), entry];
+			return true;
 		} finally {
 			// Only the newest post owns the flag: after A -> B -> A a stale post finishing would
 			// otherwise re-enable send while the post the user is waiting on is still in flight.
@@ -130,6 +140,14 @@ export const useReviewActivityStore = defineStore('workflowReviewActivity', () =
 		}
 	}
 
+	/**
+	 * Clears the decision note, unless the viewer typed something else since the submit
+	 * `expected` came from — both callers await a request first.
+	 */
+	function clearDecisionNote(expected: string) {
+		if (decisionNote.value.trim() === expected) decisionNote.value = '';
+	}
+
 	function reset() {
 		feedRequestSeq += 1;
 		currentReviewId.value = null;
@@ -141,6 +159,7 @@ export const useReviewActivityStore = defineStore('workflowReviewActivity', () =
 		posting.value = false;
 		error.value = null;
 		draft.value = '';
+		decisionNote.value = '';
 	}
 
 	return {
@@ -153,9 +172,11 @@ export const useReviewActivityStore = defineStore('workflowReviewActivity', () =
 		posting,
 		error,
 		draft,
+		decisionNote,
 		fetchFeed,
 		loadMore,
 		postComment,
+		clearDecisionNote,
 		reset,
 	};
 });

packages/frontend/editor-ui/src/features/workflow-reviews/reviewInbox.store.test.ts

@@ -454,7 +454,7 @@ describe('useReviewInboxStore', () => {
 				updatedAt: '2024-01-02T00:00:00.000Z',
 			});
 
-			await store.decideOnReview('req-1', 'approved');
+			await store.decideOnReview('req-1', { decision: 'approved' });
 
 			expect(store.sections.waiting.items).toEqual([]);
 			expect(store.sections.authored.items).toHaveLength(1);
@@ -476,8 +476,16 @@ describe('useReviewInboxStore', () => {
 				updatedAt: '2024-01-02T00:00:00.000Z',
 			});
 
-			await store.decideOnReview('req-2', 'changes_requested');
+			await store.decideOnReview('req-2', {
+				decision: 'changes_requested',
+				note: 'needs work',
+			});
 
+			expect(workflowReviewsApi.decideWorkflowReviewRequest).toHaveBeenCalledWith(
+				expect.anything(),
+				'req-2',
+				{ decision: 'changes_requested', note: 'needs work' },
+			);
 			expect(store.sections.waiting.items.map((item) => item.id)).toEqual(['req-1']);
 			expect(store.sections.authored.items).toEqual([
 				expect.objectContaining({ id: 'req-2', state: 'open', decision: 'changes_requested' }),
@@ -498,7 +506,7 @@ describe('useReviewInboxStore', () => {
 				autoPublish: { status: 'published' },
 			});
 
-			const response = await store.decideOnReview('req-1', 'approved');
+			const response = await store.decideOnReview('req-1', { decision: 'approved' });
 
 			expect(response.autoPublish).toEqual({ status: 'published' });
 		});
@@ -509,7 +517,9 @@ describe('useReviewInboxStore', () => {
 				new Error('forbidden'),
 			);
 
-			await expect(store.decideOnReview('req-1', 'approved')).rejects.toThrow('forbidden');
+			await expect(store.decideOnReview('req-1', { decision: 'approved' })).rejects.toThrow(
+				'forbidden',
+			);
 
 			expect(store.sections.waiting.items).toEqual([
 				expect.objectContaining({ state: 'open', decision: 'pending' }),

packages/frontend/editor-ui/src/features/workflow-reviews/reviewInbox.store.ts

@@ -329,8 +329,8 @@ export const useReviewInboxStore = defineStore('workflowReviewInbox', () => {
 	 * waiting and authored sections. Returns the response so callers can surface
 	 * the auto-publish outcome.
 	 */
-	async function decideOnReview(id: string, decision: WorkflowReviewDecisionInput) {
-		const summary = await decideWorkflowReviewRequest(rootStore.restApiContext, id, { decision });
+	async function decideOnReview(id: string, input: WorkflowReviewDecisionInput) {
+		const summary = await decideWorkflowReviewRequest(rootStore.restApiContext, id, input);
 
 		const item = findItemById(id);
 		if (item) {

packages/frontend/editor-ui/src/features/workflow-reviews/views/WorkflowReviewRequestsView.test.ts

@@ -77,13 +77,22 @@ const renderComponent = createComponentRenderer(WorkflowReviewRequestsView, {
 						<button data-test-id="retry-waiting" @click="$emit('retry', 'waiting')" />
 					</div>`,
 			},
-			// The real tooltip renders its content in a popper on hover, which jsdom
-			// cannot exercise; expose the bindings as attributes instead.
-			N8nTooltip: {
-				props: ['disabled', 'content'],
+			// The real popover cannot open in jsdom (Reka UI), so let buttons stand in for the
+			// decisions it emits and expose the props it is handed as attributes.
+			WorkflowReviewDecisionPopover: {
+				props: ['deciding', 'viewerCanDecide', 'viewerCanComment', 'ineligibilityHint'],
 				template: `
-					<div data-test-id="workflow-review-decision-tooltip" :data-disabled="disabled" :data-content="content">
-						<slot />
+					<div
+						data-test-id="workflow-review-decision-popover"
+						:data-deciding="deciding"
+						:data-ineligibility-hint="ineligibilityHint"
+					>
+						<button data-test-id="approve-review" @click="$emit('decide', { decision: 'approved' })" />
+						<button
+							data-test-id="request-changes"
+							@click="$emit('decide', { decision: 'changes_requested', note: 'needs work' })"
+						/>
+						<button data-test-id="comment-posted" @click="$emit('comment-posted')" />
 					</div>`,
 			},
 		},
@@ -308,12 +317,12 @@ describe('WorkflowReviewRequestsView', () => {
 			),
 		).toBeInTheDocument();
 		// The list item carries no eligibility data, so no decision actions yet
-		expect(queryByTestId('workflow-review-approve-button')).not.toBeInTheDocument();
+		expect(queryByTestId('workflow-review-decision-popover')).not.toBeInTheDocument();
 
 		store.detail = createDetail({ title: 'Detail review' });
 		await waitAllPromises();
 		expect(getByTestId('workflow-review-request-title')).toHaveTextContent('Detail review');
-		expect(getByTestId('workflow-review-approve-button')).toBeInTheDocument();
+		expect(getByTestId('workflow-review-decision-popover')).toBeInTheDocument();
 	});
 
 	it('hydrates the tab from the query before probing', async () => {
@@ -478,32 +487,23 @@ describe('WorkflowReviewRequestsView', () => {
 			store.decideOnReview.mockResolvedValue(decisionResponse());
 		});
 
-		it('renders both action buttons for an open review', async () => {
-			const { getByTestId } = renderComponent();
-			await waitAllPromises();
-
-			expect(getByTestId('workflow-review-approve-button')).toBeInTheDocument();
-			expect(getByTestId('workflow-review-request-changes-button')).toBeInTheDocument();
-		});
-
-		it('hides the action buttons for a closed review', async () => {
+		it('offers no decision actions for a closed review', async () => {
 			store.detail = createDetail({ state: 'closed', decision: 'approved' });
 
 			const { queryByTestId } = renderComponent();
 			await waitAllPromises();
 
-			expect(queryByTestId('workflow-review-approve-button')).not.toBeInTheDocument();
-			expect(queryByTestId('workflow-review-request-changes-button')).not.toBeInTheDocument();
+			expect(queryByTestId('workflow-review-decision-popover')).not.toBeInTheDocument();
 		});
 
 		it('submits an approval for the selected review', async () => {
 			const { getByTestId } = renderComponent();
 			await waitAllPromises();
 
-			getByTestId('workflow-review-approve-button').click();
+			getByTestId('approve-review').click();
 			await waitAllPromises();
 
-			expect(store.decideOnReview).toHaveBeenCalledWith('req-1', 'approved');
+			expect(store.decideOnReview).toHaveBeenCalledWith('req-1', { decision: 'approved' });
 			expect(showError).not.toHaveBeenCalled();
 		});
 
@@ -511,10 +511,13 @@ describe('WorkflowReviewRequestsView', () => {
 			const { getByTestId } = renderComponent();
 			await waitAllPromises();
 
-			getByTestId('workflow-review-request-changes-button').click();
+			getByTestId('request-changes').click();
 			await waitAllPromises();
 
-			expect(store.decideOnReview).toHaveBeenCalledWith('req-1', 'changes_requested');
+			expect(store.decideOnReview).toHaveBeenCalledWith('req-1', {
+				decision: 'changes_requested',
+				note: 'needs work',
+			});
 		});
 
 		it('shows a success toast when the approval published the workflow', async () => {
@@ -525,7 +528,7 @@ describe('WorkflowReviewRequestsView', () => {
 			const { getByTestId } = renderComponent();
 			await waitAllPromises();
 
-			getByTestId('workflow-review-approve-button').click();
+			getByTestId('approve-review').click();
 			await waitAllPromises();
 
 			expect(showMessage).toHaveBeenCalledWith({
@@ -544,7 +547,7 @@ describe('WorkflowReviewRequestsView', () => {
 			const { getByTestId } = renderComponent();
 			await waitAllPromises();
 
-			getByTestId('workflow-review-approve-button').click();
+			getByTestId('approve-review').click();
 			await waitAllPromises();
 
 			expect(showMessage).toHaveBeenCalledWith({
@@ -566,7 +569,7 @@ describe('WorkflowReviewRequestsView', () => {
 			const { getByTestId } = renderComponent();
 			await waitAllPromises();
 
-			getByTestId('workflow-review-approve-button').click();
+			getByTestId('approve-review').click();
 			await waitAllPromises();
 
 			expect(showMessage).toHaveBeenCalledWith(
@@ -584,7 +587,7 @@ describe('WorkflowReviewRequestsView', () => {
 			const { getByTestId } = renderComponent();
 			await waitAllPromises();
 
-			getByTestId('workflow-review-request-changes-button').click();
+			getByTestId('request-changes').click();
 			await waitAllPromises();
 
 			expect(showMessage).not.toHaveBeenCalled();
@@ -598,7 +601,7 @@ describe('WorkflowReviewRequestsView', () => {
 			const { getByTestId } = renderComponent();
 			await waitAllPromises();
 
-			getByTestId('workflow-review-approve-button').click();
+			getByTestId('approve-review').click();
 			await waitAllPromises();
 
 			expect(router.currentRoute.value.fullPath).toBe(
@@ -617,7 +620,7 @@ describe('WorkflowReviewRequestsView', () => {
 			const { getByTestId } = renderComponent();
 			await waitAllPromises();
 
-			getByTestId('workflow-review-approve-button').click();
+			getByTestId('approve-review').click();
 			await waitAllPromises();
 
 			expect(router.currentRoute.value.fullPath).toBe(
@@ -633,7 +636,7 @@ describe('WorkflowReviewRequestsView', () => {
 			const { getByTestId } = renderComponent();
 			await waitAllPromises();
 
-			getByTestId('workflow-review-request-changes-button').click();
+			getByTestId('request-changes').click();
 			await waitAllPromises();
 
 			expect(router.currentRoute.value.fullPath).toBe('/workflow-review-requests/req-1');
@@ -647,7 +650,7 @@ describe('WorkflowReviewRequestsView', () => {
 			await waitAllPromises();
 			const replaceSpy = vi.spyOn(router, 'replace');
 
-			getByTestId('workflow-review-approve-button').click();
+			getByTestId('approve-review').click();
 			await waitAllPromises();
 
 			expect(replaceSpy).not.toHaveBeenCalled();
@@ -662,7 +665,7 @@ describe('WorkflowReviewRequestsView', () => {
 
 			const { getByTestId } = renderComponent();
 			await waitAllPromises();
-			getByTestId('workflow-review-approve-button').click();
+			getByTestId('approve-review').click();
 			await waitAllPromises();
 
 			expect(showError).toHaveBeenCalledWith(error, 'Could not submit review decision');
@@ -670,83 +673,151 @@ describe('WorkflowReviewRequestsView', () => {
 
 		// The detail pane wins over the list item, so a failed decision must refresh it
 		// too — otherwise the pane stays open and actionable and every retry re-fails.
-		it('refreshes the detail as well as both sections when the decision fails', async () => {
+		it('refreshes the detail, both sections and the feed when the decision fails', async () => {
 			store.decideOnReview.mockRejectedValueOnce(new Error('conflict'));
 
 			const { getByTestId } = renderComponent();
 			await waitAllPromises();
 			store.fetchDetail.mockClear();
+			activityStore.fetchFeed.mockClear();
 			store.fetchActiveTab.mockClear();
-			getByTestId('workflow-review-approve-button').click();
+			getByTestId('approve-review').click();
 			await waitAllPromises();
 
 			expect(store.fetchActiveTab).toHaveBeenCalledTimes(1);
 			expect(store.fetchDetail).toHaveBeenCalledWith('req-1');
+			// The other reviewer's decision entry is in the feed, not in the detail payload.
+			expect(activityStore.fetchFeed).toHaveBeenCalledWith('req-1');
+			// The note stays, so the reviewer can retry with it.
+			expect(activityStore.clearDecisionNote).not.toHaveBeenCalled();
 		});
 
-		// Each button carries its own tooltip, so both must agree.
-		it('keeps the buttons enabled and the tooltips off when the viewer can decide', async () => {
-			const { getByTestId } = renderComponent();
-			await waitAllPromises();
-
-			for (const button of decisionButtons(getByTestId)) {
-				expect(button).not.toBeDisabled();
-				expect(decisionTooltip(button)).toHaveAttribute('data-disabled', 'true');
-			}
-		});
-
-		it('disables the buttons and says why when the viewer contributed a version', async () => {
-			store.detail = createDetail({
-				viewerCanDecide: false,
-				viewerDecisionIneligibilityReason: 'author',
-			});
+		it('shows the submitted decision in the feed and drops the note behind it', async () => {
+			store.decideOnReview.mockResolvedValueOnce(
+				decisionResponse({ state: 'open', decision: 'changes_requested' }),
+			);
 
 			const { getByTestId } = renderComponent();
 			await waitAllPromises();
+			activityStore.fetchFeed.mockClear();
 
-			for (const button of decisionButtons(getByTestId)) {
-				expect(button).toBeDisabled();
-				expect(decisionTooltip(button)).toHaveAttribute('data-disabled', 'false');
-				expect(decisionTooltip(button)).toHaveAttribute(
-					'data-content',
-					'You contributed a version to this review.',
-				);
-			}
+			getByTestId('request-changes').click();
+			await waitAllPromises();
+
+			expect(activityStore.fetchFeed).toHaveBeenCalledWith('req-1');
+			// The note it submitted, so a note typed while the decision was in flight survives.
+			expect(activityStore.clearDecisionNote).toHaveBeenCalledWith('needs work');
 		});
 
-		it('falls back to the generic permission hint for any other reason', async () => {
-			store.detail = createDetail({
-				viewerCanDecide: false,
-				viewerDecisionIneligibilityReason: 'missing_reviewer_permission',
-			});
+		// Otherwise the late refetch of the decided review wipes the feed of the one the
+		// viewer is now reading.
+		it('leaves the feed alone when the decision lands after the viewer moved on', async () => {
+			let resolveDecision!: () => void;
+			store.decideOnReview.mockImplementationOnce(
+				async () =>
+					await new Promise<DecideWorkflowReviewRequestResponse>((resolve) => {
+						resolveDecision = () =>
+							resolve(decisionResponse({ state: 'open', decision: 'changes_requested' }));
+					}),
+			);
 
 			const { getByTestId } = renderComponent();
 			await waitAllPromises();
+			getByTestId('approve-review').click();
+			await router.replace('/workflow-review-requests/req-2');
+			await waitAllPromises();
+			activityStore.fetchFeed.mockClear();
 
-			for (const button of decisionButtons(getByTestId)) {
-				expect(button).toBeDisabled();
-				expect(decisionTooltip(button)).toHaveAttribute(
-					'data-content',
-					'Missing permissions to perform this action',
-				);
-			}
+			resolveDecision();
+			await waitAllPromises();
+
+			expect(activityStore.fetchFeed).not.toHaveBeenCalled();
+			expect(activityStore.clearDecisionNote).not.toHaveBeenCalled();
 		});
 
-		it('does not submit a decision for an ineligible viewer', async () => {
-			store.detail = createDetail({
-				viewerCanDecide: false,
-				viewerDecisionIneligibilityReason: 'author',
-			});
+		// Following the approved review to the closed tab would also switch the feed back,
+		// throwing away the comment and note the viewer started on the review they moved to.
+		it('keeps the viewer on the review they moved to when the approval lands', async () => {
+			let resolveDecision!: () => void;
+			store.decideOnReview.mockImplementationOnce(
+				async () =>
+					await new Promise<DecideWorkflowReviewRequestResponse>((resolve) => {
+						resolveDecision = () => resolve(decisionResponse());
+					}),
+			);
 
 			const { getByTestId } = renderComponent();
 			await waitAllPromises();
-			getByTestId('workflow-review-approve-button').click();
+			getByTestId('approve-review').click();
+			await router.replace('/workflow-review-requests/req-2');
 			await waitAllPromises();
 
-			expect(store.decideOnReview).not.toHaveBeenCalled();
+			resolveDecision();
+			await waitAllPromises();
+
+			expect(router.currentRoute.value.fullPath).toBe('/workflow-review-requests/req-2');
 		});
 
-		it('disables both buttons while a decision is in flight', async () => {
+		it('shows no publish toast once the viewer has left the page', async () => {
+			let resolveDecision!: () => void;
+			store.decideOnReview.mockImplementationOnce(
+				async () =>
+					await new Promise<DecideWorkflowReviewRequestResponse>((resolve) => {
+						resolveDecision = () =>
+							resolve(
+								decisionResponse({
+									autoPublish: { status: 'failed', message: 'Version not found' },
+								}),
+							);
+					}),
+			);
+
+			const { getByTestId, unmount } = renderComponent();
+			await waitAllPromises();
+			getByTestId('approve-review').click();
+			unmount();
+
+			resolveDecision();
+			await waitAllPromises();
+
+			expect(showMessage).not.toHaveBeenCalled();
+		});
+
+		it('shows no failure toast once the viewer has left the page', async () => {
+			let rejectDecision!: (error: Error) => void;
+			store.decideOnReview.mockImplementationOnce(
+				async () =>
+					await new Promise<DecideWorkflowReviewRequestResponse>((_resolve, reject) => {
+						rejectDecision = reject;
+					}),
+			);
+
+			const { getByTestId, unmount } = renderComponent();
+			await waitAllPromises();
+			getByTestId('approve-review').click();
+			unmount();
+
+			rejectDecision(new Error('conflict'));
+			await waitAllPromises();
+
+			expect(showError).not.toHaveBeenCalled();
+			expect(store.fetchActiveTab).not.toHaveBeenCalled();
+		});
+
+		// The tab write is reached from a resolved post too, by which time the viewer may be
+		// on a page where `tab` means something else entirely.
+		it('leaves the query of another page alone when a comment lands late', async () => {
+			const { getByTestId } = renderComponent();
+			await waitAllPromises();
+
+			await router.replace('/settings/roles?tab=roles');
+			getByTestId('comment-posted').click();
+			await waitAllPromises();
+
+			expect(router.currentRoute.value.fullPath).toBe('/settings/roles?tab=roles');
+		});
+
+		it('locks the decision actions while a decision is in flight', async () => {
 			let resolveDecision!: () => void;
 			store.decideOnReview.mockImplementationOnce(
 				async () =>
@@ -758,17 +829,21 @@ describe('WorkflowReviewRequestsView', () => {
 			const { getByTestId } = renderComponent();
 			await waitAllPromises();
 
-			getByTestId('workflow-review-approve-button').click();
+			getByTestId('approve-review').click();
 			await vi.waitFor(() => {
-				expect(getByTestId('workflow-review-approve-button')).toBeDisabled();
+				expect(getByTestId('workflow-review-decision-popover')).toHaveAttribute(
+					'data-deciding',
+					'true',
+				);
 			});
-			expect(getByTestId('workflow-review-request-changes-button')).toBeDisabled();
 
 			resolveDecision();
 			await waitAllPromises();
 
-			expect(getByTestId('workflow-review-approve-button')).not.toBeDisabled();
-			expect(getByTestId('workflow-review-request-changes-button')).not.toBeDisabled();
+			expect(getByTestId('workflow-review-decision-popover')).toHaveAttribute(
+				'data-deciding',
+				'false',
+			);
 		});
 	});
 
@@ -808,23 +883,6 @@ describe('WorkflowReviewRequestsView', () => {
 	});
 });
 
-function decisionButtons(getByTestId: (id: string) => HTMLElement) {
-	return [
-		getByTestId('workflow-review-approve-button'),
-		getByTestId('workflow-review-request-changes-button'),
-	];
-}
-
-/**
- * The tab bar renders a tooltip per tab, which the N8nTooltip stub matches too,
- * so walk up from the button rather than querying the test id globally.
- */
-function decisionTooltip(button: HTMLElement) {
-	const tooltip = button.closest('[data-test-id="workflow-review-decision-tooltip"]');
-	if (!tooltip) throw new Error('decision button is not wrapped in a tooltip');
-	return tooltip;
-}
-
 function createInboxItem(): WorkflowReviewInboxItem {
 	return {
 		id: 'req-1',

packages/frontend/editor-ui/src/features/workflow-reviews/views/WorkflowReviewRequestsView.vue

@@ -65,6 +65,14 @@ function firstParam(value: string | string[] | undefined): string | null {
 
 const selectedReviewId = computed(() => firstParam(route.params.reviewRequestId));
 
+/**
+ * Watchers and resolved requests below both reach this view after the viewer may have left it,
+ * where the query params it writes mean something else entirely.
+ */
+function isOnInbox() {
+	return route.name === WORKFLOW_REVIEW_REQUESTS_VIEW;
+}
+
 function stateFromQuery(value: unknown): WorkflowReviewRequestState {
 	return value === 'closed' ? 'closed' : 'open';
 }
@@ -93,7 +101,7 @@ function handleListError(error: unknown) {
 watch(
 	selectedReviewId,
 	(id) => {
-		if (route.name !== WORKFLOW_REVIEW_REQUESTS_VIEW) return;
+		if (!isOnInbox()) return;
 		if (id) {
 			void store.fetchDetail(id).catch(handleListError);
 			// Failures surface in the feed's own error row, never as a second toast.
@@ -109,7 +117,7 @@ watch(
 watch(
 	() => route.query[REVIEW_INBOX_QUERY_PARAM.state],
 	(next) => {
-		if (route.name !== WORKFLOW_REVIEW_REQUESTS_VIEW) return;
+		if (!isOnInbox()) return;
 		void store.setActiveTab(stateFromQuery(next)).catch(handleListError);
 	},
 );
@@ -137,6 +145,7 @@ const detailTab = computed<WorkflowReviewDetailTab>(() =>
 );
 
 function onDetailTabChange(tab: WorkflowReviewDetailTab) {
+	if (!isOnInbox()) return;
 	const query = { ...route.query };
 	if (tab === 'changes') query[REVIEW_INBOX_QUERY_PARAM.tab] = tab;
 	else delete query[REVIEW_INBOX_QUERY_PARAM.tab];
@@ -174,6 +183,7 @@ function asSentence(message: string) {
  * refetches the list from here.
  */
 function followClosedReview(id: string) {
+	if (!isOnInbox()) return;
 	if (activeTab.value === 'closed') return;
 	void router.replace({
 		params: { reviewRequestId: id },
@@ -181,14 +191,27 @@ function followClosedReview(id: string) {
 	});
 }
 
-async function onDecide(id: string, decision: WorkflowReviewDecisionInput) {
+async function onDecide(id: string, input: WorkflowReviewDecisionInput) {
 	deciding.value = true;
 	try {
-		const { autoPublish, state } = await store.decideOnReview(id, decision);
-		if (state === 'closed') {
-			followClosedReview(id);
+		const { autoPublish, state } = await store.decideOnReview(id, input);
+		// The selection does not change, so the `selectedReviewId` watcher never refires and the
+		// entry this decision just wrote needs an explicit refetch. Guarded because the await
+		// above lets the viewer pick another review meanwhile: refetching the old one would wipe
+		// its feed and discard the newer review's in-flight page, and following it to the closed
+		// tab would yank the viewer off the review they are now typing on.
+		if (selectedReviewId.value === id) {
+			activityStore.clearDecisionNote(input.note ?? '');
+			void activityStore.fetchFeed(id);
+			if (state === 'closed') {
+				followClosedReview(id);
+			}
 		}
 
+		// The view can be gone by now, and a toast — the sticky publish warning above all —
+		// would sit on an unrelated page.
+		if (!isMounted) return;
+
 		if (autoPublish?.status === 'published') {
 			showMessage({
 				type: 'success',
@@ -208,6 +231,9 @@ async function onDecide(id: string, decision: WorkflowReviewDecisionInput) {
 			});
 		}
 	} catch (error) {
+		// `onUnmounted` has already reset both stores, so there is nothing left to refresh and
+		// nowhere for the toast to land but an unrelated page.
+		if (!isMounted) return;
 		showError(error, i18n.baseText('workflowReviews.decision.error.title'));
 		// The decision failed because someone else already decided (409), so
 		// refetch. Otherwise the item keeps showing as open and every retry
@@ -220,6 +246,9 @@ async function onDecide(id: string, decision: WorkflowReviewDecisionInput) {
 		} catch (refetchError) {
 			handleListError(refetchError);
 		}
+		// The feed too, or the panel keeps one missing the decision that beat this one. The
+		// note stays: the reviewer may want to retry with it.
+		if (selectedReviewId.value === id) void activityStore.fetchFeed(id);
 	} finally {
 		deciding.value = false;
 	}

packages/frontend/editor-ui/src/features/workflow-reviews/workflowReviews.api.ts

@@ -25,8 +25,8 @@ export type FetchWorkflowReviewInboxParams = {
 	cursor?: string;
 };
 
-/** A decision a reviewer can submit; `pending` is the initial state, never an input. */
-export type WorkflowReviewDecisionInput = DecideWorkflowReviewRequestDto['decision'];
+/** What a reviewer submits with a decision; `pending` is the initial state, never an input. */
+export type WorkflowReviewDecisionInput = Pick<DecideWorkflowReviewRequestDto, 'decision' | 'note'>;
 
 /** Workflow-scoped list used by the review status sync (toggle + canvas banner). */
 export async function fetchWorkflowReviewRequests(

packages/frontend/editor-ui/src/features/workflow-reviews/workflowReviews.utils.ts

@@ -11,3 +11,8 @@ type UserNameParts = {
 export function formatUserDisplayName(user: UserNameParts): string {
 	return [user.firstName, user.lastName].filter(Boolean).join(' ') || user.email || '';
 }
+
+/** Names whoever produced an activity entry. The fallback is passed in to keep i18n out of here. */
+export function formatActorName(actor: UserNameParts | null, fallback: string): string {
+	return actor ? formatUserDisplayName(actor) : fallback;
+}