feat(core): Add REST endpoints for agent-eval ratings (no-changelog) (#35419)

packages/cli/src/modules/agent-evals/__tests__/agent-eval-rating.service.test.ts

@@ -1,8 +1,6 @@
-import { AGENT_EVALS_FLAG, type CreateAgentEvalRatingPayload } from '@n8n/api-types';
-import type { Logger } from '@n8n/backend-common';
+import type { CreateAgentEvalRatingPayload } from '@n8n/api-types';
+import type { Logger, ModuleRegistry } from '@n8n/backend-common';
 import type {
-	AgentEvalDataset,
-	AgentEvalDatasetRepository,
 	AgentEvalRating,
 	AgentEvalRatingRepository,
 	AgentEvalResult,
@@ -15,126 +13,150 @@ import type { Mocked } from 'vitest';
 import { mock } from 'vitest-mock-extended';
 
 import { BadRequestError } from '@/errors/response-errors/bad-request.error';
-import { ForbiddenError } from '@/errors/response-errors/forbidden.error';
 import { NotFoundError } from '@/errors/response-errors/not-found.error';
 import type { AgentRepository } from '@/modules/agents/repositories/agent.repository';
-import { userHasScopes } from '@/permissions.ee/check-access';
-import type { PostHogClient } from '@/posthog';
 
 import { AgentEvalRatingService } from '../agent-eval-rating.service';
 
-// Stub the statically imported specifiers to keep the agents module graph out.
+// Stub the statically imported specifier to keep the agents module graph out.
 vi.mock('@/modules/agents/repositories/agent.repository', () => ({
 	AgentRepository: class AgentRepository {},
 }));
-vi.mock('@/permissions.ee/check-access', () => ({
-	userHasScopes: vi.fn().mockResolvedValue(true),
-}));
 
 const user = mock<User>({ id: 'user-1' });
 const PROJECT_ID = 'project-1';
+const AGENT_ID = 'agent-1';
+
+// Fixed so the mapper's ISO strings are assertable.
+const RATED_AT = new Date('2026-07-31T12:00:00.000Z');
+
+const makeRating = (overrides: Partial<AgentEvalRating> = {}) =>
+	mock<AgentEvalRating>({
+		id: 'rating-1',
+		resultId: 'res-1',
+		vote: 'up',
+		comment: null,
+		correction: null,
+		ratedById: 'user-1',
+		createdAt: RATED_AT,
+		updatedAt: RATED_AT,
+		...overrides,
+	});
 
 describe('AgentEvalRatingService', () => {
 	let service: AgentEvalRatingService;
 	let logger: Mocked<Logger>;
+	let moduleRegistry: Mocked<ModuleRegistry>;
 	let ratingRepository: Mocked<AgentEvalRatingRepository>;
 	let resultRepository: Mocked<AgentEvalResultRepository>;
 	let runRepository: Mocked<AgentEvalRunRepository>;
-	let datasetRepository: Mocked<AgentEvalDatasetRepository>;
 	let agentRepository: Mocked<AgentRepository>;
-	let postHogClient: Mocked<PostHogClient>;
 
 	const result = mock<AgentEvalResult>({ id: 'res-1', runId: 'run-1', status: 'success' });
 	const run = mock<AgentEvalRun>({ id: 'run-1', datasetId: 'ds-1' });
-	const dataset = mock<AgentEvalDataset>({ id: 'ds-1', agentId: 'agent-1' });
 
 	beforeEach(() => {
-		vi.mocked(userHasScopes).mockResolvedValue(true);
-
 		logger = mock<Logger>();
+		moduleRegistry = mock<ModuleRegistry>();
 		ratingRepository = mock<AgentEvalRatingRepository>();
 		resultRepository = mock<AgentEvalResultRepository>();
 		runRepository = mock<AgentEvalRunRepository>();
-		datasetRepository = mock<AgentEvalDatasetRepository>();
 		agentRepository = mock<AgentRepository>();
-		postHogClient = mock<PostHogClient>();
 
-		postHogClient.getFeatureFlags.mockResolvedValue({ [AGENT_EVALS_FLAG]: true });
+		moduleRegistry.isActive.mockReturnValue(true);
 		resultRepository.findById.mockResolvedValue(result);
-		runRepository.findById.mockResolvedValue(run);
-		datasetRepository.findById.mockResolvedValue(dataset);
+		runRepository.findByIdAndAgentId.mockResolvedValue(run);
 		agentRepository.existsByIdAndProjectId.mockResolvedValue(true);
-		ratingRepository.createRating.mockImplementation(async (attrs) =>
-			mock<AgentEvalRating>({ id: 'rating-1', ...attrs }),
-		);
+		ratingRepository.findByResultId.mockResolvedValue([]);
+		ratingRepository.findLatestByRunId.mockResolvedValue([]);
+		ratingRepository.createRating.mockImplementation(async (attrs) => makeRating(attrs));
 
 		service = new AgentEvalRatingService(
 			logger,
+			moduleRegistry,
 			ratingRepository,
 			resultRepository,
 			runRepository,
-			datasetRepository,
 			agentRepository,
-			postHogClient,
 		);
 	});
 
-	describe('access control', () => {
-		it('rejects when the agent-evals flag is disabled (as not-found, leaking no flag state)', async () => {
-			postHogClient.getFeatureFlags.mockResolvedValue({ [AGENT_EVALS_FLAG]: false });
-
-			await expect(
-				service.rateResult(user, PROJECT_ID, 'res-1', { vote: 'up' }),
-			).rejects.toThrowError(NotFoundError);
-			expect(ratingRepository.createRating).not.toHaveBeenCalled();
-		});
-
-		it('rejects a caller lacking the rating scope, without touching the eval tables', async () => {
-			vi.mocked(userHasScopes).mockResolvedValue(false);
-
-			await expect(
-				service.rateResult(user, PROJECT_ID, 'res-1', { vote: 'up' }),
-			).rejects.toThrowError(ForbiddenError);
-			// Authorization runs before any lookup, so result ids can't be probed.
-			expect(resultRepository.findById).not.toHaveBeenCalled();
-		});
-
-		// A project viewer holds execute but not update.
-		it('requires agent:execute to rate and agent:read to read', async () => {
-			await service.rateResult(user, PROJECT_ID, 'res-1', { vote: 'up' });
-			expect(vi.mocked(userHasScopes)).toHaveBeenCalledWith(user, ['agent:execute'], false, {
-				projectId: PROJECT_ID,
-			});
-
-			vi.mocked(userHasScopes).mockClear();
-
-			await service.listRatingsForResult(user, PROJECT_ID, 'res-1');
-			expect(vi.mocked(userHasScopes)).toHaveBeenCalledWith(user, ['agent:read'], false, {
-				projectId: PROJECT_ID,
-			});
-		});
-
+	/**
+	 * The project scope and the rollout flag are the controller's to enforce, so
+	 * what is asserted here is ownership: that no id from another agent or project
+	 * resolves, and that a foreign one reads as missing rather than forbidden.
+	 */
+	describe('ownership scoping', () => {
 		it('rejects an unknown result', async () => {
 			resultRepository.findById.mockResolvedValue(null);
 
 			await expect(
-				service.rateResult(user, PROJECT_ID, 'res-1', { vote: 'up' }),
+				service.rateResult(user, AGENT_ID, PROJECT_ID, 'res-1', { vote: 'up' }),
 			).rejects.toThrowError(NotFoundError);
+			expect(ratingRepository.createRating).not.toHaveBeenCalled();
 		});
 
-		it('rejects a result whose agent lives in another project (as not-found)', async () => {
+		it('rejects an agent outside the path project, without touching the eval tables', async () => {
 			agentRepository.existsByIdAndProjectId.mockResolvedValue(false);
 
 			await expect(
-				service.rateResult(user, PROJECT_ID, 'res-1', { vote: 'up' }),
+				service.rateResult(user, AGENT_ID, PROJECT_ID, 'res-1', { vote: 'up' }),
 			).rejects.toThrowError(NotFoundError);
+			// Ownership resolves before any lookup, so result ids can't be probed.
+			expect(resultRepository.findById).not.toHaveBeenCalled();
+		});
+
+		// The result exists and its project checks out, but its run belongs to a
+		// sibling agent — `@ProjectScope` cannot catch this, only the agent filter.
+		it.each([
+			[
+				'rateResult',
+				async (svc: AgentEvalRatingService) =>
+					await svc.rateResult(user, AGENT_ID, PROJECT_ID, 'res-1', { vote: 'up' }),
+			],
+			[
+				'listRatingsForResult',
+				async (svc: AgentEvalRatingService) =>
+					await svc.listRatingsForResult(AGENT_ID, PROJECT_ID, 'res-1'),
+			],
+			[
+				'listLatestRatingsForRun',
+				async (svc: AgentEvalRatingService) =>
+					await svc.listLatestRatingsForRun(AGENT_ID, PROJECT_ID, 'run-1'),
+			],
+		])('%s rejects an id belonging to another agent (as not-found)', async (_name, call) => {
+			runRepository.findByIdAndAgentId.mockResolvedValue(null);
+
+			await expect(call(service)).rejects.toThrowError(NotFoundError);
 			expect(ratingRepository.createRating).not.toHaveBeenCalled();
+			expect(ratingRepository.findByResultId).not.toHaveBeenCalled();
+			expect(ratingRepository.findLatestByRunId).not.toHaveBeenCalled();
+		});
+
+		it('resolves runs through the path agent rather than a bare id', async () => {
+			await service.listLatestRatingsForRun(AGENT_ID, PROJECT_ID, 'run-1');
+
+			expect(runRepository.findByIdAndAgentId).toHaveBeenCalledWith('run-1', AGENT_ID);
+		});
+
+		// With `agents` off there is no agent to address, so the surface reads as
+		// unknown rather than misused — and the message still names the module, so a
+		// TypeORM missing-metadata error never reaches the caller.
+		it('reports an inactive dependency module as not-found, naming the module', async () => {
+			moduleRegistry.isActive.mockReturnValue(false);
+
+			const rate = async () =>
+				await service.rateResult(user, AGENT_ID, PROJECT_ID, 'res-1', { vote: 'up' });
+
+			await expect(rate()).rejects.toThrowError(NotFoundError);
+			await expect(rate()).rejects.toThrow('require these modules to be active');
+			expect(agentRepository.existsByIdAndProjectId).not.toHaveBeenCalled();
 		});
 	});
 
 	describe('rateResult', () => {
 		it('persists an upvote with no correction, attributed to the rater', async () => {
-			await service.rateResult(user, PROJECT_ID, 'res-1', { vote: 'up' });
+			await service.rateResult(user, AGENT_ID, PROJECT_ID, 'res-1', { vote: 'up' });
 
 			expect(ratingRepository.createRating).toHaveBeenCalledWith({
 				resultId: 'res-1',
@@ -146,7 +168,7 @@ describe('AgentEvalRatingService', () => {
 		});
 
 		it('persists a downvote with a comment and the edited answer', async () => {
-			await service.rateResult(user, PROJECT_ID, 'res-1', {
+			await service.rateResult(user, AGENT_ID, PROJECT_ID, 'res-1', {
 				vote: 'down',
 				comment: 'missed the refund policy',
 				correction: { finalText: 'Refunds are processed within 14 days.' },
@@ -161,13 +183,31 @@ describe('AgentEvalRatingService', () => {
 			});
 		});
 
+		// Field-by-field, so a new entity column can't leak onto the wire.
+		it('returns the persisted rating as a wire record with ISO timestamps', async () => {
+			const correction = { finalText: 'the expected answer' };
+
+			await expect(
+				service.rateResult(user, AGENT_ID, PROJECT_ID, 'res-1', { vote: 'down', correction }),
+			).resolves.toEqual({
+				id: 'rating-1',
+				resultId: 'res-1',
+				vote: 'down',
+				comment: null,
+				correction,
+				ratedById: 'user-1',
+				createdAt: RATED_AT.toISOString(),
+				updatedAt: RATED_AT.toISOString(),
+			});
+		});
+
 		it.each(['new', 'running'] as const)('refuses to rate a %s case', async (status) => {
 			resultRepository.findById.mockResolvedValue(
 				mock<AgentEvalResult>({ id: 'res-1', runId: 'run-1', status }),
 			);
 
 			await expect(
-				service.rateResult(user, PROJECT_ID, 'res-1', { vote: 'up' }),
+				service.rateResult(user, AGENT_ID, PROJECT_ID, 'res-1', { vote: 'up' }),
 			).rejects.toThrowError(BadRequestError);
 			expect(ratingRepository.createRating).not.toHaveBeenCalled();
 		});
@@ -179,7 +219,7 @@ describe('AgentEvalRatingService', () => {
 					mock<AgentEvalResult>({ id: 'res-1', runId: 'run-1', status }),
 				);
 
-				await service.rateResult(user, PROJECT_ID, 'res-1', { vote: 'down' });
+				await service.rateResult(user, AGENT_ID, PROJECT_ID, 'res-1', { vote: 'down' });
 
 				expect(ratingRepository.createRating).toHaveBeenCalledWith(
 					expect.objectContaining({ vote: 'down' }),
@@ -188,8 +228,8 @@ describe('AgentEvalRatingService', () => {
 		);
 
 		it('appends on re-vote rather than overwriting the earlier rating', async () => {
-			await service.rateResult(user, PROJECT_ID, 'res-1', { vote: 'down' });
-			await service.rateResult(user, PROJECT_ID, 'res-1', { vote: 'up' });
+			await service.rateResult(user, AGENT_ID, PROJECT_ID, 'res-1', { vote: 'down' });
+			await service.rateResult(user, AGENT_ID, PROJECT_ID, 'res-1', { vote: 'up' });
 
 			expect(ratingRepository.createRating).toHaveBeenCalledTimes(2);
 			expect(ratingRepository.createRating).toHaveBeenLastCalledWith(
@@ -199,7 +239,7 @@ describe('AgentEvalRatingService', () => {
 
 		it('rejects an over-long comment', async () => {
 			await expect(
-				service.rateResult(user, PROJECT_ID, 'res-1', {
+				service.rateResult(user, AGENT_ID, PROJECT_ID, 'res-1', {
 					vote: 'down',
 					comment: 'x'.repeat(2_001),
 				}),
@@ -209,7 +249,7 @@ describe('AgentEvalRatingService', () => {
 
 		it('rejects an over-long corrected answer', async () => {
 			await expect(
-				service.rateResult(user, PROJECT_ID, 'res-1', {
+				service.rateResult(user, AGENT_ID, PROJECT_ID, 'res-1', {
 					vote: 'down',
 					correction: { finalText: 'x'.repeat(20_001) },
 				}),
@@ -226,12 +266,13 @@ describe('AgentEvalRatingService', () => {
 			};
 
 			await expect(
-				service.rateResult(user, PROJECT_ID, 'res-1', { vote: 'down', correction }),
+				service.rateResult(user, AGENT_ID, PROJECT_ID, 'res-1', { vote: 'down', correction }),
 			).rejects.toThrowError(BadRequestError);
 		});
 
-		// The service is the enforcement point until the REST layer's DTO validation
-		// lands, and a correction nothing can read defeats the point of capturing it.
+		// The route's DTO now rejects these shapes first, but the length caps live
+		// nowhere else and a correction nothing can read defeats the point of
+		// capturing it — so the service keeps checking rather than trusting a caller.
 		it.each([
 			{ label: 'an absent finalText', correction: {} },
 			{ label: 'the wrong key', correction: { output: 'the expected answer' } },
@@ -243,16 +284,16 @@ describe('AgentEvalRatingService', () => {
 			// payload type now refuses them at compile time.
 			const payload = { vote: 'down', correction } as CreateAgentEvalRatingPayload;
 
-			await expect(service.rateResult(user, PROJECT_ID, 'res-1', payload)).rejects.toThrowError(
-				BadRequestError,
-			);
+			await expect(
+				service.rateResult(user, AGENT_ID, PROJECT_ID, 'res-1', payload),
+			).rejects.toThrowError(BadRequestError);
 			expect(ratingRepository.createRating).not.toHaveBeenCalled();
 		});
 
 		it('keeps extra correction keys alongside the edited answer', async () => {
 			const correction = { finalText: 'the expected answer', fields: { tone: 'formal' } };
 
-			await service.rateResult(user, PROJECT_ID, 'res-1', { vote: 'down', correction });
+			await service.rateResult(user, AGENT_ID, PROJECT_ID, 'res-1', { vote: 'down', correction });
 
 			expect(ratingRepository.createRating).toHaveBeenCalledWith(
 				expect.objectContaining({ correction }),
@@ -261,33 +302,24 @@ describe('AgentEvalRatingService', () => {
 	});
 
 	describe('listRatingsForResult', () => {
-		it('returns the result history', async () => {
-			const ratings = [mock<AgentEvalRating>({ id: 'rating-2' })];
-			ratingRepository.findByResultId.mockResolvedValue(ratings);
+		it('returns the result history as wire records', async () => {
+			ratingRepository.findByResultId.mockResolvedValue([makeRating({ id: 'rating-2' })]);
 
-			await expect(service.listRatingsForResult(user, PROJECT_ID, 'res-1')).resolves.toBe(ratings);
+			await expect(service.listRatingsForResult(AGENT_ID, PROJECT_ID, 'res-1')).resolves.toEqual([
+				expect.objectContaining({ id: 'rating-2', resultId: 'res-1' }),
+			]);
 			expect(ratingRepository.findByResultId).toHaveBeenCalledWith('res-1');
 		});
 	});
 
 	describe('listLatestRatingsForRun', () => {
 		it('returns the newest rating per result for the run', async () => {
-			const ratings = [mock<AgentEvalRating>({ id: 'rating-3' })];
-			ratingRepository.findLatestByRunId.mockResolvedValue(ratings);
+			ratingRepository.findLatestByRunId.mockResolvedValue([makeRating({ id: 'rating-3' })]);
 
-			await expect(service.listLatestRatingsForRun(user, PROJECT_ID, 'run-1')).resolves.toBe(
-				ratings,
+			await expect(service.listLatestRatingsForRun(AGENT_ID, PROJECT_ID, 'run-1')).resolves.toEqual(
+				[expect.objectContaining({ id: 'rating-3' })],
 			);
 			expect(ratingRepository.findLatestByRunId).toHaveBeenCalledWith('run-1');
 		});
-
-		it('rejects a run from another project (as not-found)', async () => {
-			agentRepository.existsByIdAndProjectId.mockResolvedValue(false);
-
-			await expect(service.listLatestRatingsForRun(user, PROJECT_ID, 'run-1')).rejects.toThrowError(
-				NotFoundError,
-			);
-			expect(ratingRepository.findLatestByRunId).not.toHaveBeenCalled();
-		});
 	});
 });

packages/cli/src/modules/agent-evals/__tests__/agent-eval-runner.service.test.ts

@@ -207,6 +207,9 @@ describe('AgentEvalRunnerService', () => {
 			await expect(service.startRun('ds-1', 'proj-1', user)).rejects.toThrow(
 				'require these modules to be active: data-table',
 			);
+			// Not-found, so the whole agent-eval surface reads as absent when a module
+			// it depends on is off, rather than half-present.
+			await expect(service.startRun('ds-1', 'proj-1', user)).rejects.toThrow(NotFoundError);
 			expect(runRepository.createRun).not.toHaveBeenCalled();
 		});
 

packages/cli/src/modules/agent-evals/__tests__/agent-eval.service.test.ts

@@ -155,11 +155,14 @@ describe('AgentEvalService', () => {
 
 		// The agent lookup below reads a module entity, so the dependency check has
 		// to land before it — otherwise TypeORM raises missing metadata first.
+		// Not-found, not bad-request: with `agents` off there is no agent to address,
+		// matching the 404 that module's own unregistered routes already give.
 		it.each(callsRequiringAnAgent)(
-			'%s reports the inactive module instead of querying the agent',
+			'%s reports the inactive module as not-found instead of querying the agent',
 			async (_, call) => {
 				moduleRegistry.isActive.mockImplementation((name) => name !== 'agents');
 
+				await expect(call()).rejects.toThrow(NotFoundError);
 				await expect(call()).rejects.toThrow('Agent evals require these modules to be active');
 				expect(agentRepository.findByIdAndProjectId).not.toHaveBeenCalled();
 			},

packages/cli/src/modules/agent-evals/__tests__/agent-evals.controller.test.ts

@@ -6,11 +6,15 @@ import { mock, type MockProxy } from 'vitest-mock-extended';
 import { BadRequestError } from '@/errors/response-errors/bad-request.error';
 import { NotFoundError } from '@/errors/response-errors/not-found.error';
 
+import type { AgentEvalRatingService } from '../agent-eval-rating.service';
 import type { AgentEvalService } from '../agent-eval.service';
 import type { AgentEvalsFlagGate } from '../agent-evals-flag-gate';
 import { AgentEvalsController } from '../agent-evals.controller';
 
 vi.mock('../agent-eval.service', () => ({ AgentEvalService: class AgentEvalService {} }));
+vi.mock('../agent-eval-rating.service', () => ({
+	AgentEvalRatingService: class AgentEvalRatingService {},
+}));
 vi.mock('../agent-evals-flag-gate', () => ({ AgentEvalsFlagGate: class AgentEvalsFlagGate {} }));
 
 const PROJECT_ID = 'proj-1';
@@ -20,6 +24,7 @@ describe('AgentEvalsController', () => {
 	const user = mock<User>({ id: 'user-1' });
 
 	let service: MockProxy<AgentEvalService>;
+	let ratingService: MockProxy<AgentEvalRatingService>;
 	let flagGate: MockProxy<AgentEvalsFlagGate>;
 	let controller: AgentEvalsController;
 
@@ -33,12 +38,14 @@ describe('AgentEvalsController', () => {
 	const agentReq = () => makeReq({ projectId: PROJECT_ID, agentId: AGENT_ID });
 	const datasetReq = () => makeReq({ projectId: PROJECT_ID, agentId: AGENT_ID, datasetId: 'ds-1' });
 	const runReq = () => makeReq({ projectId: PROJECT_ID, agentId: AGENT_ID, runId: 'run-1' });
+	const resultReq = () => makeReq({ projectId: PROJECT_ID, agentId: AGENT_ID, resultId: 'res-1' });
 
 	beforeEach(() => {
 		service = mock<AgentEvalService>();
+		ratingService = mock<AgentEvalRatingService>();
 		flagGate = mock<AgentEvalsFlagGate>();
 		flagGate.assertEnabled.mockResolvedValue(undefined);
-		controller = new AgentEvalsController(service, flagGate);
+		controller = new AgentEvalsController(service, ratingService, flagGate);
 	});
 
 	/**
@@ -65,24 +72,38 @@ describe('AgentEvalsController', () => {
 			expect(route.accessScope?.scope.startsWith('agent:')).toBe(true);
 		});
 
-		it.each([
-			// Reads stay on agent:read; anything that writes eval config — including
-			// generation, which spends the builder's model credits — needs
-			// agent:update; starting/cancelling a run is agent:execute.
-			['listDatasets', 'agent:read'],
-			['getDataset', 'agent:read'],
-			['listRuns', 'agent:read'],
-			['getRun', 'agent:read'],
-			['getRunSummary', 'agent:read'],
-			['createDataset', 'agent:update'],
-			['updateDataset', 'agent:update'],
-			['deleteDataset', 'agent:update'],
-			['generateDraftCases', 'agent:update'],
-			['startRun', 'agent:execute'],
-			['cancelRun', 'agent:execute'],
-		])('%s uses %s', (handlerName, scope) => {
+		// Reads stay on agent:read; anything that writes eval config — including
+		// generation, which spends the builder's model credits, and rating, whose
+		// corrections seed judge calibration — needs agent:update;
+		// starting/cancelling a run is agent:execute.
+		const expectedScopes = {
+			listDatasets: 'agent:read',
+			getDataset: 'agent:read',
+			listRuns: 'agent:read',
+			getRun: 'agent:read',
+			getRunSummary: 'agent:read',
+			listRatingsForResult: 'agent:read',
+			listLatestRatingsForRun: 'agent:read',
+			createDataset: 'agent:update',
+			updateDataset: 'agent:update',
+			deleteDataset: 'agent:update',
+			generateDraftCases: 'agent:update',
+			rateResult: 'agent:update',
+			startRun: 'agent:execute',
+			cancelRun: 'agent:execute',
+		} as const;
+
+		it.each(Object.entries(expectedScopes))('%s uses %s', (handlerName, scope) => {
 			expect(metadata.routes.get(handlerName)?.accessScope?.scope).toBe(scope);
 		});
+
+		// The controller is the only enforcement point for the scopes, so a route
+		// added without a deliberate entry above is a scope nobody chose.
+		it('pins an expected scope for every registered handler', () => {
+			expect(routeCases.map(({ handlerName }) => handlerName).sort()).toEqual(
+				Object.keys(expectedScopes).sort(),
+			);
+		});
 	});
 
 	// Flag-off must look like an unknown endpoint on every route, and must not
@@ -120,6 +141,12 @@ describe('AgentEvalsController', () => {
 			['getRun', async () => await controller.getRun(runReq())],
 			['getRunSummary', async () => await controller.getRunSummary(runReq())],
 			['cancelRun', async () => await controller.cancelRun(runReq())],
+			[
+				'rateResult',
+				async () => await controller.rateResult(resultReq(), undefined, { vote: 'up' }),
+			],
+			['listRatingsForResult', async () => await controller.listRatingsForResult(resultReq())],
+			['listLatestRatingsForRun', async () => await controller.listLatestRatingsForRun(runReq())],
 		];
 
 		it.each(calls)('%s 404s when the flag is off for the user', async (_name, call) => {
@@ -212,5 +239,41 @@ describe('AgentEvalsController', () => {
 
 			expect(service.cancelRun).toHaveBeenCalledWith(AGENT_ID, PROJECT_ID, 'run-1');
 		});
+
+		// The rating service resolves the result through the path agent, so the agent
+		// has to reach it — a result id alone would be unscoped.
+		it('rates a result scoped to the path agent, attributed to the caller', async () => {
+			const payload = { vote: 'down' as const, correction: { finalText: 'the right answer' } };
+
+			await controller.rateResult(resultReq(), undefined, payload);
+
+			expect(ratingService.rateResult).toHaveBeenCalledWith(
+				user,
+				AGENT_ID,
+				PROJECT_ID,
+				'res-1',
+				payload,
+			);
+		});
+
+		it('reads a result rating history scoped to the path agent', async () => {
+			await controller.listRatingsForResult(resultReq());
+
+			expect(ratingService.listRatingsForResult).toHaveBeenCalledWith(
+				AGENT_ID,
+				PROJECT_ID,
+				'res-1',
+			);
+		});
+
+		it('reads a run rating summary scoped to the path agent', async () => {
+			await controller.listLatestRatingsForRun(runReq());
+
+			expect(ratingService.listLatestRatingsForRun).toHaveBeenCalledWith(
+				AGENT_ID,
+				PROJECT_ID,
+				'run-1',
+			);
+		});
 	});
 });

packages/cli/src/modules/agent-evals/agent-eval-rating.service.ts

@@ -1,21 +1,19 @@
-import { AGENT_EVALS_FLAG, type CreateAgentEvalRatingPayload } from '@n8n/api-types';
-import { Logger } from '@n8n/backend-common';
-import type { AgentEvalRating, AgentEvalResult, User } from '@n8n/db';
+import type { AgentEvalRatingRecord, CreateAgentEvalRatingPayload } from '@n8n/api-types';
+import { Logger, ModuleRegistry } from '@n8n/backend-common';
+import type { AgentEvalResult, User } from '@n8n/db';
 import {
-	AgentEvalDatasetRepository,
 	AgentEvalRatingRepository,
 	AgentEvalResultRepository,
 	AgentEvalRunRepository,
 } from '@n8n/db';
 import { Service } from '@n8n/di';
-import type { Scope } from '@n8n/permissions';
 
 import { BadRequestError } from '@/errors/response-errors/bad-request.error';
-import { ForbiddenError } from '@/errors/response-errors/forbidden.error';
 import { NotFoundError } from '@/errors/response-errors/not-found.error';
 import { AgentRepository } from '@/modules/agents/repositories/agent.repository';
-import { userHasScopes } from '@/permissions.ee/check-access';
-import { PostHogClient } from '@/posthog';
+
+import { toRatingRecord } from './agent-eval-record-mappers';
+import { assertRequiredModulesActive } from './agent-evals-required-modules';
 
 // The body arrives straight from a request, so bound it before it hits the column.
 const MAX_COMMENT_CHARS = 2_000;
@@ -25,33 +23,38 @@ const MAX_CORRECTION_CHARS = 32_000;
 /**
  * A human's 👍/👎 on an eval result, with an optional comment and correction.
  * Corrections stay on the rating row (never the dataset); ratings are append-only.
+ *
+ * **Every method is agent-scoped, and none of them authorizes.** As in the
+ * sibling `AgentEvalService`, the project scope and the rollout flag are the
+ * controller's to enforce — reach this service any other way and it checks
+ * neither. What it does own is ownership: `@ProjectScope` proves the caller may
+ * act on `:projectId`, not that the addressed agent lives there nor that a
+ * result/run id belongs to it, so each entry point resolves `(agentId,
+ * projectId)` and then reads through agent-filtered queries. A result on a
+ * sibling agent 404s like a missing one.
  */
 @Service()
 export class AgentEvalRatingService {
 	constructor(
 		private readonly logger: Logger,
+		private readonly moduleRegistry: ModuleRegistry,
 		private readonly ratingRepository: AgentEvalRatingRepository,
 		private readonly resultRepository: AgentEvalResultRepository,
 		private readonly runRepository: AgentEvalRunRepository,
-		private readonly datasetRepository: AgentEvalDatasetRepository,
 		private readonly agentRepository: AgentRepository,
-		private readonly postHogClient: PostHogClient,
 	) {}
 
-	/** Authorization is enforced here too, so no caller can skip it. */
 	async rateResult(
 		user: User,
+		agentId: string,
 		projectId: string,
 		resultId: string,
 		payload: CreateAgentEvalRatingPayload,
-	): Promise<AgentEvalRating> {
-		await this.assertFeatureEnabled(user);
-		// Execute, not update: a project viewer holds execute and can run the eval, and
-		// the reviewer of a result is often not its builder.
-		await this.assertProjectScopes(user, projectId, ['agent:execute']);
+	): Promise<AgentEvalRatingRecord> {
+		await this.assertAgentInProject(agentId, projectId);
 		assertPayloadWithinBounds(payload);
 
-		const result = await this.resolveResultInProject(projectId, resultId);
+		const result = await this.resolveResult(agentId, resultId);
 		// A pending case has no output, so the vote would judge nothing. Errored and
 		// cancelled stay rateable — "it failed" is a judgment.
 		if (result.status === 'new' || result.status === 'running') {
@@ -72,85 +75,77 @@ export class AgentEvalRatingService {
 			hasCorrection: rating.correction !== null,
 		});
 
-		return rating;
+		return toRatingRecord(rating);
 	}
 
 	/** The per-case history, newest first. */
 	async listRatingsForResult(
-		user: User,
+		agentId: string,
 		projectId: string,
 		resultId: string,
-	): Promise<AgentEvalRating[]> {
-		await this.assertFeatureEnabled(user);
-		await this.assertProjectScopes(user, projectId, ['agent:read']);
+	): Promise<AgentEvalRatingRecord[]> {
+		await this.assertAgentInProject(agentId, projectId);
 
-		const result = await this.resolveResultInProject(projectId, resultId);
+		const result = await this.resolveResult(agentId, resultId);
+		const ratings = await this.ratingRepository.findByResultId(result.id);
 
-		return await this.ratingRepository.findByResultId(result.id);
+		return ratings.map(toRatingRecord);
 	}
 
 	/** What a reopened run renders, and the corrections calibration reads. */
 	async listLatestRatingsForRun(
-		user: User,
+		agentId: string,
 		projectId: string,
 		runId: string,
-	): Promise<AgentEvalRating[]> {
-		await this.assertFeatureEnabled(user);
-		await this.assertProjectScopes(user, projectId, ['agent:read']);
+	): Promise<AgentEvalRatingRecord[]> {
+		await this.assertAgentInProject(agentId, projectId);
 
-		await this.resolveRunInProject(projectId, runId);
+		await this.resolveRun(agentId, runId);
+		const ratings = await this.ratingRepository.findLatestByRunId(runId);
 
-		return await this.ratingRepository.findLatestByRunId(runId);
+		return ratings.map(toRatingRecord);
 	}
 
 	// ---- internals ----
 
 	/**
-	 * Not-found rather than forbidden, so a flag-off instance leaks no flag state
-	 * (matching the case-generation gate).
+	 * Stops a caller with access to one project from addressing an agent in another,
+	 * which `@ProjectScope` alone cannot. Existence only: `findByIdAndProjectId`
+	 * would load `activeVersion` on every rating call.
+	 *
+	 * Every public method starts here, which makes it the one place to assert the
+	 * modules this one depends on — before the agent lookup that would otherwise
+	 * fail as a TypeORM missing-metadata error.
 	 */
-	private async assertFeatureEnabled(user: User): Promise<void> {
-		const flags = await this.postHogClient.getFeatureFlags(user);
-		if (flags?.[AGENT_EVALS_FLAG] !== true) {
-			throw new NotFoundError('Not found');
-		}
+	private async assertAgentInProject(agentId: string, projectId: string): Promise<void> {
+		assertRequiredModulesActive(this.moduleRegistry);
+		const inProject = await this.agentRepository.existsByIdAndProjectId(agentId, projectId);
+		if (!inProject) throw new NotFoundError(`Agent ${agentId} not found.`);
 	}
 
-	/** Runs before any lookup, so unauthorized callers can't probe for ids. */
-	private async assertProjectScopes(user: User, projectId: string, scopes: Scope[]): Promise<void> {
-		if (!(await userHasScopes(user, scopes, false, { projectId }))) {
-			throw new ForbiddenError('You do not have permission to review agent evals in this project.');
-		}
-	}
+	/**
+	 * A result is owned through its run, so this resolves the run agent-filtered
+	 * rather than trusting a bare result id. A result on a sibling agent reads as
+	 * missing, not forbidden, so its existence doesn't leak.
+	 */
+	private async resolveResult(agentId: string, resultId: string): Promise<AgentEvalResult> {
+		const notFound = () => new NotFoundError(`Agent eval result ${resultId} not found.`);
 
-	private async resolveResultInProject(
-		projectId: string,
-		resultId: string,
-	): Promise<AgentEvalResult> {
 		const result = await this.resultRepository.findById(resultId);
-		if (!result) throw new NotFoundError(`Agent eval result ${resultId} not found.`);
+		if (!result) throw notFound();
 
-		await this.resolveRunInProject(projectId, result.runId);
+		// Reported against the result, not the run: the run id is an internal detail
+		// the caller never named, so surfacing it would leak more than it explains.
+		const run = await this.runRepository.findByIdAndAgentId(result.runId, agentId);
+		if (!run) throw notFound();
 
 		return result;
 	}
 
-	/**
-	 * Walks run → dataset → agent to confirm project ownership. A run elsewhere reads
-	 * as missing, not forbidden, so its existence doesn't leak.
-	 */
-	private async resolveRunInProject(projectId: string, runId: string): Promise<void> {
-		const notFound = () => new NotFoundError(`Agent eval run ${runId} not found.`);
-
-		const run = await this.runRepository.findById(runId);
-		if (!run) throw notFound();
-
-		const dataset = await this.datasetRepository.findById(run.datasetId);
-		if (!dataset) throw notFound();
-
-		// Existence only: `findByIdAndProjectId` would load `activeVersion` per rating.
-		const inProject = await this.agentRepository.existsByIdAndProjectId(dataset.agentId, projectId);
-		if (!inProject) throw notFound();
+	/** The run's own agent scoping — `findByIdAndAgentId` walks run → dataset → agent. */
+	private async resolveRun(agentId: string, runId: string): Promise<void> {
+		const run = await this.runRepository.findByIdAndAgentId(runId, agentId);
+		if (!run) throw new NotFoundError(`Agent eval run ${runId} not found.`);
 	}
 }
 

packages/cli/src/modules/agent-evals/agent-eval-record-mappers.ts

@@ -1,12 +1,13 @@
 import type {
 	AgentEvalDatasetRecord,
+	AgentEvalRatingRecord,
 	AgentEvalResultRecord,
 	AgentEvalRunRecord,
 	DataTableDatasetRef,
 	DatasetRef,
 	GoogleSheetsDatasetRef,
 } from '@n8n/api-types';
-import type { AgentEvalDataset, AgentEvalResult, AgentEvalRun } from '@n8n/db';
+import type { AgentEvalDataset, AgentEvalRating, AgentEvalResult, AgentEvalRun } from '@n8n/db';
 import { UnexpectedError } from 'n8n-workflow';
 
 /**
@@ -95,3 +96,16 @@ export function toResultRecord(result: AgentEvalResult): AgentEvalResultRecord {
 		updatedAt: result.updatedAt.toISOString(),
 	};
 }
+
+export function toRatingRecord(rating: AgentEvalRating): AgentEvalRatingRecord {
+	return {
+		id: rating.id,
+		resultId: rating.resultId,
+		vote: rating.vote,
+		comment: rating.comment,
+		correction: rating.correction,
+		ratedById: rating.ratedById,
+		createdAt: rating.createdAt.toISOString(),
+		updatedAt: rating.updatedAt.toISOString(),
+	};
+}

packages/cli/src/modules/agent-evals/agent-evals-required-modules.ts

@@ -1,6 +1,6 @@
 import type { ModuleRegistry } from '@n8n/backend-common';
 
-import { BadRequestError } from '@/errors/response-errors/bad-request.error';
+import { NotFoundError } from '@/errors/response-errors/not-found.error';
 
 const REQUIRED_MODULES = ['agents', 'data-table'] as const;
 
@@ -8,11 +8,18 @@ const REQUIRED_MODULES = ['agents', 'data-table'] as const;
  * Agent evals read agents and Data Tables, whose entities only exist while those
  * modules are active. Callers must assert before their first repository touch,
  * or TypeORM raises a missing-metadata error instead of saying what is off.
+ *
+ * Not-found rather than bad-request: with `agents` off there is no agent to
+ * address, and that module's own controller isn't even registered — so its routes
+ * already 404. A nested agent surface answering 400 for the same instance state
+ * would be the odd one out. The caller sent nothing wrong; the resource is
+ * genuinely absent. The message still names the inactive module, so an operator
+ * reading the response body gets the same diagnostic as before.
  */
 export function assertRequiredModulesActive(moduleRegistry: ModuleRegistry): void {
 	const inactive = REQUIRED_MODULES.filter((name) => !moduleRegistry.isActive(name));
 	if (inactive.length > 0) {
-		throw new BadRequestError(
+		throw new NotFoundError(
 			`Agent evals require these modules to be active: ${inactive.join(', ')}.`,
 		);
 	}

packages/cli/src/modules/agent-evals/agent-evals.controller.ts

@@ -1,9 +1,11 @@
 import {
+	CreateAgentEvalRatingDto,
 	CreateAgentEvalRunDto,
 	GenerateDraftCasesOptionsDto,
 	UpdateAgentEvalDatasetDto,
 	createAgentEvalDatasetSchema,
 	type AgentEvalDatasetRecord,
+	type AgentEvalRatingRecord,
 	type AgentEvalRunDetail,
 	type AgentEvalRunRecord,
 	type AgentEvalRunSummary,
@@ -14,26 +16,33 @@ import { Body, Delete, Get, Patch, Post, ProjectScope, RestController } from '@n
 
 import { BadRequestError } from '@/errors/response-errors/bad-request.error';
 
+import { AgentEvalRatingService } from './agent-eval-rating.service';
 import { AgentEvalService } from './agent-eval.service';
 import { AgentEvalsFlagGate } from './agent-evals-flag-gate';
 
 type AgentParam = { projectId: string; agentId: string };
 type DatasetParam = AgentParam & { datasetId: string };
 type RunParam = AgentParam & { runId: string };
+type ResultParam = AgentParam & { resultId: string };
 
 /**
- * REST surface for agent evals: generation, datasets, runs and per-case results.
- * Nested under the agent so `@ProjectScope` rejects before the handler runs.
+ * REST surface for agent evals: generation, datasets, runs, per-case results and
+ * the human ratings of them. Nested under the agent so `@ProjectScope` rejects
+ * before the handler runs.
+ *
+ * **This is the single enforcement point** for both the project scope and the
+ * `101_agent_evals` rollout — the services behind it check neither, so a route
+ * added here without a scope decorator and a `flagGate` call is an open one.
  *
  * `agent:read` for reads, `agent:execute` for running a run, `agent:update` for
  * eval-config writes — including generation, which spends the builder's model
  * credits and so must stay closed to viewers (they hold `agent:execute`).
- * Ratings ship with the service that persists them.
  */
 @RestController('/projects/:projectId/agents/v2')
 export class AgentEvalsController {
 	constructor(
 		private readonly service: AgentEvalService,
+		private readonly ratingService: AgentEvalRatingService,
 		private readonly flagGate: AgentEvalsFlagGate,
 	) {}
 
@@ -153,4 +162,48 @@ export class AgentEvalsController {
 		const { agentId, projectId, runId } = req.params;
 		return await this.service.cancelRun(agentId, projectId, runId);
 	}
+
+	// ---- ratings ----
+
+	/**
+	 * `agent:update`, not `agent:execute`: the chat-user role holds execute and
+	 * nothing else, and a rating — especially a correction, which seeds later judge
+	 * calibration — is eval config a chat-only member has no business writing.
+	 */
+	@Post('/:agentId/evals/results/:resultId/ratings')
+	@ProjectScope('agent:update')
+	async rateResult(
+		req: AuthenticatedRequest<ResultParam>,
+		_res: unknown,
+		@Body payload: CreateAgentEvalRatingDto,
+	): Promise<AgentEvalRatingRecord> {
+		await this.flagGate.assertEnabled(req.user);
+		const { agentId, projectId, resultId } = req.params;
+		return await this.ratingService.rateResult(req.user, agentId, projectId, resultId, payload);
+	}
+
+	/** Ratings are append-only, so this is the case's full history, newest first. */
+	@Get('/:agentId/evals/results/:resultId/ratings')
+	@ProjectScope('agent:read')
+	async listRatingsForResult(
+		req: AuthenticatedRequest<ResultParam>,
+	): Promise<AgentEvalRatingRecord[]> {
+		await this.flagGate.assertEnabled(req.user);
+		const { agentId, projectId, resultId } = req.params;
+		return await this.ratingService.listRatingsForResult(agentId, projectId, resultId);
+	}
+
+	/**
+	 * Newest rating per rated case — what reopening a run renders. Not every rating
+	 * in the run: superseded votes stay on record but are the per-case route's job.
+	 */
+	@Get('/:agentId/evals/runs/:runId/ratings')
+	@ProjectScope('agent:read')
+	async listLatestRatingsForRun(
+		req: AuthenticatedRequest<RunParam>,
+	): Promise<AgentEvalRatingRecord[]> {
+		await this.flagGate.assertEnabled(req.user);
+		const { agentId, projectId, runId } = req.params;
+		return await this.ratingService.listLatestRatingsForRun(agentId, projectId, runId);
+	}
 }