mirror of
https://github.com/n8n-io/n8n.git
synced 2026-08-30 18:01:23 +08:00
feat(core): Add agent-eval run deadline and harden run aggregation (no-changelog) (#35099)
This commit is contained in:
@@ -51,4 +51,11 @@ export class EvaluationConfig {
|
||||
*/
|
||||
@Env('N8N_AGENT_EVALS_ENABLED')
|
||||
agentEvalsEnabled: boolean = false;
|
||||
|
||||
/**
|
||||
* Wall-clock ceiling on one agent-eval run, in minutes; non-positive disables.
|
||||
* Stops further cases starting — an in-flight one runs out its own timeout.
|
||||
*/
|
||||
@Env('N8N_AGENT_EVALS_RUN_TIMEOUT_MINUTES')
|
||||
agentEvalsRunTimeoutMinutes: number = 60;
|
||||
}
|
||||
|
||||
@@ -473,6 +473,7 @@ describe('GlobalConfig', () => {
|
||||
collectionsEnabled: false,
|
||||
configEvalsEnabled: false,
|
||||
agentEvalsEnabled: false,
|
||||
agentEvalsRunTimeoutMinutes: 60,
|
||||
},
|
||||
generic: {
|
||||
timezone: 'America/New_York',
|
||||
|
||||
@@ -82,6 +82,25 @@ describe('AgentEvalRunRepository', () => {
|
||||
});
|
||||
});
|
||||
|
||||
it('stores metrics alongside the error so a partial run keeps its tally', async () => {
|
||||
entityManager.update.mockResolvedValueOnce({ affected: 1, generatedMaps: [], raw: [] });
|
||||
|
||||
await repo.markAsError(
|
||||
'run-1',
|
||||
'timeout',
|
||||
{ message: 'deadline exceeded' },
|
||||
{ total: 5, success: 3, usage: { inputTokens: 20, outputTokens: 40 } },
|
||||
);
|
||||
|
||||
const callArgs = entityManager.update.mock.calls[0];
|
||||
expect(callArgs?.[2]).toMatchObject({
|
||||
status: 'error',
|
||||
errorCode: 'timeout',
|
||||
errorDetails: { message: 'deadline exceeded' },
|
||||
metrics: { total: 5, success: 3, usage: { inputTokens: 20, outputTokens: 40 } },
|
||||
});
|
||||
});
|
||||
|
||||
it('clears the running instance so finished runs leave no stale pointer', async () => {
|
||||
entityManager.update.mockResolvedValueOnce({ affected: 1, generatedMaps: [], raw: [] });
|
||||
|
||||
|
||||
@@ -45,12 +45,22 @@ export class AgentEvalRunRepository extends Repository<AgentEvalRun> {
|
||||
});
|
||||
}
|
||||
|
||||
async markAsError(id: string, errorCode: string, errorDetails?: IDataObject | null) {
|
||||
/**
|
||||
* `metrics` is optional: a run failing before any case ran has none, but one
|
||||
* failing partway keeps its counts where the other statuses put them.
|
||||
*/
|
||||
async markAsError(
|
||||
id: string,
|
||||
errorCode: string,
|
||||
errorDetails?: IDataObject | null,
|
||||
metrics?: IDataObject | null,
|
||||
) {
|
||||
return await this.update(id, {
|
||||
status: 'error',
|
||||
completedAt: new Date(),
|
||||
errorCode,
|
||||
errorDetails: errorDetails ?? null,
|
||||
metrics: metrics ?? null,
|
||||
runningInstanceId: null,
|
||||
});
|
||||
}
|
||||
|
||||
+4
@@ -1,3 +1,4 @@
|
||||
import { ModuleRegistry } from '@n8n/backend-common';
|
||||
import { createTeamProject, testDb, testModules } from '@n8n/backend-test-utils';
|
||||
import { GlobalConfig } from '@n8n/config';
|
||||
import {
|
||||
@@ -47,6 +48,9 @@ const buildRunner = () =>
|
||||
mock(),
|
||||
Container.get(GlobalConfig),
|
||||
instanceSettings,
|
||||
// `loadModules` above marks agents/data-table active, so the module guard
|
||||
// in `startRun` sees the same state production would.
|
||||
Container.get(ModuleRegistry),
|
||||
Container.get(AgentEvalDatasetRepository),
|
||||
Container.get(AgentEvalRunRepository),
|
||||
Container.get(AgentEvalResultRepository),
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import type { ModuleRegistry } from '@n8n/backend-common';
|
||||
import type { GlobalConfig } from '@n8n/config';
|
||||
import type {
|
||||
AgentEvalDataset,
|
||||
@@ -94,6 +95,7 @@ describe('AgentEvalRunnerService', () => {
|
||||
|
||||
let globalConfig: GlobalConfig;
|
||||
let instanceSettings: InstanceSettings;
|
||||
let moduleRegistry: MockProxy<ModuleRegistry>;
|
||||
let datasetRepository: MockProxy<AgentEvalDatasetRepository>;
|
||||
let runRepository: MockProxy<AgentEvalRunRepository>;
|
||||
let resultRepository: MockProxy<AgentEvalResultRepository>;
|
||||
@@ -118,9 +120,13 @@ describe('AgentEvalRunnerService', () => {
|
||||
vi.mocked(resolveEvaluationConcurrencyLimit).mockReturnValue(1); // serial by default
|
||||
|
||||
globalConfig = {
|
||||
// `agentEvalsEnabled` is gone from here: the flag is the gate's business now.
|
||||
evaluation: { agentEvalsRunTimeoutMinutes: 60 },
|
||||
executions: { mode: 'regular' },
|
||||
} as unknown as GlobalConfig;
|
||||
instanceSettings = { hostId: 'main-1' } as unknown as InstanceSettings;
|
||||
moduleRegistry = mock<ModuleRegistry>();
|
||||
moduleRegistry.isActive.mockReturnValue(true);
|
||||
datasetRepository = mock<AgentEvalDatasetRepository>();
|
||||
runRepository = mock<AgentEvalRunRepository>();
|
||||
resultRepository = mock<AgentEvalResultRepository>();
|
||||
@@ -150,6 +156,7 @@ describe('AgentEvalRunnerService', () => {
|
||||
mock(),
|
||||
globalConfig,
|
||||
instanceSettings,
|
||||
moduleRegistry,
|
||||
datasetRepository,
|
||||
runRepository,
|
||||
resultRepository,
|
||||
@@ -192,6 +199,17 @@ describe('AgentEvalRunnerService', () => {
|
||||
expect(runRepository.createRun).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('refuses when a module the run depends on is inactive', async () => {
|
||||
// Entities are registered per module: without this guard the run reaches
|
||||
// TypeORM with no `data_table` entity and dies there instead.
|
||||
moduleRegistry.isActive.mockImplementation((name) => name !== 'data-table');
|
||||
|
||||
await expect(service.startRun('ds-1', 'proj-1', user)).rejects.toThrow(
|
||||
'require these modules to be active: data-table',
|
||||
);
|
||||
expect(runRepository.createRun).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('rejects when the user cannot run agents in the project', async () => {
|
||||
vi.mocked(userHasScopes).mockResolvedValueOnce(false); // agent:execute check
|
||||
await expect(service.startRun('ds-1', 'proj-1', user)).rejects.toThrow(
|
||||
@@ -434,6 +452,221 @@ describe('AgentEvalRunnerService', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('settling the run', () => {
|
||||
it('tallies the run even when a cancellation write throws mid-pool', async () => {
|
||||
seedFor(
|
||||
[
|
||||
{ id: 'row-1', question: 'Q1' },
|
||||
{ id: 'row-2', question: 'Q2' },
|
||||
],
|
||||
{ cancelled: 1, new: 1 },
|
||||
);
|
||||
runRepository.isCancellationRequested.mockResolvedValue(true);
|
||||
// This write sits outside `runCase`'s safety net: it used to reject the
|
||||
// whole pool, leaving the run errored with no counts recorded.
|
||||
resultRepository.markAsCancelled
|
||||
.mockRejectedValueOnce(new Error('db down'))
|
||||
.mockResolvedValue(undefined as never);
|
||||
|
||||
const { finished } = await service.startRun('ds-1', 'proj-1', user);
|
||||
await finished;
|
||||
|
||||
// The surviving case is still processed rather than abandoned mid-flight.
|
||||
expect(resultRepository.markAsCancelled).toHaveBeenCalledTimes(2);
|
||||
expect(runRepository.markAsCancelled).toHaveBeenCalledWith(
|
||||
'run-1',
|
||||
expect.objectContaining({ total: 2, cancelled: 1, pending: 1 }),
|
||||
);
|
||||
expect(runRepository.markAsError).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('reports a dispatch failure with the counts rather than losing the tally', async () => {
|
||||
seedFor(
|
||||
[
|
||||
{ id: 'row-1', question: 'Q1' },
|
||||
{ id: 'row-2', question: 'Q2' },
|
||||
],
|
||||
{ success: 1, new: 1 },
|
||||
);
|
||||
// The first case's cancellation read blows up; the run itself is not
|
||||
// cancelled, so the second case runs to completion.
|
||||
runRepository.isCancellationRequested
|
||||
.mockRejectedValueOnce(new Error('db down'))
|
||||
.mockResolvedValue(false);
|
||||
evalAgentExecutionService.executeWithLlmMock.mockResolvedValue(successExec() as never);
|
||||
|
||||
const { finished } = await service.startRun('ds-1', 'proj-1', user);
|
||||
await finished;
|
||||
|
||||
expect(evalAgentExecutionService.executeWithLlmMock).toHaveBeenCalledTimes(1);
|
||||
expect(runRepository.markAsError).toHaveBeenCalledWith(
|
||||
'run-1',
|
||||
'case_dispatch_failed',
|
||||
expect.objectContaining({ errors: ['db down'] }),
|
||||
expect.objectContaining({ total: 2, success: 1 }),
|
||||
);
|
||||
expect(runRepository.markAsCompleted).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('keeps the tally when the final cancellation read throws', async () => {
|
||||
seedFor([{ id: 'row-1', question: 'Q1' }], { success: 1 });
|
||||
evalAgentExecutionService.executeWithLlmMock.mockResolvedValue(successExec() as never);
|
||||
// Both per-case checks pass, then the post-pool re-read fails. Letting it
|
||||
// escape would reach `failRun` and drop the counts entirely.
|
||||
runRepository.isCancellationRequested
|
||||
.mockResolvedValueOnce(false)
|
||||
.mockResolvedValueOnce(false)
|
||||
.mockRejectedValue(new Error('db down'));
|
||||
|
||||
const { finished } = await service.startRun('ds-1', 'proj-1', user);
|
||||
await finished;
|
||||
|
||||
expect(runRepository.markAsError).toHaveBeenCalledWith(
|
||||
'run-1',
|
||||
'case_dispatch_failed',
|
||||
expect.objectContaining({ errors: ['db down'] }),
|
||||
expect.objectContaining({ total: 1, success: 1 }),
|
||||
);
|
||||
// Not the tally-less `run_failed` path.
|
||||
expect(runRepository.markAsError).not.toHaveBeenCalledWith(
|
||||
'run-1',
|
||||
'run_failed',
|
||||
expect.anything(),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('run deadline', () => {
|
||||
afterEach(() => {
|
||||
vi.useRealTimers();
|
||||
});
|
||||
|
||||
it('stops starting cases at the deadline and errors the run as timed out', async () => {
|
||||
vi.useFakeTimers();
|
||||
globalConfig.evaluation.agentEvalsRunTimeoutMinutes = 30;
|
||||
seedFor(
|
||||
[
|
||||
{ id: 'row-1', question: 'Q1' },
|
||||
{ id: 'row-2', question: 'Q2' },
|
||||
],
|
||||
{ cancelled: 2 },
|
||||
);
|
||||
// Never granted a slot: on a 1-concurrency plan this is the run that would
|
||||
// otherwise sit here until the process restarts.
|
||||
concurrencyControl.throttle.mockImplementation(async () => await new Promise<void>(() => {}));
|
||||
|
||||
const { finished } = await service.startRun('ds-1', 'proj-1', user);
|
||||
await vi.advanceTimersByTimeAsync(30 * 60_000);
|
||||
await finished;
|
||||
|
||||
expect(evalAgentExecutionService.executeWithLlmMock).not.toHaveBeenCalled();
|
||||
// The queued case is evicted so it stops holding a place in the queue.
|
||||
expect(concurrencyControl.remove).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ mode: 'evaluation' }),
|
||||
);
|
||||
expect(resultRepository.markAsCancelled).toHaveBeenCalledTimes(2);
|
||||
expect(runRepository.markAsError).toHaveBeenCalledWith(
|
||||
'run-1',
|
||||
'timeout',
|
||||
expect.objectContaining({
|
||||
message: expect.stringContaining('2 case(s) were not started'),
|
||||
}),
|
||||
// counts land under `metrics`, same as every other terminal status
|
||||
expect.objectContaining({ total: 2, cancelled: 2 }),
|
||||
);
|
||||
// A deadline is a failure, not the user's Stop.
|
||||
expect(runRepository.markAsCancelled).not.toHaveBeenCalled();
|
||||
expect(runRepository.markAsCompleted).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('reports a user Stop as cancelled even when the deadline also fired', async () => {
|
||||
vi.useFakeTimers();
|
||||
globalConfig.evaluation.agentEvalsRunTimeoutMinutes = 30;
|
||||
seedFor([{ id: 'row-1', question: 'Q1' }], { cancelled: 1 });
|
||||
concurrencyControl.throttle.mockImplementation(async () => await new Promise<void>(() => {}));
|
||||
// Stop was requested; the case is parked, so only the post-pool re-read
|
||||
// observes it.
|
||||
runRepository.isCancellationRequested.mockResolvedValueOnce(false).mockResolvedValue(true);
|
||||
|
||||
const { finished } = await service.startRun('ds-1', 'proj-1', user);
|
||||
await vi.advanceTimersByTimeAsync(30 * 60_000);
|
||||
await finished;
|
||||
|
||||
expect(runRepository.markAsCancelled).toHaveBeenCalledWith('run-1', expect.anything());
|
||||
expect(runRepository.markAsError).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('completes a run whose last case finished just past the deadline', async () => {
|
||||
vi.useFakeTimers();
|
||||
globalConfig.evaluation.agentEvalsRunTimeoutMinutes = 30;
|
||||
seedFor([{ id: 'row-1', question: 'Q1' }], { success: 1 });
|
||||
// The only case is still executing when the deadline fires, so the abort
|
||||
// has nothing left to skip. Expiry alone must not fail a run that finished.
|
||||
let caseStarted: (() => void) | undefined;
|
||||
const started = new Promise<void>((resolve) => {
|
||||
caseStarted = resolve;
|
||||
});
|
||||
let finishCase: (() => void) | undefined;
|
||||
evalAgentExecutionService.executeWithLlmMock.mockImplementation(async () => {
|
||||
caseStarted?.();
|
||||
await new Promise<void>((resolve) => {
|
||||
finishCase = resolve;
|
||||
});
|
||||
return successExec() as never;
|
||||
});
|
||||
|
||||
const { finished } = await service.startRun('ds-1', 'proj-1', user);
|
||||
await started;
|
||||
await vi.advanceTimersByTimeAsync(30 * 60_000);
|
||||
finishCase?.();
|
||||
await finished;
|
||||
|
||||
expect(runRepository.markAsCompleted).toHaveBeenCalledWith(
|
||||
'run-1',
|
||||
expect.objectContaining({ total: 1, success: 1 }),
|
||||
);
|
||||
expect(runRepository.markAsError).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('does not fire instantly when the configured deadline overflows the timer', async () => {
|
||||
vi.useFakeTimers();
|
||||
// Past the 32-bit ms ceiling, where an unclamped delay collapses to 1ms.
|
||||
globalConfig.evaluation.agentEvalsRunTimeoutMinutes = 40_000;
|
||||
seedFor([{ id: 'row-1', question: 'Q1' }], { success: 1 });
|
||||
evalAgentExecutionService.executeWithLlmMock.mockResolvedValue(successExec() as never);
|
||||
// Park the case on the clock, so an over-eager deadline gets to abort it
|
||||
// before the pool can finish on microtasks alone.
|
||||
let releaseSlot: (() => void) | undefined;
|
||||
concurrencyControl.throttle.mockImplementation(async () => {
|
||||
await new Promise<void>((resolve) => {
|
||||
releaseSlot = resolve;
|
||||
});
|
||||
});
|
||||
|
||||
const { finished } = await service.startRun('ds-1', 'proj-1', user);
|
||||
await vi.advanceTimersByTimeAsync(60_000);
|
||||
releaseSlot?.();
|
||||
await finished;
|
||||
|
||||
expect(evalAgentExecutionService.executeWithLlmMock).toHaveBeenCalledTimes(1);
|
||||
expect(runRepository.markAsCompleted).toHaveBeenCalled();
|
||||
expect(runRepository.markAsError).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('runs without a deadline when the timeout is disabled', async () => {
|
||||
vi.useFakeTimers();
|
||||
globalConfig.evaluation.agentEvalsRunTimeoutMinutes = 0;
|
||||
seedFor([{ id: 'row-1', question: 'Q1' }], { success: 1 });
|
||||
evalAgentExecutionService.executeWithLlmMock.mockResolvedValue(successExec() as never);
|
||||
|
||||
const { finished } = await service.startRun('ds-1', 'proj-1', user);
|
||||
await finished;
|
||||
|
||||
expect(vi.getTimerCount()).toBe(0);
|
||||
expect(runRepository.markAsCompleted).toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe('concurrency', () => {
|
||||
it('runs each case through the shared evaluation queue and releases the slot', async () => {
|
||||
seedFor(
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import type { ModuleRegistry } from '@n8n/backend-common';
|
||||
import type {
|
||||
AgentEvalDataset,
|
||||
AgentEvalDatasetRepository,
|
||||
@@ -36,6 +37,7 @@ const PROJECT_ID = 'proj-1';
|
||||
describe('AgentEvalService', () => {
|
||||
const user = mock<User>({ id: 'user-1' });
|
||||
|
||||
let moduleRegistry: MockProxy<ModuleRegistry>;
|
||||
let agentRepository: MockProxy<AgentRepository>;
|
||||
let datasetRepository: MockProxy<AgentEvalDatasetRepository>;
|
||||
let runRepository: MockProxy<AgentEvalRunRepository>;
|
||||
@@ -79,6 +81,8 @@ describe('AgentEvalService', () => {
|
||||
});
|
||||
|
||||
beforeEach(() => {
|
||||
moduleRegistry = mock<ModuleRegistry>();
|
||||
moduleRegistry.isActive.mockReturnValue(true);
|
||||
agentRepository = mock<AgentRepository>();
|
||||
datasetRepository = mock<AgentEvalDatasetRepository>();
|
||||
runRepository = mock<AgentEvalRunRepository>();
|
||||
@@ -91,6 +95,7 @@ describe('AgentEvalService', () => {
|
||||
runRepository.findByIdAndAgentId.mockResolvedValue(makeRun());
|
||||
|
||||
service = new AgentEvalService(
|
||||
moduleRegistry,
|
||||
agentRepository,
|
||||
datasetRepository,
|
||||
runRepository,
|
||||
@@ -147,6 +152,18 @@ describe('AgentEvalService', () => {
|
||||
|
||||
expect(agentRepository.findByIdAndProjectId).toHaveBeenCalledWith(AGENT_ID, PROJECT_ID);
|
||||
});
|
||||
|
||||
// The agent lookup below reads a module entity, so the dependency check has
|
||||
// to land before it — otherwise TypeORM raises missing metadata first.
|
||||
it.each(callsRequiringAnAgent)(
|
||||
'%s reports the inactive module instead of querying the agent',
|
||||
async (_, call) => {
|
||||
moduleRegistry.isActive.mockImplementation((name) => name !== 'agents');
|
||||
|
||||
await expect(call()).rejects.toThrow('Agent evals require these modules to be active');
|
||||
expect(agentRepository.findByIdAndProjectId).not.toHaveBeenCalled();
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
// A dataset/run id from another agent must not resolve just because the caller
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import type { AgentEvalRunSummary } from '@n8n/api-types';
|
||||
import { Logger } from '@n8n/backend-common';
|
||||
import { Logger, ModuleRegistry } from '@n8n/backend-common';
|
||||
import { GlobalConfig } from '@n8n/config';
|
||||
import type { AgentEvalDataset, AgentEvalResult, User } from '@n8n/db';
|
||||
import {
|
||||
@@ -31,6 +31,7 @@ import { EvalAgentExecutionService } from '@/modules/instance-ai/eval/agent-exec
|
||||
import { userHasScopes } from '@/permissions.ee/check-access';
|
||||
|
||||
import { AgentEvalsFlagGate } from './agent-evals-flag-gate';
|
||||
import { assertRequiredModulesActive } from './agent-evals-required-modules';
|
||||
|
||||
const ROW_PAGE_SIZE = 100;
|
||||
// Per-run in-flight cap layered on the shared evaluation queue: keeps one run
|
||||
@@ -39,6 +40,9 @@ const MAX_PER_RUN_CONCURRENCY = 10;
|
||||
// Each case is a real agent execution, so a run is capped rather than letting a
|
||||
// large table launch thousands of model calls. Raise deliberately if needed.
|
||||
const MAX_CASES = 500;
|
||||
// `setTimeout` holds its delay in a 32-bit int and silently coerces anything
|
||||
// larger to 1ms, so a deadline past ~24.8 days must be clamped, not passed on.
|
||||
const MAX_TIMER_DELAY_MS = 2_147_483_647;
|
||||
|
||||
/** A dataset row resolved into a runnable case via the dataset's column mapping. */
|
||||
interface ResolvedCase {
|
||||
@@ -66,6 +70,8 @@ interface CaseUsage {
|
||||
* queue (the same one the workflow eval uses), so a single run can't flood the
|
||||
* queue and concurrent runs can't collectively exceed the plan limit;
|
||||
* cross-main *cancellation* is honored via the run's `cancelRequested` flag.
|
||||
* A run-level deadline bounds the wall clock, since a 1-slot plan otherwise
|
||||
* lets a large dataset run until the process restarts.
|
||||
* Behind the `101_agent_evals` flag.
|
||||
*/
|
||||
@Service()
|
||||
@@ -74,6 +80,7 @@ export class AgentEvalRunnerService {
|
||||
private readonly logger: Logger,
|
||||
private readonly globalConfig: GlobalConfig,
|
||||
private readonly instanceSettings: InstanceSettings,
|
||||
private readonly moduleRegistry: ModuleRegistry,
|
||||
private readonly datasetRepository: AgentEvalDatasetRepository,
|
||||
private readonly runRepository: AgentEvalRunRepository,
|
||||
private readonly resultRepository: AgentEvalResultRepository,
|
||||
@@ -104,6 +111,9 @@ export class AgentEvalRunnerService {
|
||||
throw new BadRequestError('Agent eval runs are not supported in queue mode.');
|
||||
}
|
||||
|
||||
// Backstop for direct callers; the REST path asserts before its own lookups.
|
||||
assertRequiredModulesActive(this.moduleRegistry);
|
||||
|
||||
// Authorize up front. `executeWithLlmMock` also checks `agent:execute`, but
|
||||
// it returns an error result rather than throwing — without this a caller
|
||||
// lacking permission would get a created run with every case marked failed
|
||||
@@ -227,48 +237,59 @@ export class AgentEvalRunnerService {
|
||||
? Math.min(resolvedLimit, MAX_PER_RUN_CONCURRENCY)
|
||||
: MAX_PER_RUN_CONCURRENCY,
|
||||
);
|
||||
// Cooperative cancellation: whichever case first observes the flag aborts,
|
||||
// which evicts every case still waiting for a queue slot.
|
||||
// Whichever case first observes a cancel aborts, evicting every case still
|
||||
// queued. The deadline below trips the same signal.
|
||||
const abort = new AbortController();
|
||||
const totalUsage: CaseUsage = { inputTokens: 0, outputTokens: 0 };
|
||||
|
||||
const cancelCase = async (resultRow: AgentEvalResult) => {
|
||||
// Set only when a case actually observed the cancellation flag, so the
|
||||
// settle step can tell a user cancel apart from a deadline — both abort.
|
||||
let cancelObserved = false;
|
||||
const shouldStopCase = async (): Promise<boolean> => {
|
||||
if (abort.signal.aborted) return true;
|
||||
// One DB read per case at most: once the first observer aborts, the
|
||||
// check above short-circuits for the rest.
|
||||
if (!(await this.runRepository.isCancellationRequested(runId))) return false;
|
||||
cancelObserved = true;
|
||||
abort.abort();
|
||||
return true;
|
||||
};
|
||||
|
||||
// Cases the run never started: lets the settle step tell a deadline that
|
||||
// cost work from one that merely elapsed as the last case finished.
|
||||
let stoppedCases = 0;
|
||||
const stopCase = async (resultRow: AgentEvalResult) => {
|
||||
stoppedCases++;
|
||||
await this.resultRepository.markAsCancelled(resultRow.id);
|
||||
};
|
||||
|
||||
await Promise.all(
|
||||
const deadline = this.startRunDeadline(abort);
|
||||
|
||||
// The stop reads/writes around `runCase` sit outside its safety net, so
|
||||
// collect their throws instead of letting one reject the whole pool.
|
||||
const settlements = await Promise.allSettled(
|
||||
cases.map(
|
||||
async (resolvedCase, index) =>
|
||||
await limit(async () => {
|
||||
const resultRow = seeded[index];
|
||||
// Don't enqueue a case once the run is cancelled (the flag short-
|
||||
// circuits after the first observer, so this is one DB read per
|
||||
// case at most).
|
||||
if (
|
||||
abort.signal.aborted ||
|
||||
(await this.runRepository.isCancellationRequested(runId))
|
||||
) {
|
||||
await cancelCase(resultRow);
|
||||
// Don't enqueue a case once the run is stopping.
|
||||
if (await shouldStopCase()) {
|
||||
await stopCase(resultRow);
|
||||
return;
|
||||
}
|
||||
|
||||
// Wait for a queue slot, bailing (and evicting the entry) if the run
|
||||
// is cancelled while queued. The helper owns release/remove for the
|
||||
// bail path; the finally owns release for the acquired path.
|
||||
// The helper owns release/remove when the run stops while queued;
|
||||
// the finally owns release once a slot is held.
|
||||
const executionId = `${runId}-case-${index}`;
|
||||
if (!(await this.acquireEvaluationSlot(executionId, abort.signal))) {
|
||||
await this.resultRepository.markAsCancelled(resultRow.id);
|
||||
await stopCase(resultRow);
|
||||
return;
|
||||
}
|
||||
|
||||
try {
|
||||
// A cancel may have landed while we waited for the slot.
|
||||
if (
|
||||
abort.signal.aborted ||
|
||||
(await this.runRepository.isCancellationRequested(runId))
|
||||
) {
|
||||
await cancelCase(resultRow);
|
||||
// A cancel or the deadline may have landed while we waited.
|
||||
if (await shouldStopCase()) {
|
||||
await stopCase(resultRow);
|
||||
return;
|
||||
}
|
||||
const usage = await this.runCase(resultRow, resolvedCase, ctx);
|
||||
@@ -282,19 +303,70 @@ export class AgentEvalRunnerService {
|
||||
}),
|
||||
),
|
||||
);
|
||||
// `allSettled` never rejects, so the timer is always cleared here.
|
||||
deadline.clear();
|
||||
|
||||
const dispatchFailures: string[] = [];
|
||||
for (const settlement of settlements) {
|
||||
if (settlement.status !== 'rejected') continue;
|
||||
const reason: unknown = settlement.reason;
|
||||
dispatchFailures.push(reason instanceof Error ? reason.message : String(reason));
|
||||
}
|
||||
// Re-read once more: a cancel that arrived after every case had already
|
||||
// settled is never observed above, so honor it rather than reporting the
|
||||
// run completed with the flag still set.
|
||||
const wasCancelled =
|
||||
abort.signal.aborted || (await this.runRepository.isCancellationRequested(runId));
|
||||
// run completed with the flag still set. Collected like the pool's throws,
|
||||
// since letting this one escape would lose the tally it guards.
|
||||
let wasCancelled = cancelObserved;
|
||||
if (!wasCancelled) {
|
||||
try {
|
||||
wasCancelled = await this.runRepository.isCancellationRequested(runId);
|
||||
} catch (error) {
|
||||
dispatchFailures.push(error instanceof Error ? error.message : String(error));
|
||||
}
|
||||
}
|
||||
|
||||
if (dispatchFailures.length > 0) {
|
||||
this.logger.error(
|
||||
`[AgentEvalRunner] ${dispatchFailures.length} failure(s) outside case execution in run ${runId}`,
|
||||
{ errors: dispatchFailures },
|
||||
);
|
||||
}
|
||||
|
||||
const counts = await this.resultRepository.countByStatus(runId);
|
||||
const metrics: IDataObject = { ...toSummaryCounts(counts), usage: { ...totalUsage } };
|
||||
|
||||
// Every branch records the tally under `metrics`, so pollers never find a
|
||||
// settled run whose counts are missing or filed elsewhere.
|
||||
if (wasCancelled) {
|
||||
// A user stop outranks the deadline: it's intent, not a failure.
|
||||
await this.runRepository.markAsCancelled(runId, metrics);
|
||||
} else if (deadline.hasExpired() && stoppedCases > 0) {
|
||||
await this.runRepository.markAsError(
|
||||
runId,
|
||||
'timeout',
|
||||
{
|
||||
message: `Run exceeded its ${deadline.deadlineMinutes}-minute deadline; ${stoppedCases} case(s) were not started.`,
|
||||
},
|
||||
metrics,
|
||||
);
|
||||
} else if (dispatchFailures.length > 0) {
|
||||
await this.runRepository.markAsError(
|
||||
runId,
|
||||
'case_dispatch_failed',
|
||||
{
|
||||
message: `${dispatchFailures.length} failure(s) occurred outside case execution.`,
|
||||
errors: dispatchFailures,
|
||||
},
|
||||
metrics,
|
||||
);
|
||||
} else {
|
||||
if (deadline.hasExpired()) {
|
||||
// Overran, but nothing was left to skip. Logged for anyone tuning the
|
||||
// limit, since the run itself still succeeded.
|
||||
this.logger.debug(
|
||||
`[AgentEvalRunner] Run ${runId} overran its ${deadline.deadlineMinutes}-minute deadline, but every case had finished`,
|
||||
);
|
||||
}
|
||||
await this.runRepository.markAsCompleted(runId, metrics);
|
||||
}
|
||||
} catch (error) {
|
||||
@@ -303,6 +375,34 @@ export class AgentEvalRunnerService {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Arm the run-level deadline; expiring trips the run's abort. Non-positive
|
||||
* disables. Bounds when the last case may *start*, not when the run ends.
|
||||
*/
|
||||
private startRunDeadline(abort: AbortController): {
|
||||
hasExpired: () => boolean;
|
||||
clear: () => void;
|
||||
deadlineMinutes: number;
|
||||
} {
|
||||
const deadlineMinutes = this.globalConfig.evaluation.agentEvalsRunTimeoutMinutes;
|
||||
if (deadlineMinutes <= 0) {
|
||||
return { hasExpired: () => false, clear: () => undefined, deadlineMinutes };
|
||||
}
|
||||
|
||||
let expired = false;
|
||||
const timer = setTimeout(
|
||||
() => {
|
||||
expired = true;
|
||||
abort.abort();
|
||||
},
|
||||
Math.min(deadlineMinutes * 60_000, MAX_TIMER_DELAY_MS),
|
||||
);
|
||||
// Never hold the process open for a run that outlived its deadline.
|
||||
timer.unref();
|
||||
|
||||
return { hasExpired: () => expired, clear: () => clearTimeout(timer), deadlineMinutes };
|
||||
}
|
||||
|
||||
/**
|
||||
* Acquire a slot in the shared evaluation queue, abort-aware. Resolves `true`
|
||||
* with the slot held (the caller must `release`); resolves `false` if the run
|
||||
|
||||
@@ -9,6 +9,7 @@ import type {
|
||||
GenerateDraftCasesResult,
|
||||
UpdateAgentEvalDatasetPayload,
|
||||
} from '@n8n/api-types';
|
||||
import { ModuleRegistry } from '@n8n/backend-common';
|
||||
import type { AgentEvalDataset, AgentEvalRun, User } from '@n8n/db';
|
||||
import {
|
||||
AgentEvalDatasetRepository,
|
||||
@@ -24,6 +25,7 @@ import { AgentRepository } from '@/modules/agents/repositories/agent.repository'
|
||||
import { AgentEvalCaseGenerationService } from './agent-eval-case-generation.service';
|
||||
import { toDatasetRecord, toResultRecord, toRunRecord } from './agent-eval-record-mappers';
|
||||
import { AgentEvalRunnerService } from './agent-eval-runner.service';
|
||||
import { assertRequiredModulesActive } from './agent-evals-required-modules';
|
||||
|
||||
/** Statuses a run can still be asked to stop from. */
|
||||
const CANCELLABLE_STATUSES = new Set(['new', 'running']);
|
||||
@@ -39,6 +41,7 @@ const CANCELLABLE_STATUSES = new Set(['new', 'running']);
|
||||
@Service()
|
||||
export class AgentEvalService {
|
||||
constructor(
|
||||
private readonly moduleRegistry: ModuleRegistry,
|
||||
private readonly agentRepository: AgentRepository,
|
||||
private readonly datasetRepository: AgentEvalDatasetRepository,
|
||||
private readonly runRepository: AgentEvalRunRepository,
|
||||
@@ -205,7 +208,12 @@ export class AgentEvalService {
|
||||
|
||||
// `@ProjectScope` only checks the project in the URL, so this is what stops a
|
||||
// caller with access to one project from addressing an agent in another.
|
||||
//
|
||||
// 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 assertAgentInProject(agentId: string, projectId: string): Promise<void> {
|
||||
assertRequiredModulesActive(this.moduleRegistry);
|
||||
const agent = await this.agentRepository.findByIdAndProjectId(agentId, projectId);
|
||||
if (!agent) throw new NotFoundError(`Agent ${agentId} not found.`);
|
||||
}
|
||||
|
||||
@@ -0,0 +1,19 @@
|
||||
import type { ModuleRegistry } from '@n8n/backend-common';
|
||||
|
||||
import { BadRequestError } from '@/errors/response-errors/bad-request.error';
|
||||
|
||||
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.
|
||||
*/
|
||||
export function assertRequiredModulesActive(moduleRegistry: ModuleRegistry): void {
|
||||
const inactive = REQUIRED_MODULES.filter((name) => !moduleRegistry.isActive(name));
|
||||
if (inactive.length > 0) {
|
||||
throw new BadRequestError(
|
||||
`Agent evals require these modules to be active: ${inactive.join(', ')}.`,
|
||||
);
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user