From 67e9b6a9b45ea73232160ab3135144222c94ceef Mon Sep 17 00:00:00 2001 From: Luka Zivkovic Date: Thu, 15 Oct 2026 00:54:08 +0200 Subject: [PATCH 01/18] fix(editor): Show tool progress in running agent checks --- .../src/schemas/instance-ai.schema.ts | 3 + .../agent-eval-result.repository.test.ts | 12 ++ .../agent-eval-result.repository.ee.ts | 9 +- .../agent-eval-runner.service.test.ts | 33 ++++ .../agent-evals/agent-eval-runner.service.ts | 11 +- .../__tests__/agent-execution.service.test.ts | 175 +++++++++++++++++- .../eval/agent-execution.service.ts | 56 ++++++ .../frontend/@n8n/i18n/src/locales/en.json | 1 + .../__tests__/AgentEvalChecksPanel.test.ts | 24 +++ .../agents/components/AgentEvalTryRow.vue | 27 ++- .../agents/components/EvalInitialSample.vue | 7 +- .../__tests__/AgentEvalTryRow.test.ts | 22 ++- .../__tests__/agent-eval-tool-calls.test.ts | 14 ++ .../agents/utils/agent-eval-tool-calls.ts | 18 +- .../InstanceAiTestAgentPreviewPanel.test.ts | 17 +- 15 files changed, 396 insertions(+), 33 deletions(-) diff --git a/packages/@n8n/api-types/src/schemas/instance-ai.schema.ts b/packages/@n8n/api-types/src/schemas/instance-ai.schema.ts index dd6a6ed2047..6a5441040c3 100644 --- a/packages/@n8n/api-types/src/schemas/instance-ai.schema.ts +++ b/packages/@n8n/api-types/src/schemas/instance-ai.schema.ts @@ -2933,6 +2933,9 @@ export class InstanceAiEvalExecutionRequest extends Z.class({ // --------------------------------------------------------------------------- export interface InstanceAiEvalAgentToolCallRecord { + /** Present on live progress records. Older completed records omit these fields. */ + toolCallId?: string; + status?: 'running' | 'completed' | 'error'; /** Sanitized tool name — matches what the model called. */ tool: string; /** Where the tool executes. 'other' covers built-ins (skills, todos, environment). */ diff --git a/packages/@n8n/db/src/repositories/__tests__/agent-eval-result.repository.test.ts b/packages/@n8n/db/src/repositories/__tests__/agent-eval-result.repository.test.ts index c8d0944c383..cf6bffc17a0 100644 --- a/packages/@n8n/db/src/repositories/__tests__/agent-eval-result.repository.test.ts +++ b/packages/@n8n/db/src/repositories/__tests__/agent-eval-result.repository.test.ts @@ -57,6 +57,18 @@ describe('AgentEvalResultRepository', () => { }); }); + it('updates progress only while the attempt is running', async () => { + const calls = { calls: [{ tool: 'Slack_Tool', status: 'running' }] }; + const runAt = new Date(); + await repo.updateRunningToolCalls('res-1', runAt, calls); + expect(entityManager.update.mock.calls[0]?.[1]).toEqual({ + id: 'res-1', + status: 'running', + runAt, + }); + expect(entityManager.update.mock.calls[0]?.[2]).toEqual({ toolCalls: calls }); + }); + describe('markAsCompleted', () => { it('stores the agent output and defaults toolCalls/metrics to null', async () => { entityManager.update.mockResolvedValueOnce({ affected: 1, generatedMaps: [], raw: [] }); diff --git a/packages/@n8n/db/src/repositories/agent-eval-result.repository.ee.ts b/packages/@n8n/db/src/repositories/agent-eval-result.repository.ee.ts index 3a1fa8a29ed..1a630d75f9c 100644 --- a/packages/@n8n/db/src/repositories/agent-eval-result.repository.ee.ts +++ b/packages/@n8n/db/src/repositories/agent-eval-result.repository.ee.ts @@ -84,7 +84,14 @@ export class AgentEvalResultRepository extends Repository { /** Starts an attempt. Clears whatever a previous attempt left behind, so a * rerun can't settle with an old error next to a new answer (or the reverse). */ async markAsRunning(id: string) { - return await this.update(id, startedAttempt()); + const attempt = startedAttempt(); + await this.update(id, attempt); + return attempt.runAt; + } + + /** Progress belongs to one running attempt, identified by its start timestamp. */ + async updateRunningToolCalls(id: string, runAt: Date, toolCalls: JsonObject) { + return await this.update({ id, status: 'running', runAt }, { toolCalls }); } /** diff --git a/packages/cli/src/modules/agent-evals/__tests__/agent-eval-runner.service.test.ts b/packages/cli/src/modules/agent-evals/__tests__/agent-eval-runner.service.test.ts index 5548f5d9760..8af351a9973 100644 --- a/packages/cli/src/modules/agent-evals/__tests__/agent-eval-runner.service.test.ts +++ b/packages/cli/src/modules/agent-evals/__tests__/agent-eval-runner.service.test.ts @@ -369,6 +369,7 @@ describe('AgentEvalRunnerService', () => { user, { projectId: 'proj-1' }, 'What is 2+2?', + { onProgress: expect.any(Function) }, ); expect(runRepository.markAsRunning).toHaveBeenCalledWith('run-1', 'main-1'); expect(resultRepository.markAsCompleted).toHaveBeenCalledTimes(2); @@ -382,6 +383,36 @@ describe('AgentEvalRunnerService', () => { ); }); + it('persists in-flight tool calls for the current attempt before completion', async () => { + seedFor([{ id: 'row-1', question: 'Send a message' }], { success: 1 }); + const runAt = new Date(); + resultRepository.markAsRunning.mockResolvedValue(runAt); + evalAgentExecutionService.executeWithLlmMock.mockImplementation( + async (_agent, _user, _options, _input, hooks) => { + await hooks?.onProgress?.([ + { + tool: 'Slack_Tool', + toolCallId: 'call-1', + status: 'running', + kind: 'node', + mocked: false, + interceptedRequests: [], + }, + ]); + expect(resultRepository.markAsCompleted).not.toHaveBeenCalled(); + return successExec() as never; + }, + ); + const { finished } = await service.startRun('ds-1', 'proj-1', user); + await finished; + expect(resultRepository.updateRunningToolCalls).toHaveBeenCalledWith( + expect.any(String), + runAt, + { calls: [expect.objectContaining({ status: 'running', toolCallId: 'call-1' })] }, + ); + expect(resultRepository.markAsCompleted).toHaveBeenCalledOnce(); + }); + it('snapshots input/expectedOutput/criteria onto the seeded case', async () => { seedFor([{ id: 'row-1', question: 'Q', answer: 'A', check: 'C' }], { success: 1 }); evalAgentExecutionService.executeWithLlmMock.mockResolvedValue(successExec() as never); @@ -1311,6 +1342,7 @@ describe('AgentEvalRunnerService', () => { user, { projectId: 'proj-1' }, 'What is 2+2?', + { onProgress: expect.any(Function) }, ); expect(resultRepository.markAsRunning).toHaveBeenCalledWith('res-1'); expect(resultRepository.markAsCompleted).toHaveBeenCalledWith( @@ -1376,6 +1408,7 @@ describe('AgentEvalRunnerService', () => { user, { projectId: 'proj-1' }, 'What is 2+2?', + { onProgress: expect.any(Function) }, ); }); diff --git a/packages/cli/src/modules/agent-evals/agent-eval-runner.service.ts b/packages/cli/src/modules/agent-evals/agent-eval-runner.service.ts index 504623e5087..1fe9d0c2a9e 100644 --- a/packages/cli/src/modules/agent-evals/agent-eval-runner.service.ts +++ b/packages/cli/src/modules/agent-evals/agent-eval-runner.service.ts @@ -610,13 +610,22 @@ export class AgentEvalRunnerService { ctx: { agentId: string; projectId: string; user: User; timeoutMs?: number }, ): Promise { try { - await this.resultRepository.markAsRunning(resultRow.id); + const runAt = await this.resultRepository.markAsRunning(resultRow.id); const execResult = await this.evalAgentExecutionService.executeWithLlmMock( ctx.agentId, ctx.user, { projectId: ctx.projectId, ...(ctx.timeoutMs ? { timeoutMs: ctx.timeoutMs } : {}) }, resolvedCase.input, + { + onProgress: async (calls) => { + await this.resultRepository.updateRunningToolCalls( + resultRow.id, + runAt, + toJsonObject({ calls }), + ); + }, + }, ); const usage = normalizeUsage(execResult.usage); diff --git a/packages/cli/src/modules/instance-ai/eval/__tests__/agent-execution.service.test.ts b/packages/cli/src/modules/instance-ai/eval/__tests__/agent-execution.service.test.ts index e873bec3ebb..54dbccda43a 100644 --- a/packages/cli/src/modules/instance-ai/eval/__tests__/agent-execution.service.test.ts +++ b/packages/cli/src/modules/instance-ai/eval/__tests__/agent-execution.service.test.ts @@ -1,4 +1,4 @@ -import type { GenerateResult } from '@n8n/agents'; +import { AgentEvent, type AgentEventHandler, type GenerateResult } from '@n8n/agents'; import type { AgentJsonConfig } from '@n8n/api-types'; import type { Logger, ModuleRegistry } from '@n8n/backend-common'; import type { OutboundHttp } from '@n8n/backend-network'; @@ -160,6 +160,179 @@ describe('EvalAgentExecutionService.executeWithLlmMock', () => { const request = { projectId: 'proj-1' }; + it('reports tool progress before generation finishes and flushes writes before returning', async () => { + const handlers = new Map(); + const onProgress = vi.fn().mockResolvedValue(undefined); + let finish: (value: GenerateResult) => void = () => {}; + const generate = vi.fn(async () => { + handlers.get(AgentEvent.ToolExecutionStart)?.({ + type: AgentEvent.ToolExecutionStart, + toolCallId: 'send-1', + toolName: 'Slack_Tool', + args: { text: 'hello' }, + }); + handlers.get(AgentEvent.ToolExecutionEnd)?.({ + type: AgentEvent.ToolExecutionEnd, + toolCallId: 'send-1', + toolName: 'Slack_Tool', + result: { sent: true }, + isError: false, + }); + handlers.get(AgentEvent.ToolExecutionStart)?.({ + type: AgentEvent.ToolExecutionStart, + toolCallId: 'wait-1', + toolName: 'wait_for_reply', + args: {}, + }); + return await new Promise((resolve) => { + finish = resolve; + }); + }); + reconstructFromAgentEntity.mockResolvedValue({ + agent: { + generate, + close: vi.fn(), + on: (event: AgentEvent, handler: AgentEventHandler) => handlers.set(event, handler), + }, + toolRegistry: {}, + }); + let settled = false; + const running = buildService() + .executeWithLlmMock('agent-1', user, request, 'Send a message', { onProgress }) + .then((value) => { + settled = true; + return value; + }); + await vi.waitFor(() => expect(onProgress).toHaveBeenCalledTimes(3)); + expect(settled).toBe(false); + expect(onProgress.mock.calls[2][0]).toEqual([ + expect.objectContaining({ + toolCallId: 'send-1', + status: 'completed', + output: { sent: true }, + }), + expect.objectContaining({ toolCallId: 'wait-1', status: 'running' }), + ]); + finish(makeGenerateResult()); + await running; + }); + + it('serializes progress writes and waits for the last write without failing the run', async () => { + const handlers = new Map(); + let releaseWrite: () => void = () => {}; + const onProgress = vi + .fn() + .mockImplementationOnce( + async () => + await new Promise((resolve) => { + releaseWrite = resolve; + }), + ) + .mockRejectedValueOnce(new Error('progress unavailable')) + .mockResolvedValue(undefined); + const generate = vi.fn(async () => { + handlers.get(AgentEvent.ToolExecutionStart)?.({ + type: AgentEvent.ToolExecutionStart, + toolCallId: 'call-1', + toolName: 'Slack_Tool', + args: {}, + }); + handlers.get(AgentEvent.ToolExecutionEnd)?.({ + type: AgentEvent.ToolExecutionEnd, + toolCallId: 'call-1', + toolName: 'Slack_Tool', + result: { sent: true }, + isError: false, + }); + return makeGenerateResult(); + }); + reconstructFromAgentEntity.mockResolvedValue({ + agent: { + generate, + close: vi.fn(), + on: (event: AgentEvent, handler: AgentEventHandler) => handlers.set(event, handler), + }, + toolRegistry: {}, + }); + let settled = false; + const running = buildService() + .executeWithLlmMock('agent-1', user, request, 'Send a message', { onProgress }) + .then((value) => { + settled = true; + return value; + }); + await vi.waitFor(() => expect(onProgress).toHaveBeenCalledOnce()); + expect(settled).toBe(false); + releaseWrite(); + expect((await running).success).toBe(true); + expect(onProgress).toHaveBeenCalledTimes(3); + handlers.get(AgentEvent.ToolExecutionStart)?.({ + type: AgentEvent.ToolExecutionStart, + toolCallId: 'late', + toolName: 'Slack_Tool', + args: {}, + }); + await Promise.resolve(); + expect(onProgress).toHaveBeenCalledTimes(3); + }); + + it('flushes failed and interrupted tool progress before returning a generation error', async () => { + const handlers = new Map(); + const onProgress = vi.fn().mockResolvedValue(undefined); + const close = vi.fn(); + reconstructFromAgentEntity.mockResolvedValue({ + agent: { + generate: vi.fn(async () => { + for (const toolCallId of ['failed', 'interrupted']) { + handlers.get(AgentEvent.ToolExecutionStart)?.({ + type: AgentEvent.ToolExecutionStart, + toolCallId, + toolName: 'Slack_Tool', + args: {}, + }); + } + handlers.get(AgentEvent.ToolExecutionEnd)?.({ + type: AgentEvent.ToolExecutionEnd, + toolCallId: 'failed', + toolName: 'Slack_Tool', + result: 'Tool failed', + isError: true, + }); + throw new Error('Generation stopped'); + }), + close, + on: (event: AgentEvent, handler: AgentEventHandler) => handlers.set(event, handler), + }, + toolRegistry: {}, + }); + const result = await buildService().executeWithLlmMock( + 'agent-1', + user, + request, + 'Send a message', + { onProgress }, + ); + expect(result.success).toBe(false); + expect(result.errors).toContain('Agent run failed: Generation stopped'); + expect(onProgress).toHaveBeenLastCalledWith([ + expect.objectContaining({ + toolCallId: 'failed', + status: 'error', + error: 'tool_execution_failed', + output: 'Tool failed', + }), + expect.objectContaining({ + toolCallId: 'interrupted', + status: 'error', + error: 'execution_stopped', + }), + ]); + expect(close).toHaveBeenCalledOnce(); + expect(onProgress.mock.invocationCallOrder.at(-1)).toBeLessThan( + close.mock.invocationCallOrder[0], + ); + }); + it('refuses queue mode upfront', async () => { const result = await buildService({ queueMode: true }).executeWithLlmMock( 'agent-1', diff --git a/packages/cli/src/modules/instance-ai/eval/agent-execution.service.ts b/packages/cli/src/modules/instance-ai/eval/agent-execution.service.ts index 9fb89bcbe77..f2654aa04c8 100644 --- a/packages/cli/src/modules/instance-ai/eval/agent-execution.service.ts +++ b/packages/cli/src/modules/instance-ai/eval/agent-execution.service.ts @@ -1,4 +1,5 @@ import { + AgentEvent, sanitizeToolName as sanitizeMcpToolName, type Agent as RuntimeAgent, type GenerateResult, @@ -166,6 +167,7 @@ export class EvalAgentExecutionService { // per-tool hints keep the tool mocks coherent), but the message the agent // receives is this input verbatim — how a fixed eval case is executed. caseInput?: string, + hooks?: { onProgress?: (calls: InstanceAiEvalAgentToolCallRecord[]) => Promise }, ): Promise { // Workflow-tool sub-executions carry a configureAdditionalData closure // that doesn't survive queue serialization — refuse upfront so tool @@ -408,6 +410,53 @@ export class EvalAgentExecutionService { return this.errorResult(`Failed to build agent runtime: ${message}`, seed, skippedFeatures); } + // Event handlers cannot await writes. Serialize snapshots and drain them before + // returning, so progress cannot overwrite completion or a later attempt. + const progressCalls = new Map(); + let progressWrites = Promise.resolve(); + let progressClosed = false; + const publishProgress = () => { + if (progressClosed || !hooks?.onProgress) return; + const calls = [...progressCalls.values()].map((call) => ({ ...call })); + progressWrites = progressWrites + .then(async () => await hooks.onProgress?.(calls)) + .catch((error: unknown) => { + this.logger.warn('[EvalAgentMock] Could not record tool progress', { error }); + }); + }; + if (hooks?.onProgress) { + agent.on(AgentEvent.ToolExecutionStart, (event) => { + if (progressClosed || event.type !== AgentEvent.ToolExecutionStart) return; + progressCalls.set(event.toolCallId, { + toolCallId: event.toolCallId, + tool: event.toolName, + kind: toolSummaries.find((tool) => tool.name === event.toolName)?.kind ?? 'other', + input: truncateRecordedValue(event.args), + status: 'running', + mocked: false, + interceptedRequests: [], + }); + publishProgress(); + }); + agent.on(AgentEvent.ToolExecutionEnd, (event) => { + if (progressClosed || event.type !== AgentEvent.ToolExecutionEnd) return; + const previous = progressCalls.get(event.toolCallId); + const interceptedRequests = toolLedger.get(event.toolName) ?? []; + progressCalls.set(event.toolCallId, { + ...previous, + toolCallId: event.toolCallId, + tool: event.toolName, + kind: previous?.kind ?? 'other', + output: truncateRecordedValue(event.result), + status: event.isError ? 'error' : 'completed', + ...(event.isError ? { error: 'tool_execution_failed' } : {}), + mocked: interceptedRequests.length > 0, + interceptedRequests: [...interceptedRequests], + }); + publishProgress(); + }); + } + const errors: string[] = []; // GenerateResult.toolCalls carry no toolCallId — approvals are attributed // by tool name, enough to flag auto-approval for the judge. @@ -479,6 +528,13 @@ export class EvalAgentExecutionService { credentialHelpers, }); } finally { + for (const [id, call] of progressCalls) { + if (call.status === 'running') + progressCalls.set(id, { ...call, status: 'error', error: 'execution_stopped' }); + } + if (progressCalls.size > 0) publishProgress(); + progressClosed = true; + await progressWrites; await recorder.flush(); try { await agent.close(); diff --git a/packages/frontend/@n8n/i18n/src/locales/en.json b/packages/frontend/@n8n/i18n/src/locales/en.json index 704b610a44a..adbccda1b05 100644 --- a/packages/frontend/@n8n/i18n/src/locales/en.json +++ b/packages/frontend/@n8n/i18n/src/locales/en.json @@ -9170,6 +9170,7 @@ "agents.builder.agentEvals.checks.addCheckMore": "+{count} more", "agents.builder.agentEvals.checks.addCheckFewer": "Show fewer", "agents.builder.agentEvals.checks.status.passes": "Passes", + "agents.builder.agentEvals.checks.waitingForResponse": "Waiting for the agent’s response…", "agents.builder.agentEvals.checks.status.running": "Running", "agents.builder.agentEvals.checks.status.neverRan": "Never ran", "agents.builder.agentEvals.checks.status.needsWork": "Needs work", diff --git a/packages/frontend/editor-ui/src/features/agents/__tests__/AgentEvalChecksPanel.test.ts b/packages/frontend/editor-ui/src/features/agents/__tests__/AgentEvalChecksPanel.test.ts index 47933e0285c..cf6b12a2a44 100644 --- a/packages/frontend/editor-ui/src/features/agents/__tests__/AgentEvalChecksPanel.test.ts +++ b/packages/frontend/editor-ui/src/features/agents/__tests__/AgentEvalChecksPanel.test.ts @@ -38,6 +38,7 @@ vi.mock('../components/AgentEvalTryRow.vue', () => ({ status: {}, input: {}, output: {}, + toolCalls: {}, label: {}, testId: {}, disabled: { type: Boolean }, @@ -61,6 +62,8 @@ vi.mock('../components/AgentEvalTryRow.vue', () => ({ template: `
{ }); }); + it('passes current tool progress to a running check without an answer', () => { + const { getByTestId } = render( + { + results: [ + { + ...result('c1', 'running'), + output: null, + toolCalls: { + calls: [{ tool: 'wait_for_reply', toolCallId: 'call-1', status: 'running' }], + }, + }, + ], + }, + true, + ); + const row = getByTestId('agent-eval-check-c1'); + expect(row).toHaveAttribute('data-status', 'waiting'); + expect(row).toHaveAttribute('data-tool-status', 'running'); + expect(row).toHaveAttribute('data-tool-id', 'call-1'); + }); + it('disables "Run all checks" while a run is already in flight', () => { const { getByTestId } = render({ results: [result('c1', 'running')] }, true); diff --git a/packages/frontend/editor-ui/src/features/agents/components/AgentEvalTryRow.vue b/packages/frontend/editor-ui/src/features/agents/components/AgentEvalTryRow.vue index b1e2b948e54..ba9cafb67b9 100644 --- a/packages/frontend/editor-ui/src/features/agents/components/AgentEvalTryRow.vue +++ b/packages/frontend/editor-ui/src/features/agents/components/AgentEvalTryRow.vue @@ -1,13 +1,7 @@ diff --git a/packages/frontend/editor-ui/src/features/agents/components/AgentEvalTryRow.vue b/packages/frontend/editor-ui/src/features/agents/components/AgentEvalTryRow.vue index ba9cafb67b9..8de4c5794ec 100644 --- a/packages/frontend/editor-ui/src/features/agents/components/AgentEvalTryRow.vue +++ b/packages/frontend/editor-ui/src/features/agents/components/AgentEvalTryRow.vue @@ -258,7 +258,6 @@ watch( type="button" :class="$style.expandToggle" :data-test-id="testId && `${testId}-toggle`" - :aria-expanded="view === 'complete' ? expanded : undefined" :aria-label="i18n.baseText('instanceAi.testAgentPreview.conversation')" > diff --git a/packages/frontend/editor-ui/src/features/agents/utils/__tests__/agent-eval-tool-calls.test.ts b/packages/frontend/editor-ui/src/features/agents/utils/__tests__/agent-eval-tool-calls.test.ts index 8672bc2c2c1..256a12bd671 100644 --- a/packages/frontend/editor-ui/src/features/agents/utils/__tests__/agent-eval-tool-calls.test.ts +++ b/packages/frontend/editor-ui/src/features/agents/utils/__tests__/agent-eval-tool-calls.test.ts @@ -106,3 +106,27 @@ it('preserves live call identity and running state', () => { }); expect(calls[0]).toMatchObject({ toolCallId: 'call-1', state: 'running' }); }); + +it('preserves recorded mock requests without treating them as completed output', () => { + const request = { + url: 'https://slack.com/api/chat.postMessage', + method: 'POST', + nodeType: 'n8n-nodes-base.slack', + requestBody: { text: 'Mock update' }, + mockResponse: { ok: true }, + }; + const [call] = toDisplayToolCalls({ + calls: [ + { tool: 'send_and_wait', status: 'running', mocked: true, interceptedRequests: [request] }, + ], + }); + expect(call).toMatchObject({ mocked: true, interceptedRequests: [request], state: 'running' }); + expect(call.output).toBeUndefined(); +}); + +it('filters malformed intercepted requests without discarding the tool call', () => { + const [call] = toDisplayToolCalls({ + calls: [{ tool: 'lookup', mocked: 'yes', interceptedRequests: [null, {}, { url: 42 }] }], + }); + expect(call).toMatchObject({ mocked: false, interceptedRequests: [] }); +}); diff --git a/packages/frontend/editor-ui/src/features/agents/utils/agent-eval-tool-calls.ts b/packages/frontend/editor-ui/src/features/agents/utils/agent-eval-tool-calls.ts index 4c485548696..3dd9b82b886 100644 --- a/packages/frontend/editor-ui/src/features/agents/utils/agent-eval-tool-calls.ts +++ b/packages/frontend/editor-ui/src/features/agents/utils/agent-eval-tool-calls.ts @@ -1,3 +1,4 @@ +import type { InstanceAiEvalInterceptedRequest } from '@n8n/api-types'; import { isRecord } from '@n8n/utils/is-record'; import { isUnknownArray } from '@n8n/utils/is-unknown-array'; @@ -11,14 +12,27 @@ const CANCELED_ERROR = 'canceled'; /** * Only the fields the review view renders. Narrowing to the full record would - * reject rows written before a field existed, for no gain — the extra fields - * (`kind`, `mocked`, `interceptedRequests`) have no bearing on the disclosure. + * reject rows written before a field existed. */ type PersistedToolCall = Pick< InstanceAiEvalAgentToolCallRecord, - 'tool' | 'input' | 'output' | 'error' | 'toolCallId' | 'status' + 'tool' | 'input' | 'output' | 'error' | 'toolCallId' | 'status' | 'mocked' | 'interceptedRequests' >; +export type EvalDisplayToolCall = ToolCall & { + mocked?: boolean; + interceptedRequests?: InstanceAiEvalInterceptedRequest[]; +}; + +function isInterceptedRequest(value: unknown): value is InstanceAiEvalInterceptedRequest { + return ( + isRecord(value) && + typeof value.url === 'string' && + typeof value.method === 'string' && + typeof value.nodeType === 'string' + ); +} + function isPersistedToolCall(value: unknown): value is PersistedToolCall { return isRecord(value) && typeof value.tool === 'string'; } @@ -48,7 +62,7 @@ function resolveState( * says nothing useful about its elements, and narrowing from `JsonValue` can't * reach a record shape whose `input`/`output` are themselves opaque. */ -export function toDisplayToolCalls(toolCalls: unknown): ToolCall[] { +export function toDisplayToolCalls(toolCalls: unknown): EvalDisplayToolCall[] { if (!isRecord(toolCalls) || !isUnknownArray(toolCalls.calls)) return []; return toolCalls.calls.filter(isPersistedToolCall).map((call, index) => ({ @@ -58,6 +72,10 @@ export function toDisplayToolCalls(toolCalls: unknown): ToolCall[] { input: call.input, output: call.output, state: resolveState(call.error, call.status), + mocked: call.mocked === true, + interceptedRequests: isUnknownArray(call.interceptedRequests) + ? call.interceptedRequests.filter(isInterceptedRequest) + : [], // `error` is deliberately not forwarded as `output`: it can be an // internal diagnostic about unattributed requests or interrupted execution. Leaving `output` empty // makes the renderer fall back to its generic tool-error copy, which is From 0abc7f7b854ca3ea62e2d740e57ce3f2727ce5b5 Mon Sep 17 00:00:00 2001 From: Luka Zivkovic Date: Thu, 15 Oct 2026 01:13:50 +0200 Subject: [PATCH 06/18] fix(editor): Allow check changes during active agent eval runs --- .../frontend/@n8n/i18n/src/locales/en.json | 2 + .../__tests__/AgentEvalAddCheckPanel.test.ts | 29 +++++++- .../__tests__/AgentEvalChecksPanel.test.ts | 66 ++++++++++++++++--- .../components/AgentEvalAddCheckPanel.vue | 21 ++++-- .../components/AgentEvalChecksPanel.vue | 60 +++++++++++++++-- .../InstanceAiTestAgentExamplesPanel.test.ts | 10 ++- .../InstanceAiTestAgentExamplesPanel.vue | 12 +++- 7 files changed, 173 insertions(+), 27 deletions(-) diff --git a/packages/frontend/@n8n/i18n/src/locales/en.json b/packages/frontend/@n8n/i18n/src/locales/en.json index e60780d2ed1..3711d8dd3cf 100644 --- a/packages/frontend/@n8n/i18n/src/locales/en.json +++ b/packages/frontend/@n8n/i18n/src/locales/en.json @@ -9172,6 +9172,8 @@ "agents.builder.agentEvals.checks.status.passes": "Passes", "agents.builder.agentEvals.checks.mockedRequestRunning": "Mocked request sent; tool still running | Mocked requests sent; tool still running", "agents.builder.agentEvals.checks.mockedRequests": "Mocked requests", + "agents.builder.agentEvals.checks.manageChecks": "Manage checks", + "agents.builder.agentEvals.checks.savedForNextRun": "{count} added check is saved for the next run. | {count} added check is saved for the next run. | {count} added checks are saved for the next run.", "agents.builder.agentEvals.checks.waitingForResponse": "Waiting for the agent’s response…", "agents.builder.agentEvals.checks.status.running": "Running", "agents.builder.agentEvals.checks.status.neverRan": "Never ran", diff --git a/packages/frontend/editor-ui/src/features/agents/__tests__/AgentEvalAddCheckPanel.test.ts b/packages/frontend/editor-ui/src/features/agents/__tests__/AgentEvalAddCheckPanel.test.ts index 481ffa81838..755b697e921 100644 --- a/packages/frontend/editor-ui/src/features/agents/__tests__/AgentEvalAddCheckPanel.test.ts +++ b/packages/frontend/editor-ui/src/features/agents/__tests__/AgentEvalAddCheckPanel.test.ts @@ -28,7 +28,12 @@ const draft = (index: number): AgentEvalDraftCase => ({ scenario: `Scenario ${index}`, }); -type Props = { disabled?: boolean; busy?: boolean; caseSource?: typeof caseSource | null }; +type Props = { + deferRun?: boolean; + disabled?: boolean; + busy?: boolean; + caseSource?: typeof caseSource | null; +}; const renderPanel = ( props: Props = {}, @@ -63,6 +68,7 @@ const renderPanel = ( caseSource: 'caseSource' in props ? props.caseSource : caseSource, disabled: props.disabled, busy: props.busy, + deferRun: props.deferRun, }, })({ pinia }); @@ -74,6 +80,27 @@ describe('AgentEvalAddCheckPanel', () => { vi.clearAllMocks(); }); + it.each([true, false])( + 'defers additions if a run is active at either end of the save (initial: %s)', + async (initiallyRunning) => { + const user = userEvent.setup(); + const { store, findAllByTestId, rerender, emitted } = renderPanel({ + deferRun: initiallyRunning, + }); + let save!: () => void; + vi.mocked(store.createCase).mockImplementation(async () => { + await new Promise((resolve) => { + save = resolve; + }); + return { rowId: 9, input: 'new message', whatToCheck: 'new rule' }; + }); + await user.click((await findAllByTestId('agent-eval-add-check-prepared-add'))[0]); + await rerender({ deferRun: !initiallyRunning }); + save(); + await waitFor(() => expect(emitted('added')).toEqual([[true]])); + }, + ); + describe('the prepared cases', () => { it('drafts a batch of ten without saving anything when it opens', async () => { const { store, findAllByTestId } = renderPanel(); diff --git a/packages/frontend/editor-ui/src/features/agents/__tests__/AgentEvalChecksPanel.test.ts b/packages/frontend/editor-ui/src/features/agents/__tests__/AgentEvalChecksPanel.test.ts index cf6b12a2a44..88a10cd9b9b 100644 --- a/packages/frontend/editor-ui/src/features/agents/__tests__/AgentEvalChecksPanel.test.ts +++ b/packages/frontend/editor-ui/src/features/agents/__tests__/AgentEvalChecksPanel.test.ts @@ -105,6 +105,7 @@ vi.mock('../components/AgentEvalAddCheckPanel.vue', () => ({ > +
`, }, })); @@ -722,6 +723,29 @@ describe('AgentEvalChecksPanel', () => { expect(store.applySuggestions).toHaveBeenCalledWith('project-1', 'agent-1', ['c1', 'c3']); }); + it('applies settled suggestions during a batch and leaves running checks out', async () => { + const user = userEvent.setup(); + const order: string[] = []; + const { getByTestId, store } = render( + { + results: [ + failedWith('c1', 'Be brief'), + { ...failedWith('c2', 'Wait'), status: 'running' }, + ], + }, + true, + builderWrite(order), + ); + vi.mocked(store.applySuggestions).mockImplementation(async () => { + order.push('apply'); + return { configHash: 'h', results: [] }; + }); + expect(getByTestId(BUTTON)).toBeEnabled(); + await user.click(getByTestId(BUTTON)); + await vi.waitFor(() => expect(order).toEqual(['lock', 'apply', 'unlock'])); + expect(store.applySuggestions).toHaveBeenCalledWith('project-1', 'agent-1', ['c1']); + }); + it('sends a long list as successive requests of at most one request’s worth', async () => { const user = userEvent.setup(); const many = Array.from({ length: MAX_APPLY_SUGGESTIONS + 2 }, (_, i) => @@ -957,6 +981,30 @@ describe('AgentEvalChecksPanel', () => { expect(addCheckPanelMounts.count).toBe(1); }); + it('saves additions for the next run while keeping the active batch visible', async () => { + const user = userEvent.setup(); + const { getByTestId, getByText, emitted } = renderWithDataset({ inFlight: true }); + await user.click(getByTestId('agent-eval-checks-add-check')); + expect(getByTestId('add-check-panel-stub')).toHaveAttribute('data-busy', 'false'); + await user.click(getByText('added a check')); + expect(emitted('rerun')).toBeUndefined(); + expect(getByTestId('agent-eval-check-c1')).toBeVisible(); + expect(getByTestId('agent-eval-checks-saved-next-run')).toHaveTextContent( + '1 added check is saved for the next run.', + ); + await user.click(getByText('close panel')); + expect(getByTestId('agent-eval-checks-saved-next-run')).toBeVisible(); + }); + + it('keeps an addition deferred after the original run finishes', async () => { + const user = userEvent.setup(); + const { getByTestId, getByText, emitted } = renderWithDataset(); + await user.click(getByTestId('agent-eval-checks-add-check')); + await user.click(getByText('added after run finished')); + expect(emitted('rerun')).toBeUndefined(); + expect(getByTestId('agent-eval-checks-saved-next-run')).toBeVisible(); + }); + it('runs the checks again once a check was added, since a run only holds the cases it started with', async () => { const user = userEvent.setup(); const { getByTestId, getByText, emitted } = renderWithDataset(); @@ -1028,17 +1076,17 @@ describe('AgentEvalChecksPanel', () => { }); }); - it.each([ - ['a rerun is starting', { rerunning: true }], - ['a run is in flight', { inFlight: true }], - ])('tells the panel it is busy while %s', async (_label, options) => { - const user = userEvent.setup(); - const { getByTestId } = renderWithDataset(options); + it.each([['a rerun is starting', { rerunning: true }]])( + 'tells the panel it is busy while %s', + async (_label, options) => { + const user = userEvent.setup(); + const { getByTestId } = renderWithDataset(options); - await user.click(getByTestId('agent-eval-checks-add-check')); + await user.click(getByTestId('agent-eval-checks-add-check')); - expect(getByTestId('add-check-panel-stub')).toHaveAttribute('data-busy', 'true'); - }); + expect(getByTestId('add-check-panel-stub')).toHaveAttribute('data-busy', 'true'); + }, + ); it('disables the button and the panel for a read-only viewer', async () => { const { getByTestId } = renderWithDataset({ disabled: true }); diff --git a/packages/frontend/editor-ui/src/features/agents/components/AgentEvalAddCheckPanel.vue b/packages/frontend/editor-ui/src/features/agents/components/AgentEvalAddCheckPanel.vue index a80846ec801..e09e5462792 100644 --- a/packages/frontend/editor-ui/src/features/agents/components/AgentEvalAddCheckPanel.vue +++ b/packages/frontend/editor-ui/src/features/agents/components/AgentEvalAddCheckPanel.vue @@ -22,14 +22,16 @@ const props = defineProps<{ caseSource: AgentEvalCaseSource | null; /** No `agent:update` — nothing can be added. */ disabled?: boolean; - /** The checks are starting or running, so another add would start a second run. */ + /** A replacement run is starting, so its dataset is not ready for another write. */ busy?: boolean; + /** Save new cases for the next run while this run keeps its original cases. */ + deferRun?: boolean; }>(); const emit = defineEmits<{ close: []; /** A case was written to the dataset. */ - added: []; + added: [deferRun: boolean]; }>(); const i18n = useI18n(); @@ -85,8 +87,11 @@ onBeforeUnmount(() => { isMounted = false; }); -/** Writes the case to the dataset, then tells the parent to run the checks again. */ -async function writeCase(value: { input: string; whatToCheck: string }): Promise { +/** Reports the saved case and whether it must wait for the next run. */ +async function writeCase( + value: { input: string; whatToCheck: string }, + deferRun: boolean, +): Promise { const source = props.caseSource; if (!source) return false; try { @@ -97,7 +102,7 @@ async function writeCase(value: { input: string; whatToCheck: string }): Promise if (isMounted) toast.showError(error, i18n.baseText('agents.builder.agentEvals.case.addError')); return false; } - emit('added'); + emit('added', deferRun || Boolean(props.deferRun)); return true; } @@ -105,6 +110,7 @@ async function onAddRule() { const rule = ruleInput.value.trim(); if (!rule || !canAdd.value) return; + const deferRun = Boolean(props.deferRun); adding.value = 'rule'; try { let input: string; @@ -124,7 +130,7 @@ async function onAddRule() { } if (!isMounted) return; - if (await writeCase({ input, whatToCheck: rule })) ruleInput.value = ''; + if (await writeCase({ input, whatToCheck: rule }, deferRun)) ruleInput.value = ''; } finally { if (isMounted) adding.value = null; } @@ -133,9 +139,10 @@ async function onAddRule() { async function onAddPrepared(row: PreparedCase) { if (!canAdd.value) return; + const deferRun = Boolean(props.deferRun); adding.value = row.key; try { - const written = await writeCase({ input: row.input, whatToCheck: row.whatToCheck }); + const written = await writeCase({ input: row.input, whatToCheck: row.whatToCheck }, deferRun); if (written && isMounted) { prepared.value = prepared.value.filter((candidate) => candidate.key !== row.key); } diff --git a/packages/frontend/editor-ui/src/features/agents/components/AgentEvalChecksPanel.vue b/packages/frontend/editor-ui/src/features/agents/components/AgentEvalChecksPanel.vue index b6f687c28c6..4d276c93c08 100644 --- a/packages/frontend/editor-ui/src/features/agents/components/AgentEvalChecksPanel.vue +++ b/packages/frontend/editor-ui/src/features/agents/components/AgentEvalChecksPanel.vue @@ -10,7 +10,7 @@ * never passes `label` to `AgentEvalTryRow`. */ import { computed, onBeforeUnmount, onMounted, ref, watch } from 'vue'; -import { N8nButton, N8nIcon } from '@n8n/design-system'; +import { N8nButton, N8nIcon, N8nText } from '@n8n/design-system'; import { useToast } from '@n8n/composables/useToast'; import { useI18n, type BaseTextKey } from '@n8n/i18n'; @@ -209,7 +209,18 @@ watch( { immediate: true }, ); +function canChangeResult(resultId: string) { + return ( + !props.disabled && + !props.rerunning && + !showingPreviousRun.value && + !isApplyingSuggestions.value && + rows.value.some((row) => row.id === resultId && row.status !== 'waiting') + ); +} + async function onActuallyFine(resultId: string) { + if (!canChangeResult(resultId)) return; try { await store.acceptResult(props.projectId, props.agentId, resultId); } catch (error) { @@ -218,6 +229,7 @@ async function onActuallyFine(resultId: string) { } async function onRerunCheck(resultId: string) { + if (!canChangeResult(resultId)) return; try { await store.rerunResult(props.projectId, props.agentId, resultId); } catch (error) { @@ -243,10 +255,13 @@ const applicableSuggestionIds = computed(() => const isApplyingSuggestions = computed(() => applyingSuggestionIds.value.length > 0); async function onApplySuggestion(resultId: string) { + if (!canChangeResult(resultId)) return; await applySuggestions([resultId]); } async function onApplyAllSuggestions() { + if (props.disabled || props.rerunning || showingPreviousRun.value || isApplyingSuggestions.value) + return; applyingAll.value = true; try { await applySuggestions(applicableSuggestionIds.value); @@ -260,6 +275,7 @@ async function onApplyAllSuggestions() { // this result's snapshot. A result with no row to write to (an older run, or a dataset // with no rule column) keeps the snapshot-only edit. async function onSaveWhatToCheck(resultId: string, whatToCheck: string) { + if (!canChangeResult(resultId)) return; // `Number(null)` is 0, which would aim the write at a row that is not this check's. const sourceRowId = rows.value.find((row) => row.id === resultId)?.sourceRowId; const rowId = sourceRowId ? Number(sourceRowId) : Number.NaN; @@ -299,6 +315,21 @@ const addCheckSource = computed(() => { }); const addCheckOpen = ref(false); +const savedForNextRunCount = ref(0); +watch( + () => props.runId, + () => { + savedForNextRunCount.value = 0; + }, +); + +function onCheckAdded(deferRun = false) { + if (deferRun || inFlight.value || props.rerunning) { + savedForNextRunCount.value++; + return; + } + emit('rerun'); +} // Mounted on first open and kept after that, so closing and reopening the panel, // or a rerun that briefly empties the review, doesn't draft a new batch of // prepared cases. @@ -310,6 +341,7 @@ function toggleAddCheck() { } async function onDeleteCheck(row: CheckRow) { + if (!canChangeResult(row.id)) return; const source = resolveCaseSource(); if (!source || !row.sourceRowId) { toast.showError( @@ -474,7 +506,7 @@ onBeforeUnmount(store.stopPollingRun); v-if="applicableSuggestionIds.length > 0 && !showingPreviousRun" variant="solid" size="small" - :disabled="disabled || rerunning || inFlight || isApplyingSuggestions" + :disabled="disabled || rerunning || isApplyingSuggestions" :loading="applyingAll" data-testid="agent-eval-checks-apply-all-suggestions" @click="onApplyAllSuggestions" @@ -484,7 +516,7 @@ onBeforeUnmount(store.stopPollingRun); + + {{ + i18n.baseText('agents.builder.agentEvals.checks.savedForNextRun', { + adjustToNumber: savedForNextRunCount, + interpolate: { count: String(savedForNextRunCount) }, + }) + }} + + @@ -532,9 +578,10 @@ onBeforeUnmount(store.stopPollingRun); :agent-id="agentId" :case-source="addCheckSource" :disabled="disabled || showingPreviousRun" - :busy="rerunning || inFlight" + :busy="Boolean(rerunning)" + :defer-run="inFlight" @close="addCheckOpen = false" - @added="emit('rerun')" + @added="onCheckAdded" /> @@ -570,6 +617,7 @@ onBeforeUnmount(store.stopPollingRun); .header { display: flex; + flex-wrap: wrap; align-items: center; justify-content: space-between; gap: var(--spacing--sm); diff --git a/packages/frontend/editor-ui/src/features/ai/instanceAi/__tests__/InstanceAiTestAgentExamplesPanel.test.ts b/packages/frontend/editor-ui/src/features/ai/instanceAi/__tests__/InstanceAiTestAgentExamplesPanel.test.ts index 27bc8eb7005..ed2fda88012 100644 --- a/packages/frontend/editor-ui/src/features/ai/instanceAi/__tests__/InstanceAiTestAgentExamplesPanel.test.ts +++ b/packages/frontend/editor-ui/src/features/ai/instanceAi/__tests__/InstanceAiTestAgentExamplesPanel.test.ts @@ -540,12 +540,16 @@ describe('InstanceAiTestAgentExamplesPanel', () => { expect(getByTestId(BUTTON)).toBeDisabled(); }); - it('is hidden while the run is still going', () => { - const { queryByTestId } = renderComponent({ + it('applies settled suggestions and offers the checks view while another check is running', async () => { + const user = userEvent.setup(); + const { getByTestId, emitted } = renderComponent({ props: { caseRuns: [failed(1), failed(2), failed(3, { status: 'waiting' as const })] }, }); - expect(queryByTestId(BUTTON)).not.toBeInTheDocument(); + await user.click(getByTestId(BUTTON)); + expect(emitted('apply-suggestions')).toEqual([[['result-1', 'result-2']]]); + await user.click(getByTestId('instance-ai-test-agent-examples-manage-checks')); + expect(emitted('open-case')).toEqual([[null]]); }); }); diff --git a/packages/frontend/editor-ui/src/features/ai/instanceAi/components/InstanceAiTestAgentExamplesPanel.vue b/packages/frontend/editor-ui/src/features/ai/instanceAi/components/InstanceAiTestAgentExamplesPanel.vue index a5e8fd25890..87b45a35772 100644 --- a/packages/frontend/editor-ui/src/features/ai/instanceAi/components/InstanceAiTestAgentExamplesPanel.vue +++ b/packages/frontend/editor-ui/src/features/ai/instanceAi/components/InstanceAiTestAgentExamplesPanel.vue @@ -121,6 +121,7 @@ watch(applyingAny, (applying) => { }); function onApplyAllSuggestions() { + if (applyingAny.value) return; applyingAll.value = true; emit('apply-suggestions', applicableSuggestionIds.value); } @@ -290,7 +291,7 @@ function onCheckYourAgent() { + + {{ i18n.baseText('agents.builder.agentEvals.checks.manageChecks') }} + + Date: Thu, 15 Oct 2026 01:12:14 +0200 Subject: [PATCH 07/18] fix(core): Keep active agent eval batches on their starting config --- .../agent-eval-run.repository.ee.ts | 98 ++++++++--- ...nt-eval-runner.service.integration.test.ts | 99 +++++++++++ .../agent-eval-runner.service.test.ts | 150 ++++++++++++++++- .../agent-evals/agent-eval-runner.service.ts | 157 +++++++++++------- .../__tests__/agent-execution.service.test.ts | 73 ++++++++ .../eval/agent-execution.service.ts | 47 +++++- 6 files changed, 530 insertions(+), 94 deletions(-) diff --git a/packages/@n8n/db/src/repositories/agent-eval-run.repository.ee.ts b/packages/@n8n/db/src/repositories/agent-eval-run.repository.ee.ts index 43ae0896b0e..7fd99a697b5 100644 --- a/packages/@n8n/db/src/repositories/agent-eval-run.repository.ee.ts +++ b/packages/@n8n/db/src/repositories/agent-eval-run.repository.ee.ts @@ -1,8 +1,10 @@ import { Service } from '@n8n/di'; import { DataSource, In, Repository } from '@n8n/typeorm'; +import type { QueryDeepPartialEntity } from '@n8n/typeorm/query-builder/QueryPartialEntity'; import type { IDataObject } from 'n8n-workflow'; -import { AgentEvalRun } from '../entities'; +import { AgentEvalRun, AgentEvalResult } from '../entities'; +import type { AgentEvalResultStatusCounts } from './agent-eval-result.repository.ee'; type CreateAgentEvalRunAttrs = { datasetId: string; @@ -36,13 +38,21 @@ export class AgentEvalRunRepository extends Repository { }); } - async markAsCompleted(id: string, metrics: IDataObject | null) { - return await this.update(id, { - status: 'completed', - completedAt: new Date(), - metrics, - runningInstanceId: null, - }); + async markAsCompleted( + id: string, + metrics: IDataObject | null, + counts?: AgentEvalResultStatusCounts, + ) { + return await this.settleWithResultCounts( + id, + { + status: 'completed', + completedAt: new Date(), + metrics, + runningInstanceId: null, + }, + counts, + ); } /** @@ -54,15 +64,20 @@ export class AgentEvalRunRepository extends Repository { errorCode: string, errorDetails?: IDataObject | null, metrics?: IDataObject | null, + counts?: AgentEvalResultStatusCounts, ) { - return await this.update(id, { - status: 'error', - completedAt: new Date(), - errorCode, - errorDetails: errorDetails ?? null, - metrics: metrics ?? null, - runningInstanceId: null, - }); + return await this.settleWithResultCounts( + id, + { + status: 'error', + completedAt: new Date(), + errorCode, + errorDetails: errorDetails ?? null, + metrics: metrics ?? null, + runningInstanceId: null, + }, + counts, + ); } /** Rewrites only the recorded tally — used when a result is removed from a settled run. */ @@ -70,13 +85,50 @@ export class AgentEvalRunRepository extends Repository { return await this.update(id, { metrics }); } - async markAsCancelled(id: string, metrics: IDataObject | null = null) { - return await this.update(id, { - status: 'cancelled', - completedAt: new Date(), - metrics, - runningInstanceId: null, - }); + async markAsCancelled( + id: string, + metrics: IDataObject | null = null, + counts?: AgentEvalResultStatusCounts, + ) { + return await this.settleWithResultCounts( + id, + { + status: 'cancelled', + completedAt: new Date(), + metrics, + runningInstanceId: null, + }, + counts, + ); + } + + /** Reject stale aggregation if a result was claimed or settled after the read. */ + private async settleWithResultCounts( + id: string, + attributes: QueryDeepPartialEntity, + counts?: AgentEvalResultStatusCounts, + ) { + if (!counts) return await this.update(id, attributes); + const query = this.createQueryBuilder() + .update() + .set(attributes) + .where({ id, status: 'running' }); + if (attributes.status !== 'cancelled') query.andWhere({ cancelRequested: false }); + for (const status of ['new', 'running', 'success', 'error', 'cancelled'] as const) { + const countQuery = this.manager + .createQueryBuilder(AgentEvalResult, 'result') + .select('COUNT(*)') + .where('result.runId = :runId') + .andWhere(`result.status = :status_${status}`) + .getQuery(); + const expected = status === 'new' || status === 'running' ? 0 : counts[status]; + query.andWhere(`(${countQuery}) = :count_${status}`, { + runId: id, + [`status_${status}`]: status, + [`count_${status}`]: expected, + }); + } + return await query.execute(); } /** diff --git a/packages/cli/src/modules/agent-evals/__tests__/agent-eval-runner.service.integration.test.ts b/packages/cli/src/modules/agent-evals/__tests__/agent-eval-runner.service.integration.test.ts index 2f999f202ed..53832ad347d 100644 --- a/packages/cli/src/modules/agent-evals/__tests__/agent-eval-runner.service.integration.test.ts +++ b/packages/cli/src/modules/agent-evals/__tests__/agent-eval-runner.service.integration.test.ts @@ -111,6 +111,105 @@ afterAll(async () => { }); describe('AgentEvalRunnerService (integration)', () => { + it('rejects aggregation when a result starts or finishes a new attempt after the count read', async () => { + const project = await createTeamProject(); + const agent = await createAgent(project.id); + const dataset = await Container.get(AgentEvalDatasetRepository).createDataset({ + name: 'ds', + agentId: agent.id, + datasetSource: 'data_table', + datasetRef: { dataTableId: 'table' }, + columnMapping: { input: 'question' }, + }); + const runs = Container.get(AgentEvalRunRepository); + const results = Container.get(AgentEvalResultRepository); + const run = await runs.createRun({ datasetId: dataset.id }); + await runs.markAsRunning(run.id); + const [result] = await results.seedResults([{ runId: run.id }]); + await results.markAsError(result.id, 'execution_failed'); + const oldCounts = await results.countByStatus(run.id); + expect(await results.claimForRerun(result.id)).toBe(true); + const oldMetrics = { total: 1, pending: 0, success: 0, error: 1, cancelled: 0 }; + expect((await runs.markAsCompleted(run.id, oldMetrics, oldCounts)).affected).toBe(0); + await results.markAsCompleted(result.id, { output: { finalText: 'Updated answer' } }); + expect((await runs.markAsCompleted(run.id, oldMetrics, oldCounts)).affected).toBe(0); + const currentCounts = await results.countByStatus(run.id); + const currentMetrics = { ...oldMetrics, success: 1, error: 0 }; + await runs.requestCancellation(run.id); + expect((await runs.markAsCompleted(run.id, currentMetrics, currentCounts)).affected).toBe(0); + expect((await runs.markAsCancelled(run.id, currentMetrics, currentCounts)).affected).toBe(1); + expect(await runs.findById(run.id)).toMatchObject({ + status: 'cancelled', + metrics: currentMetrics, + }); + }); + + it('keeps an original batch running until a targeted rerun has settled', async () => { + const project = await createTeamProject(); + const agent = await createAgent(project.id); + const dataTables = Container.get(DataTableService); + const table = await dataTables.createDataTable(project.id, { + name: 'cases', + columns: [{ name: 'question', type: 'string' }], + }); + await dataTables.insertRows(table.id, project.id, [ + { question: 'First' }, + { question: 'Second' }, + ]); + const dataset = await Container.get(AgentEvalDatasetRepository).createDataset({ + name: 'ds', + agentId: agent.id, + datasetSource: 'data_table', + datasetRef: { dataTableId: table.id }, + columnMapping: { input: 'question' }, + }); + agentRepository.findByIdAndProjectId.mockResolvedValue(agent); + let finishOriginal!: () => void; + let finishRerun!: () => void; + const original = new Promise((resolve) => { + finishOriginal = resolve; + }); + const rerun = new Promise((resolve) => { + finishRerun = resolve; + }); + let firstAttempts = 0; + evalAgentExecutionService.executeWithLlmMock.mockImplementation( + async (_id, _user, _options, input) => { + if (input === 'First' && ++firstAttempts > 1) await rerun; + if (input === 'Second') await original; + return { success: true, finalText: 'Answer', toolCalls: [], skippedFeatures: [] } as never; + }, + ); + const runner = buildRunner(); + const { runId, finished } = await runner.startRun(dataset.id, project.id, owner); + const results = Container.get(AgentEvalResultRepository); + let firstResult; + await vi.waitFor(async () => { + const [rows] = await results.findAndCountByRunId(runId); + firstResult = rows.find((row) => row.input?.input === 'First'); + expect(firstResult?.status).toBe('success'); + }); + if (!firstResult) throw new Error('Missing first result'); + const rerunFinished = runner.rerunResult(firstResult, agent.id, project.id, owner); + await vi.waitFor(() => expect(firstAttempts).toBe(2)); + finishOriginal(); + await vi.waitFor(async () => { + expect(await results.countByStatus(runId)).toMatchObject({ success: 1, running: 1 }); + }); + expect((await Container.get(AgentEvalRunRepository).findById(runId))?.status).toBe('running'); + finishRerun(); + await Promise.all([rerunFinished, finished]); + expect(await runner.getRunSummary(runId, agent.id)).toMatchObject({ + status: 'completed', + counts: { total: 2, success: 2, pending: 0 }, + }); + expect((await Container.get(AgentEvalRunRepository).findById(runId))?.metrics).toMatchObject({ + total: 2, + success: 2, + pending: 0, + }); + }); + it('runs each dataset row against the agent and persists results + run status', async () => { const project = await createTeamProject(); const agent = await createAgent(project.id); diff --git a/packages/cli/src/modules/agent-evals/__tests__/agent-eval-runner.service.test.ts b/packages/cli/src/modules/agent-evals/__tests__/agent-eval-runner.service.test.ts index 8af351a9973..1881030fae9 100644 --- a/packages/cli/src/modules/agent-evals/__tests__/agent-eval-runner.service.test.ts +++ b/packages/cli/src/modules/agent-evals/__tests__/agent-eval-runner.service.test.ts @@ -164,6 +164,10 @@ describe('AgentEvalRunnerService', () => { moduleRegistry.isActive.mockReturnValue(true); datasetRepository = mock(); runRepository = mock(); + const updated = { affected: 1, raw: [], generatedMaps: [] }; + runRepository.markAsCompleted.mockResolvedValue(updated); + runRepository.markAsCancelled.mockResolvedValue(updated); + runRepository.markAsError.mockResolvedValue(updated); resultRepository = mock(); agentRepository = mock(); dataTableService = mock(); @@ -341,6 +345,120 @@ describe('AgentEvalRunnerService', () => { }); describe('running cases', () => { + it('keeps one config snapshot when instructions change with a queued case', async () => { + seedFor( + [ + { id: 'row-1', question: 'First' }, + { id: 'row-2', question: 'Second' }, + ], + { success: 2 }, + ); + let releaseFirst!: () => void; + const first = new Promise((resolve) => { + releaseFirst = resolve; + }); + const source = { agent: mock({ id: 'agent-1' }), skillRefs: [], skills: {} }; + evalAgentExecutionService.captureRunSource.mockResolvedValue(source); + evalAgentExecutionService.executeWithLlmMock + .mockImplementationOnce(async () => { + await first; + return successExec() as never; + }) + .mockResolvedValue(successExec() as never); + const { finished } = await service.startRun('ds-1', 'proj-1', user); + await vi.waitFor(() => + expect(evalAgentExecutionService.executeWithLlmMock).toHaveBeenCalledOnce(), + ); + agentRepository.findByIdAndProjectId.mockResolvedValue( + mock({ id: 'agent-1', schema: { instructions: 'Changed' } as never }), + ); + releaseFirst(); + await finished; + expect(evalAgentExecutionService.captureRunSource).toHaveBeenCalledOnce(); + for (const call of evalAgentExecutionService.executeWithLlmMock.mock.calls) { + expect(call[4]?.source).toBe(source); + } + }); + + it('waits for an active targeted rerun after the last original case settles', async () => { + seedFor( + [ + { id: 'row-1', question: 'First' }, + { id: 'row-2', question: 'Second' }, + ], + { success: 2 }, + ); + evalAgentExecutionService.executeWithLlmMock.mockResolvedValue(successExec() as never); + resultRepository.countByStatus.mockResolvedValue(counts({ running: 1, success: 1 })); + let finished = false; + const run = await service.startRun('ds-1', 'proj-1', user); + void run.finished.then(() => { + finished = true; + }); + await vi.waitFor(() => expect(resultRepository.countByStatus).toHaveBeenCalled()); + expect(finished).toBe(false); + expect(runRepository.markAsCompleted).not.toHaveBeenCalled(); + resultRepository.countByStatus.mockResolvedValue(counts({ success: 2 })); + await run.finished; + expect(runRepository.markAsCompleted).toHaveBeenCalledWith( + 'run-1', + expect.objectContaining({ total: 2, success: 2, pending: 0 }), + expect.anything(), + ); + }); + + it('honors a cancellation received while a targeted rerun is finishing', async () => { + seedFor([{ id: 'row-1', question: 'First' }], { running: 1 }); + evalAgentExecutionService.executeWithLlmMock.mockResolvedValue(successExec() as never); + const { finished } = await service.startRun('ds-1', 'proj-1', user); + await vi.waitFor(() => expect(resultRepository.countByStatus).toHaveBeenCalled()); + runRepository.isCancellationRequested.mockResolvedValue(true); + resultRepository.countByStatus.mockResolvedValue(counts({ success: 1 })); + await finished; + expect(runRepository.markAsCancelled).toHaveBeenCalledWith( + 'run-1', + expect.objectContaining({ pending: 0 }), + counts({ success: 1 }), + ); + expect(runRepository.markAsCompleted).not.toHaveBeenCalled(); + }); + + it('ends with an error when a result cannot be recorded after execution', async () => { + seedFor([{ id: 'row-1', question: 'First' }], { running: 1 }); + evalAgentExecutionService.executeWithLlmMock.mockRejectedValue(new Error('Execution failed')); + resultRepository.markAsError.mockRejectedValue(new Error('Result write failed')); + const { finished } = await service.startRun('ds-1', 'proj-1', user); + await finished; + expect(runRepository.markAsError).toHaveBeenCalledWith( + 'run-1', + 'case_dispatch_failed', + expect.anything(), + expect.objectContaining({ pending: 1 }), + undefined, + ); + expect(runRepository.markAsCompleted).not.toHaveBeenCalled(); + }); + + it('bounds the wait for a result left running by another process', async () => { + const now = vi.spyOn(Date, 'now').mockReturnValue(1_000); + try { + seedFor([{ id: 'row-1', question: 'First' }], { running: 1 }); + evalAgentExecutionService.executeWithLlmMock.mockResolvedValue(successExec() as never); + const { finished } = await service.startRun('ds-1', 'proj-1', user); + await vi.waitFor(() => expect(resultRepository.countByStatus).toHaveBeenCalled()); + now.mockReturnValue(1_000 + 65 * 60_000); + await finished; + expect(runRepository.markAsError).toHaveBeenCalledWith( + 'run-1', + 'result_settlement_timeout', + expect.anything(), + expect.objectContaining({ pending: 1 }), + ); + } finally { + now.mockRestore(); + } + }); + it('runs each case with its input verbatim, persists results, aggregates the run', async () => { seedFor( [ @@ -380,6 +498,7 @@ describe('AgentEvalRunnerService', () => { error: 0, usage: { inputTokens: 6, outputTokens: 14 }, }), + expect.anything(), ); }); @@ -490,7 +609,7 @@ describe('AgentEvalRunnerService', () => { expect(resultRepository.markAsRunning).not.toHaveBeenCalled(); }); - it('still completes the run when recording an empty-input case fails', async () => { + it('reports an error and processes other cases when recording an empty-input case fails', async () => { seedFor( [ { id: 'row-1', question: ' ' }, @@ -506,8 +625,14 @@ describe('AgentEvalRunnerService', () => { const { finished } = await service.startRun('ds-1', 'proj-1', user); await finished; - expect(runRepository.markAsCompleted).toHaveBeenCalled(); - expect(runRepository.markAsError).not.toHaveBeenCalled(); + expect(runRepository.markAsCompleted).not.toHaveBeenCalled(); + expect(runRepository.markAsError).toHaveBeenCalledWith( + 'run-1', + 'case_dispatch_failed', + expect.anything(), + expect.anything(), + undefined, + ); }); it('pages through every row when the table exceeds one page', async () => { @@ -803,6 +928,7 @@ describe('AgentEvalRunnerService', () => { expect(runRepository.markAsCompleted).toHaveBeenCalledWith( 'run-1', expect.objectContaining({ success: 1 }), + expect.anything(), ); }); @@ -844,6 +970,7 @@ describe('AgentEvalRunnerService', () => { expect(runRepository.markAsCancelled).toHaveBeenCalledWith( 'run-1', expect.objectContaining({ cancelled: 2 }), + expect.anything(), ); expect(runRepository.markAsCompleted).not.toHaveBeenCalled(); }); @@ -891,6 +1018,7 @@ describe('AgentEvalRunnerService', () => { expect(runRepository.markAsCancelled).toHaveBeenCalledWith( 'run-1', expect.objectContaining({ total: 2, cancelled: 1, pending: 1 }), + undefined, ); expect(runRepository.markAsError).not.toHaveBeenCalled(); }); @@ -919,6 +1047,7 @@ describe('AgentEvalRunnerService', () => { 'case_dispatch_failed', expect.objectContaining({ errors: ['db down'] }), expect.objectContaining({ total: 2, success: 1 }), + undefined, ); expect(runRepository.markAsCompleted).not.toHaveBeenCalled(); }); @@ -941,6 +1070,7 @@ describe('AgentEvalRunnerService', () => { 'case_dispatch_failed', expect.objectContaining({ errors: ['db down'] }), expect.objectContaining({ total: 1, success: 1 }), + undefined, ); // Not the tally-less `run_failed` path. expect(runRepository.markAsError).not.toHaveBeenCalledWith( @@ -988,6 +1118,7 @@ describe('AgentEvalRunnerService', () => { }), // counts land under `metrics`, same as every other terminal status expect.objectContaining({ total: 2, cancelled: 2 }), + expect.anything(), ); // A deadline is a failure, not the user's Stop. expect(runRepository.markAsCancelled).not.toHaveBeenCalled(); @@ -1007,7 +1138,11 @@ describe('AgentEvalRunnerService', () => { await vi.advanceTimersByTimeAsync(30 * 60_000); await finished; - expect(runRepository.markAsCancelled).toHaveBeenCalledWith('run-1', expect.anything()); + expect(runRepository.markAsCancelled).toHaveBeenCalledWith( + 'run-1', + expect.anything(), + expect.anything(), + ); expect(runRepository.markAsError).not.toHaveBeenCalled(); }); @@ -1039,6 +1174,7 @@ describe('AgentEvalRunnerService', () => { expect(runRepository.markAsCompleted).toHaveBeenCalledWith( 'run-1', expect.objectContaining({ total: 1, success: 1 }), + expect.anything(), ); expect(runRepository.markAsError).not.toHaveBeenCalled(); }); @@ -1138,7 +1274,11 @@ describe('AgentEvalRunnerService', () => { executionId: expect.stringContaining('-case-1'), }), ); - expect(runRepository.markAsCancelled).toHaveBeenCalledWith('run-1', expect.anything()); + expect(runRepository.markAsCancelled).toHaveBeenCalledWith( + 'run-1', + expect.anything(), + expect.anything(), + ); expect(evalAgentExecutionService.executeWithLlmMock).not.toHaveBeenCalled(); }); diff --git a/packages/cli/src/modules/agent-evals/agent-eval-runner.service.ts b/packages/cli/src/modules/agent-evals/agent-eval-runner.service.ts index 1fe9d0c2a9e..f59e8dc19ea 100644 --- a/packages/cli/src/modules/agent-evals/agent-eval-runner.service.ts +++ b/packages/cli/src/modules/agent-evals/agent-eval-runner.service.ts @@ -1,3 +1,4 @@ +import { setTimeout as sleep } from 'node:timers/promises'; import type { AgentEvalVerdict, AgentEvalRunSummary } from '@n8n/api-types'; import { Logger, ModuleRegistry } from '@n8n/backend-common'; import { GlobalConfig } from '@n8n/config'; @@ -26,6 +27,7 @@ import { resolveEvaluationConcurrencyLimit } from '@/evaluation.ee/evaluation-co import { License } from '@/license'; import { AgentConfigService } from '@/modules/agents/agent-config.service'; import { AgentsSettingsService } from '@/modules/agents/agents-settings.service'; +import type { AgentRunSource } from '@/modules/agents/utils/agent-published-snapshot'; import { AgentRepository } from '@/modules/agents/repositories/agent.repository'; import { DataTableService } from '@/modules/data-table/data-table.service'; import { EvalAgentExecutionService } from '@/modules/instance-ai/eval/agent-execution.service'; @@ -46,6 +48,9 @@ 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; +// Reruns allow a one-hour execution budget. Leave five minutes for judging. +const RESULT_SETTLEMENT_TIMEOUT_MS = 65 * 60_000; +const RESULT_SETTLEMENT_POLL_MS = 1_000; /** A dataset row resolved into a runnable case via the dataset's column mapping. */ interface ResolvedCase { @@ -148,13 +153,11 @@ export class AgentEvalRunnerService { throw new BadRequestError('The dataset has no rows to run.'); } + const source = await this.evalAgentExecutionService.captureRunSource(agent); + const run = await this.runRepository.createRun({ datasetId: dataset.id, - // Left unpinned: execution reloads the live agent by id (there is no - // snapshot-execution path yet), so tagging a specific version would - // misrepresent what actually ran — mirrors the workflow eval, which - // records a version only when it executes that pinned snapshot. - // Version-pinned execution is a follow-up. + // This is a detached draft snapshot, not a published agent version. agentVersionId: null, createdById: user.id, }); @@ -191,6 +194,7 @@ export class AgentEvalRunnerService { user, cases, seeded, + source, timeoutMs: options.timeoutMs, }); @@ -299,6 +303,7 @@ export class AgentEvalRunnerService { user: User; cases: ResolvedCase[]; seeded: AgentEvalResult[]; + source: AgentRunSource; timeoutMs?: number; }): Promise { const { runId, cases, seeded } = ctx; @@ -350,8 +355,7 @@ export class AgentEvalRunnerService { }; // An empty input can't produce an execution, so its verdict is recorded - // without taking a queue slot. Self-contained like `runCase`: one unusable - // case must not fail the run just because recording it failed. + // without taking a queue slot. A failed write reaches the batch error path. const markEmptyInput = async (resultRow: AgentEvalResult) => { try { await this.resultRepository.markAsError(resultRow.id, 'empty_input', { @@ -362,6 +366,7 @@ export class AgentEvalRunnerService { `[AgentEvalRunner] Could not record empty input for case ${resultRow.id}`, { error: error instanceof Error ? error.message : String(error) }, ); + throw error; } }; @@ -421,18 +426,7 @@ export class AgentEvalRunnerService { 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. 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( @@ -441,42 +435,82 @@ export class AgentEvalRunnerService { ); } - 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`, - ); + // A settled sibling can be rerun while the original pool is active. + // Fence the terminal write against those attempts, including other mains. + const settlementDeadline = Date.now() + RESULT_SETTLEMENT_TIMEOUT_MS; + while (true) { + if (!wasCancelled) { + try { + wasCancelled = await this.runRepository.isCancellationRequested(runId); + } catch (error) { + dispatchFailures.push(error instanceof Error ? error.message : String(error)); + } } - await this.runRepository.markAsCompleted(runId, metrics); + const counts = await this.resultRepository.countByStatus(runId); + const metrics: IDataObject = { ...toSummaryCounts(counts), usage: { ...totalUsage } }; + if (Date.now() >= settlementDeadline) { + if (wasCancelled) { + await this.runRepository.markAsCancelled(runId, metrics); + } else { + await this.runRepository.markAsError( + runId, + 'result_settlement_timeout', + { + message: 'A result did not settle after the original batch finished.', + }, + metrics, + ); + } + break; + } + if (dispatchFailures.length === 0 && (counts.new > 0 || counts.running > 0)) { + await sleep(RESULT_SETTLEMENT_POLL_MS); + continue; + } + // Failed writes can leave an original row pending. Report that failure instead of waiting for it. + const finalCounts = dispatchFailures.length === 0 ? counts : undefined; + + let settled; + // 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. + settled = await this.runRepository.markAsCancelled(runId, metrics, finalCounts); + } else if (deadline.hasExpired() && stoppedCases > 0) { + settled = await this.runRepository.markAsError( + runId, + 'timeout', + { + message: `Run exceeded its ${deadline.deadlineMinutes}-minute deadline; ${stoppedCases} case(s) were not started.`, + }, + metrics, + finalCounts, + ); + } else if (dispatchFailures.length > 0) { + settled = await this.runRepository.markAsError( + runId, + 'case_dispatch_failed', + { + message: `${dispatchFailures.length} failure(s) occurred outside case execution.`, + errors: dispatchFailures, + }, + metrics, + finalCounts, + ); + } 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`, + ); + } + settled = await this.runRepository.markAsCompleted(runId, metrics, finalCounts); + } + if ((settled.affected ?? 0) > 0) break; + const current = await this.runRepository.findById(runId); + if (!current || current.status !== 'running') break; + await sleep(RESULT_SETTLEMENT_POLL_MS); } } catch (error) { const message = error instanceof Error ? error.message : String(error); @@ -593,10 +627,9 @@ export class AgentEvalRunnerService { /** * Execute one case and persist its result. Returns the case's token usage. - * Fully self-contained: a thrown execution or DB error is converted into a - * per-case error so one case can never abort the batch or leave its result - * stuck `running`. Assumes a non-empty input — the caller screens those out - * before taking a queue slot. + * Execution errors become per-case errors. If that write also fails, the batch + * reports the persistence failure. The caller screens out empty input before + * taking a queue slot. * * On a successful execution, also grades the output via {@link judgeCase} * and persists the verdict — this is the one place both `executeRun`'s pool @@ -607,7 +640,13 @@ export class AgentEvalRunnerService { private async runCase( resultRow: AgentEvalResult, resolvedCase: ResolvedCase, - ctx: { agentId: string; projectId: string; user: User; timeoutMs?: number }, + ctx: { + agentId: string; + projectId: string; + user: User; + timeoutMs?: number; + source?: AgentRunSource; + }, ): Promise { try { const runAt = await this.resultRepository.markAsRunning(resultRow.id); @@ -618,6 +657,7 @@ export class AgentEvalRunnerService { { projectId: ctx.projectId, ...(ctx.timeoutMs ? { timeoutMs: ctx.timeoutMs } : {}) }, resolvedCase.input, { + ...(ctx.source ? { source: ctx.source } : {}), onProgress: async (calls) => { await this.resultRepository.updateRunningToolCalls( resultRow.id, @@ -682,6 +722,7 @@ export class AgentEvalRunnerService { this.logger.error(`[AgentEvalRunner] Could not record failure for case ${resultRow.id}`, { error: markError instanceof Error ? markError.message : String(markError), }); + throw markError; } return undefined; } diff --git a/packages/cli/src/modules/instance-ai/eval/__tests__/agent-execution.service.test.ts b/packages/cli/src/modules/instance-ai/eval/__tests__/agent-execution.service.test.ts index 1bf07cfdafc..2212d470968 100644 --- a/packages/cli/src/modules/instance-ai/eval/__tests__/agent-execution.service.test.ts +++ b/packages/cli/src/modules/instance-ai/eval/__tests__/agent-execution.service.test.ts @@ -160,6 +160,79 @@ describe('EvalAgentExecutionService.executeWithLlmMock', () => { const request = { projectId: 'proj-1' }; + it('keeps a detached root config, tools, and resolved skills for each batch case', async () => { + const entity = makeEntity({ ...baseConfig }); + const skills = { skill1: { name: 'Support', description: 'Before', content: 'Old skill' } }; + const skillRefs = [{ type: 'skill', id: 'skill1' }]; + const skillService = createAgentSkillRefsService(); + vi.spyOn(skillService, 'readDraft').mockResolvedValue({ skillRefs, skills } as never); + Container.set(AgentSkillRefsService, skillService); + const service = buildService(); + const source = await service.captureRunSource(entity); + entity.schema!.instructions = 'Changed instructions'; + entity.tools!['t-1'].code = 'Changed code'; + skills.skill1.content = 'Changed skill'; + findByIdAndProjectId.mockResolvedValue(entity); + reconstructFromAgentEntity.mockResolvedValue({ + agent: { generate: vi.fn().mockResolvedValue(makeGenerateResult()), close: vi.fn() }, + }); + + await service.executeWithLlmMock('agent-1', user, { projectId: 'proj-1' }, 'First', { source }); + await service.executeWithLlmMock('agent-1', user, { projectId: 'proj-1' }, 'Second', { + source, + }); + + expect(skillService.readDraft).toHaveBeenCalledOnce(); + expect(reconstructFromAgentEntity).toHaveBeenCalledTimes(2); + for (const [snapshot] of reconstructFromAgentEntity.mock.calls) { + expect(snapshot.agent.schema.instructions).toBe(baseConfig.instructions); + expect(snapshot.agent.tools['t-1'].code).toBe('export default 1'); + expect(snapshot.skills.skill1.content).toBe('Old skill'); + } + expect(source.agent.schema!.instructions).toBe(baseConfig.instructions); + expect(userHasScopes).toHaveBeenCalledTimes(2); + await service.executeWithLlmMock('agent-1', user, { projectId: 'proj-1' }, 'Rerun'); + const [current] = reconstructFromAgentEntity.mock.calls[2]; + expect(current.agent.schema.instructions).toBe('Changed instructions'); + expect(current.agent.tools['t-1'].code).toBe('Changed code'); + expect(current.skills.skill1.content).toBe('Changed skill'); + expect(skillService.readDraft).toHaveBeenCalledTimes(2); + }); + + it('still checks permissions and project identity for a captured source', async () => { + const service = buildService(); + const source = await service.captureRunSource(makeEntity({ ...baseConfig })); + vi.mocked(userHasScopes).mockResolvedValue(false); + expect( + await service.executeWithLlmMock('agent-1', user, { projectId: 'proj-1' }, 'Case', { + source, + }), + ).toMatchObject({ success: false }); + vi.mocked(userHasScopes).mockResolvedValue(true); + expect( + await service.executeWithLlmMock('agent-1', user, { projectId: 'other-project' }, 'Case', { + source, + }), + ).toMatchObject({ success: false }); + expect(reconstructFromAgentEntity).not.toHaveBeenCalled(); + }); + + it('refuses a captured source when the agent is no longer in the project', async () => { + const service = buildService(); + const source = await service.captureRunSource(makeEntity({ ...baseConfig })); + findByIdAndProjectId.mockResolvedValue(null); + const result = await service.executeWithLlmMock( + 'agent-1', + user, + { projectId: 'proj-1' }, + 'Case', + { source }, + ); + expect(result.success).toBe(false); + expect(result.errors).toEqual(['Agent agent-1 not found or not accessible']); + expect(reconstructFromAgentEntity).not.toHaveBeenCalled(); + }); + it('reports tool progress before generation finishes and flushes writes before returning', async () => { const handlers = new Map(); const onProgress = vi.fn().mockResolvedValue(undefined); diff --git a/packages/cli/src/modules/instance-ai/eval/agent-execution.service.ts b/packages/cli/src/modules/instance-ai/eval/agent-execution.service.ts index 209666ce3c0..c3a168efc2a 100644 --- a/packages/cli/src/modules/instance-ai/eval/agent-execution.service.ts +++ b/packages/cli/src/modules/instance-ai/eval/agent-execution.service.ts @@ -23,7 +23,7 @@ import { ExecutionsConfig } from '@n8n/config'; import type { User } from '@n8n/db'; import { Container, Service } from '@n8n/di'; import type { EvalLlmMockHandler } from 'n8n-core'; -import { nodeNameToToolName } from 'n8n-workflow'; +import { deepCopy, nodeNameToToolName } from 'n8n-workflow'; import { CredentialsService } from '@/credentials/credentials.service'; // Static agents-module imports are safe here: the ModuleRegistry gate decides @@ -32,6 +32,7 @@ import { AgentRuntimeReconstructionService } from '@/modules/agents/agent-runtim import { hashAgentSandboxPrincipal } from '@/modules/agents/agent-sandbox-principal'; import { AgentsSettingsService } from '@/modules/agents/agents-settings.service'; import type { Agent as AgentEntity } from '@/modules/agents/entities/agent.entity'; +import type { AgentRunSource } from '@/modules/agents/utils/agent-published-snapshot'; import { AgentSkillRefsService } from '@/modules/agents/agent-skill-refs.service'; import { sanitizeToolName } from '@/modules/agents/json-config/agent-config-composition'; import { fromAgentDocument, toAgentDocument } from '@/modules/agents/json-config/agent-document'; @@ -158,6 +159,19 @@ export class EvalAgentExecutionService { private readonly credentialsService: CredentialsService, ) {} + /** Keep the root agent and resolved skills fixed while a batch is in flight. */ + async captureRunSource(agent: AgentEntity): Promise { + const detached = { + ...agent, + schema: deepCopy(agent.schema), + tools: deepCopy(agent.tools), + skills: deepCopy(agent.skills), + integrations: deepCopy(agent.integrations), + }; + const skills = await Container.get(AgentSkillRefsService).readDraft(detached, {}); + return { agent: detached, ...deepCopy(skills) }; + } + async executeWithLlmMock( agentId: string, user: User, @@ -167,7 +181,10 @@ export class EvalAgentExecutionService { // per-tool hints keep the tool mocks coherent), but the message the agent // receives is this input verbatim — how a fixed eval case is executed. caseInput?: string, - hooks?: { onProgress?: (calls: InstanceAiEvalAgentToolCallRecord[]) => Promise }, + hooks?: { + onProgress?: (calls: InstanceAiEvalAgentToolCallRecord[]) => Promise; + source?: AgentRunSource; + }, ): Promise { // Workflow-tool sub-executions carry a configureAdditionalData closure // that doesn't survive queue serialization — refuse upfront so tool @@ -192,19 +209,33 @@ export class EvalAgentExecutionService { return this.errorResult(`Agent ${agentId} not found or not accessible`); } - const agentEntity = await Container.get(AgentRepository).findByIdAndProjectId( + const source = hooks?.source; + if (source && (source.agent.id !== agentId || source.agent.projectId !== projectId)) { + return this.errorResult(`Agent ${agentId} not found or not accessible`); + } + // Keep the scoped existence check even when content comes from the batch snapshot. + const currentAgent = await Container.get(AgentRepository).findByIdAndProjectId( agentId, projectId, ); - if (!agentEntity) { + if (!currentAgent) { return this.errorResult(`Agent ${agentId} not found or not accessible`); } - if (!agentEntity.schema) { + const storedAgent = source?.agent ?? currentAgent; + if (!storedAgent.schema) { return this.errorResult(`Agent ${agentId} has no JSON config to run.`); } - const agentSkillRefs = Container.get(AgentSkillRefsService); - const skillRefs = await agentSkillRefs.refsForDraft(agentEntity, {}); + // Each case gets its own mutable copy; pruning must not change the batch source. + const runSource = source ?? (await this.captureRunSource(storedAgent)); + const agentEntity = { + ...runSource.agent, + schema: deepCopy(storedAgent.schema), + tools: deepCopy(runSource.agent.tools), + skills: deepCopy(runSource.agent.skills), + integrations: deepCopy(runSource.agent.integrations), + }; + const skillRefs = deepCopy(runSource.skillRefs); const { config, skippedFeatures } = pruneConfigForEval( toAgentDocument(agentEntity.schema, skillRefs), ); @@ -365,7 +396,7 @@ export class EvalAgentExecutionService { { agent: agentEntity, skillRefs: pruned.skillRefs, - skills: await agentSkillRefs.skillsForRefs(agentEntity, pruned.skillRefs, {}), + skills: deepCopy(runSource.skills), }, credentialProvider, 'test', From 0582e60ab46a88bfe97865322b664349aece4498 Mon Sep 17 00:00:00 2001 From: Luka Zivkovic Date: Thu, 15 Oct 2026 08:21:30 +0200 Subject: [PATCH 08/18] refactor(core): Keep check progress updates within repository boundary --- .../db/src/repositories/agent-eval-result.repository.ee.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/@n8n/db/src/repositories/agent-eval-result.repository.ee.ts b/packages/@n8n/db/src/repositories/agent-eval-result.repository.ee.ts index 1a630d75f9c..80c9d39df95 100644 --- a/packages/@n8n/db/src/repositories/agent-eval-result.repository.ee.ts +++ b/packages/@n8n/db/src/repositories/agent-eval-result.repository.ee.ts @@ -90,8 +90,8 @@ export class AgentEvalResultRepository extends Repository { } /** Progress belongs to one running attempt, identified by its start timestamp. */ - async updateRunningToolCalls(id: string, runAt: Date, toolCalls: JsonObject) { - return await this.update({ id, status: 'running', runAt }, { toolCalls }); + async updateRunningToolCalls(id: string, runAt: Date, toolCalls: JsonObject): Promise { + await this.update({ id, status: 'running', runAt }, { toolCalls }); } /** From f3e0ae0f9a284d29577739c9ca7a178449c02833 Mon Sep 17 00:00:00 2001 From: Luka Zivkovic Date: Thu, 15 Oct 2026 08:25:17 +0200 Subject: [PATCH 09/18] fix(editor): Guard queued checks and defer additions across active runs --- .../__tests__/AgentEvalAddCheckPanel.test.ts | 40 ++++++++++++ .../__tests__/AgentEvalChecksPanel.test.ts | 64 +++++++++++++++++-- .../components/AgentEvalAddCheckPanel.vue | 27 +++++--- .../components/AgentEvalChecksPanel.vue | 10 ++- 4 files changed, 121 insertions(+), 20 deletions(-) diff --git a/packages/frontend/editor-ui/src/features/agents/__tests__/AgentEvalAddCheckPanel.test.ts b/packages/frontend/editor-ui/src/features/agents/__tests__/AgentEvalAddCheckPanel.test.ts index 755b697e921..ff3d5c897d3 100644 --- a/packages/frontend/editor-ui/src/features/agents/__tests__/AgentEvalAddCheckPanel.test.ts +++ b/packages/frontend/editor-ui/src/features/agents/__tests__/AgentEvalAddCheckPanel.test.ts @@ -80,6 +80,46 @@ describe('AgentEvalAddCheckPanel', () => { vi.clearAllMocks(); }); + it.each([ + ['drafting a rule', 'deferRun'], + ['saving a rule', 'busy'], + ['saving a prepared case', 'deferRun'], + ['saving a prepared case', 'busy'], + ] as const)('remembers a run that starts and finishes while %s (%s)', async (phase, runProp) => { + const user = userEvent.setup(); + const { store, findAllByTestId, getByTestId, rerender, emitted } = renderPanel(); + await findAllByTestId('agent-eval-add-check-prepared-add'); + let finish!: () => void; + const pending = new Promise((resolve) => { + finish = resolve; + }); + if (phase === 'drafting a rule') { + vi.mocked(store.generateDraftCases).mockImplementationOnce(async () => { + await pending; + return { cases: [draft(9)] }; + }); + } else { + vi.mocked(store.createCase).mockImplementationOnce(async () => { + await pending; + return { rowId: 9, input: 'new message', whatToCheck: 'new rule' }; + }); + } + if (phase === 'saving a prepared case') { + await user.click((await findAllByTestId('agent-eval-add-check-prepared-add'))[0]); + } else { + await user.type(getByTestId('agent-eval-add-check-rule-input'), 'Be polite{Enter}'); + } + + await rerender({ [runProp]: true }); + await rerender({ [runProp]: false }); + finish(); + await waitFor(() => expect(emitted('added')).toEqual([[true]])); + + // The next addition has no overlap with that finished run. + await user.click((await findAllByTestId('agent-eval-add-check-prepared-add'))[0]); + await waitFor(() => expect(emitted('added')).toEqual([[true], [false]])); + }); + it.each([true, false])( 'defers additions if a run is active at either end of the save (initial: %s)', async (initiallyRunning) => { diff --git a/packages/frontend/editor-ui/src/features/agents/__tests__/AgentEvalChecksPanel.test.ts b/packages/frontend/editor-ui/src/features/agents/__tests__/AgentEvalChecksPanel.test.ts index 88a10cd9b9b..43e9addf7cf 100644 --- a/packages/frontend/editor-ui/src/features/agents/__tests__/AgentEvalChecksPanel.test.ts +++ b/packages/frontend/editor-ui/src/features/agents/__tests__/AgentEvalChecksPanel.test.ts @@ -2,7 +2,7 @@ import { configure, within } from '@testing-library/vue'; import { createTestingPinia } from '@pinia/testing'; import userEvent from '@testing-library/user-event'; import { beforeEach, describe, expect, it, vi } from 'vitest'; -import { ref } from 'vue'; +import { nextTick, ref } from 'vue'; import { ResponseError } from '@n8n/rest-api-client'; @@ -86,8 +86,15 @@ const { addCheckPanelMounts } = vi.hoisted(() => ({ addCheckPanelMounts: { count vi.mock('../components/AgentEvalAddCheckPanel.vue', () => ({ default: { name: 'AgentEvalAddCheckPanel', - setup() { + setup(props: { deferRun: boolean }) { addCheckPanelMounts.count++; + const deferredAtStart = ref(false); + return { + deferredAtStart, + startAdding: () => { + deferredAtStart.value = props.deferRun; + }, + }; }, props: { projectId: {}, @@ -95,17 +102,20 @@ vi.mock('../components/AgentEvalAddCheckPanel.vue', () => ({ caseSource: {}, disabled: { type: Boolean }, busy: { type: Boolean }, + deferRun: { type: Boolean }, }, emits: ['close', 'added'], template: `
- - + + +
`, }, })); @@ -315,6 +325,11 @@ describe('AgentEvalChecksPanel', () => { live.value = loadedReview([result('n1', 'new')]); + await vi.waitFor(() => + expect(getByTestId('agent-eval-check-n1')).toHaveAttribute('data-disabled', 'true'), + ); + live.value = loadedReview([result('n1', 'success')]); + await vi.waitFor(() => expect(getByTestId('agent-eval-check-n1')).toHaveAttribute('data-disabled', 'false'), ); @@ -723,7 +738,7 @@ describe('AgentEvalChecksPanel', () => { expect(store.applySuggestions).toHaveBeenCalledWith('project-1', 'agent-1', ['c1', 'c3']); }); - it('applies settled suggestions during a batch and leaves running checks out', async () => { + it('applies settled suggestions during a batch and leaves queued and running checks out', async () => { const user = userEvent.setup(); const order: string[] = []; const { getByTestId, store } = render( @@ -731,6 +746,7 @@ describe('AgentEvalChecksPanel', () => { results: [ failedWith('c1', 'Be brief'), { ...failedWith('c2', 'Wait'), status: 'running' }, + { ...failedWith('c3', 'Still queued'), status: 'new' }, ], }, true, @@ -876,7 +892,8 @@ describe('AgentEvalChecksPanel', () => { loading: false, loadingMore: false, }); - vi.mocked(store.isRunInFlight).mockReturnValue(options.inFlight ?? false); + const inFlight = ref(options.inFlight ?? false); + vi.mocked(store.isRunInFlight).mockImplementation(() => inFlight.value); vi.mocked(store.isStartingRun).mockReturnValue(false); vi.mocked(store.getDatasets).mockReturnValue([ { ...dataset, columnMapping: options.columnMapping ?? dataset.columnMapping }, @@ -887,6 +904,7 @@ describe('AgentEvalChecksPanel', () => { props: { disabled: options.disabled, rerunning: options.rerunning }, }), store, + inFlight, }; }; @@ -998,8 +1016,13 @@ describe('AgentEvalChecksPanel', () => { it('keeps an addition deferred after the original run finishes', async () => { const user = userEvent.setup(); - const { getByTestId, getByText, emitted } = renderWithDataset(); + const { getByTestId, getByText, emitted, inFlight } = renderWithDataset({ inFlight: true }); await user.click(getByTestId('agent-eval-checks-add-check')); + expect(getByTestId('add-check-panel-stub')).toHaveAttribute('data-defer-run', 'true'); + await user.click(getByText('start adding a check')); + inFlight.value = false; + await nextTick(); + expect(getByTestId('add-check-panel-stub')).toHaveAttribute('data-defer-run', 'false'); await user.click(getByText('added after run finished')); expect(emitted('rerun')).toBeUndefined(); expect(getByTestId('agent-eval-checks-saved-next-run')).toBeVisible(); @@ -1192,6 +1215,33 @@ describe('AgentEvalChecksPanel', () => { }); }); + it.each(['new', 'running'] as const)( + 'blocks mutations on a %s check during a batch', + async (status) => { + const user = userEvent.setup(); + const { getByTestId, store } = render({ results: [result('c1', status)] }, true); + const row = getByTestId('agent-eval-check-c1'); + expect(row).toHaveAttribute('data-disabled', 'true'); + for (const action of [ + 'actually fine', + 'run check', + 'save rule', + 'delete check', + 'apply suggestion', + ]) { + // Guard handlers as well as controls against an event already in flight. + await user.click(within(row).getByText(action)); + } + expect(store.acceptResult).not.toHaveBeenCalled(); + expect(store.rerunResult).not.toHaveBeenCalled(); + expect(store.updateCaseRule).not.toHaveBeenCalled(); + expect(store.deleteCase).not.toHaveBeenCalled(); + expect(store.deleteResult).not.toHaveBeenCalled(); + expect(store.applySuggestions).not.toHaveBeenCalled(); + expect(showError).not.toHaveBeenCalled(); + }, + ); + it('passes current tool progress to a running check without an answer', () => { const { getByTestId } = render( { diff --git a/packages/frontend/editor-ui/src/features/agents/components/AgentEvalAddCheckPanel.vue b/packages/frontend/editor-ui/src/features/agents/components/AgentEvalAddCheckPanel.vue index e09e5462792..1efb3d79c3b 100644 --- a/packages/frontend/editor-ui/src/features/agents/components/AgentEvalAddCheckPanel.vue +++ b/packages/frontend/editor-ui/src/features/agents/components/AgentEvalAddCheckPanel.vue @@ -5,7 +5,7 @@ * Either way the case is written to the run's dataset, and the parent is told * so it can run the checks again — a run only holds the cases it started with. */ -import { computed, onBeforeUnmount, onMounted, ref } from 'vue'; +import { computed, onBeforeUnmount, onMounted, ref, watch } from 'vue'; import type { AgentEvalDraftCase } from '@n8n/api-types'; import { N8nButton, N8nIcon, N8nSpinner, N8nText } from '@n8n/design-system'; import { useToast } from '@n8n/composables/useToast'; @@ -49,6 +49,16 @@ const loadingPrepared = ref(false); const expanded = ref(false); // 'rule' for the typed rule, or the key of the prepared case being added. const adding = ref<'rule' | number | null>(null); +let deferAddedCheck = false; + +// A run can start and finish before drafting or saving the new check completes. +watch( + () => props.deferRun || props.busy, + (busy) => { + if (adding.value !== null && busy) deferAddedCheck = true; + }, + { flush: 'sync' }, +); const canAdd = computed( () => !props.disabled && props.caseSource !== null && !props.busy && adding.value === null, @@ -88,10 +98,7 @@ onBeforeUnmount(() => { }); /** Reports the saved case and whether it must wait for the next run. */ -async function writeCase( - value: { input: string; whatToCheck: string }, - deferRun: boolean, -): Promise { +async function writeCase(value: { input: string; whatToCheck: string }): Promise { const source = props.caseSource; if (!source) return false; try { @@ -102,7 +109,7 @@ async function writeCase( if (isMounted) toast.showError(error, i18n.baseText('agents.builder.agentEvals.case.addError')); return false; } - emit('added', deferRun || Boolean(props.deferRun)); + emit('added', deferAddedCheck || Boolean(props.deferRun || props.busy)); return true; } @@ -110,7 +117,7 @@ async function onAddRule() { const rule = ruleInput.value.trim(); if (!rule || !canAdd.value) return; - const deferRun = Boolean(props.deferRun); + deferAddedCheck = Boolean(props.deferRun || props.busy); adding.value = 'rule'; try { let input: string; @@ -130,7 +137,7 @@ async function onAddRule() { } if (!isMounted) return; - if (await writeCase({ input, whatToCheck: rule }, deferRun)) ruleInput.value = ''; + if (await writeCase({ input, whatToCheck: rule })) ruleInput.value = ''; } finally { if (isMounted) adding.value = null; } @@ -139,10 +146,10 @@ async function onAddRule() { async function onAddPrepared(row: PreparedCase) { if (!canAdd.value) return; - const deferRun = Boolean(props.deferRun); + deferAddedCheck = Boolean(props.deferRun || props.busy); adding.value = row.key; try { - const written = await writeCase({ input: row.input, whatToCheck: row.whatToCheck }, deferRun); + const written = await writeCase({ input: row.input, whatToCheck: row.whatToCheck }); if (written && isMounted) { prepared.value = prepared.value.filter((candidate) => candidate.key !== row.key); } diff --git a/packages/frontend/editor-ui/src/features/agents/components/AgentEvalChecksPanel.vue b/packages/frontend/editor-ui/src/features/agents/components/AgentEvalChecksPanel.vue index 4d276c93c08..bced5dab63d 100644 --- a/packages/frontend/editor-ui/src/features/agents/components/AgentEvalChecksPanel.vue +++ b/packages/frontend/editor-ui/src/features/agents/components/AgentEvalChecksPanel.vue @@ -209,13 +209,17 @@ watch( { immediate: true }, ); +function isSettledCheck(row: CheckRow) { + return row.status !== 'waiting' && row.status !== 'idle'; +} + function canChangeResult(resultId: string) { return ( !props.disabled && !props.rerunning && !showingPreviousRun.value && !isApplyingSuggestions.value && - rows.value.some((row) => row.id === resultId && row.status !== 'waiting') + rows.value.some((row) => row.id === resultId && isSettledCheck(row)) ); } @@ -246,7 +250,7 @@ const applyingAll = ref(false); // composable's concern, not this button's. const applicableSuggestionIds = computed(() => rows.value - .filter((row) => row.fixSuggestion !== null && row.status !== 'waiting') + .filter((row) => row.fixSuggestion !== null && isSettledCheck(row)) .map((row) => row.id), ); @@ -540,7 +544,7 @@ onBeforeUnmount(store.stopPollingRun); :what-to-check="row.whatToCheck" :fix-suggestion="row.fixSuggestion" :applying-suggestion="applyingSuggestionIds.includes(row.id)" - :disabled="disabled || rerunning || showingPreviousRun || isApplyingSuggestions" + :disabled="!canChangeResult(row.id)" :running-check="row.status === 'waiting'" hide-revise view="complete" From b8c663ba60a98ab1c765e9101fe577eb31805416 Mon Sep 17 00:00:00 2001 From: Luka Zivkovic Date: Thu, 15 Oct 2026 08:29:13 +0200 Subject: [PATCH 10/18] fix(core): Serialize agent eval claims and run settlement --- .../agent-eval-result.repository.test.ts | 29 +++++-- .../agent-eval-run.repository.test.ts | 4 + .../agent-eval-result.repository.ee.ts | 52 ++++++++---- .../agent-eval-run.repository.ee.ts | 56 ++++++------ ...nt-eval-runner.service.integration.test.ts | 85 ++++++++++++++++++- .../agent-eval-runner.service.test.ts | 70 ++++++++++++--- .../agent-evals/agent-eval-runner.service.ts | 40 +++++++-- 7 files changed, 273 insertions(+), 63 deletions(-) diff --git a/packages/@n8n/db/src/repositories/__tests__/agent-eval-result.repository.test.ts b/packages/@n8n/db/src/repositories/__tests__/agent-eval-result.repository.test.ts index cf6bffc17a0..aef6928625a 100644 --- a/packages/@n8n/db/src/repositories/__tests__/agent-eval-result.repository.test.ts +++ b/packages/@n8n/db/src/repositories/__tests__/agent-eval-result.repository.test.ts @@ -1,12 +1,17 @@ import { Container } from '@n8n/di'; +import { TransactionRunner } from '../../services/transaction'; import type { Mock } from 'vitest'; +import { AgentEvalRun } from '../../entities/agent-eval-run.ee'; import { AgentEvalResult } from '../../entities/agent-eval-result.ee'; import { mockEntityManager } from '../../utils/test-utils/mock-entity-manager'; import { AgentEvalResultRepository } from '../agent-eval-result.repository.ee'; describe('AgentEvalResultRepository', () => { const entityManager = mockEntityManager(AgentEvalResult); + Container.set(TransactionRunner, { + run: async (ctx, fn) => await fn(ctx), + } satisfies TransactionRunner); const repo = Container.get(AgentEvalResultRepository); beforeEach(() => { @@ -185,20 +190,34 @@ describe('AgentEvalResultRepository', () => { }); describe('claimForRerun', () => { + it('locks the parent run before claiming a result', async () => { + entityManager.update.mockResolvedValue({ affected: 1, generatedMaps: [], raw: [] }); + await repo.claimForRerun('res-1', 'run-1'); + expect(entityManager.update.mock.calls[0]).toEqual([ + AgentEvalRun, + { id: 'run-1' }, + { id: 'run-1' }, + ]); + expect(entityManager.update.mock.calls[1]?.[1]).toMatchObject({ + id: 'res-1', + runId: 'run-1', + }); + }); + it('claims only a settled row, clearing the previous attempt', async () => { - entityManager.update.mockResolvedValueOnce({ affected: 1, generatedMaps: [], raw: [] }); + entityManager.update.mockResolvedValue({ affected: 1, generatedMaps: [], raw: [] }); - await expect(repo.claimForRerun('res-1')).resolves.toBe(true); + await expect(repo.claimForRerun('res-1', 'run-1')).resolves.toBe(true); - const [, where, patch] = entityManager.update.mock.calls[0] ?? []; + const [, where, patch] = entityManager.update.mock.calls[1] ?? []; expect(where).toMatchObject({ id: 'res-1' }); expect(patch).toMatchObject({ status: 'running', output: null, errorCode: null }); }); it('reports false when no row matched, because it was already running or claimed', async () => { - entityManager.update.mockResolvedValueOnce({ affected: 0, generatedMaps: [], raw: [] }); + entityManager.update.mockResolvedValue({ affected: 0, generatedMaps: [], raw: [] }); - await expect(repo.claimForRerun('res-1')).resolves.toBe(false); + await expect(repo.claimForRerun('res-1', 'run-1')).resolves.toBe(false); }); }); diff --git a/packages/@n8n/db/src/repositories/__tests__/agent-eval-run.repository.test.ts b/packages/@n8n/db/src/repositories/__tests__/agent-eval-run.repository.test.ts index f2350855914..0109558eec3 100644 --- a/packages/@n8n/db/src/repositories/__tests__/agent-eval-run.repository.test.ts +++ b/packages/@n8n/db/src/repositories/__tests__/agent-eval-run.repository.test.ts @@ -1,4 +1,5 @@ import { Container } from '@n8n/di'; +import { TransactionRunner } from '../../services/transaction'; import type { Mock } from 'vitest'; import { AgentEvalRun } from '../../entities/agent-eval-run.ee'; @@ -7,6 +8,9 @@ import { AgentEvalRunRepository } from '../agent-eval-run.repository.ee'; describe('AgentEvalRunRepository', () => { const entityManager = mockEntityManager(AgentEvalRun); + Container.set(TransactionRunner, { + run: async (ctx, fn) => await fn(ctx), + } satisfies TransactionRunner); const repo = Container.get(AgentEvalRunRepository); beforeEach(() => { diff --git a/packages/@n8n/db/src/repositories/agent-eval-result.repository.ee.ts b/packages/@n8n/db/src/repositories/agent-eval-result.repository.ee.ts index 1a630d75f9c..d1e0eb44632 100644 --- a/packages/@n8n/db/src/repositories/agent-eval-result.repository.ee.ts +++ b/packages/@n8n/db/src/repositories/agent-eval-result.repository.ee.ts @@ -1,8 +1,10 @@ import { Service } from '@n8n/di'; -import { DataSource, In, Repository } from '@n8n/typeorm'; +import { DataSource, In } from '@n8n/typeorm'; import type { IDataObject, JsonObject } from 'n8n-workflow'; -import { AgentEvalResult } from '../entities'; +import { AgentEvalResult, AgentEvalRun } from '../entities'; +import { TransactionRunner } from '../services/transaction'; +import { BaseRepository } from './base-repository'; import type { AgentEvalResultStatus } from '../entities/agent-eval-result.ee'; type CreateAgentEvalResultAttrs = { @@ -18,9 +20,9 @@ export type AgentEvalResultStatusCounts = Record; // parameter limit (SQLite in particular). const SEED_CHUNK_SIZE = 100; -const startedAttempt = () => ({ +const startedAttempt = (runAt = new Date()) => ({ status: 'running' as const, - runAt: new Date(), + runAt, completedAt: null, output: null, toolCalls: null, @@ -31,9 +33,9 @@ const startedAttempt = () => ({ }); @Service() -export class AgentEvalResultRepository extends Repository { - constructor(dataSource: DataSource) { - super(AgentEvalResult, dataSource.manager); +export class AgentEvalResultRepository extends BaseRepository { + constructor(dataSource: DataSource, transactionRunner: TransactionRunner) { + super(AgentEvalResult, dataSource.manager, transactionRunner); } /** @@ -83,8 +85,8 @@ export class AgentEvalResultRepository extends Repository { /** Starts an attempt. Clears whatever a previous attempt left behind, so a * rerun can't settle with an old error next to a new answer (or the reverse). */ - async markAsRunning(id: string) { - const attempt = startedAttempt(); + async markAsRunning(id: string, runAt = new Date()) { + const attempt = startedAttempt(runAt); await this.update(id, attempt); return attempt.runAt; } @@ -99,12 +101,32 @@ export class AgentEvalResultRepository extends Repository { * not settled — already running, or claimed by a concurrent request — so only * one caller ever executes a given rerun. */ - async claimForRerun(id: string): Promise { - const claimed = await this.update( - { id, status: In(['success', 'error', 'cancelled']) }, - startedAttempt(), - ); - return (claimed.affected ?? 0) > 0; + async claimForRerun(id: string, runId: string): Promise { + return await this.runInTransaction({}, async (manager) => { + // Serialize with terminal aggregation before changing any result. A write + // lock works on both Postgres and SQLite and needs no schema change. + await manager.update(AgentEvalRun, { id: runId }, { id: runId }); + const claimed = await manager.update( + AgentEvalResult, + { id, runId, status: In(['success', 'error', 'cancelled']) }, + startedAttempt(), + ); + return (claimed.affected ?? 0) > 0; + }); + } + + /** Repair only an unfinished original attempt. Never overwrite a newer rerun. */ + async markUnfinishedAsError(id: string, runAt: Date | undefined, message: string): Promise { + await this.createQueryBuilder() + .update() + .set({ + status: 'error', + completedAt: new Date(), + errorCode: 'case_dispatch_failed', + errorDetails: { message }, + }) + .where([{ id, status: 'new' }, ...(runAt ? [{ id, status: 'running', runAt }] : [])]) + .execute(); } async markAsCancelled(id: string) { diff --git a/packages/@n8n/db/src/repositories/agent-eval-run.repository.ee.ts b/packages/@n8n/db/src/repositories/agent-eval-run.repository.ee.ts index 7fd99a697b5..7950ff1e4fd 100644 --- a/packages/@n8n/db/src/repositories/agent-eval-run.repository.ee.ts +++ b/packages/@n8n/db/src/repositories/agent-eval-run.repository.ee.ts @@ -1,9 +1,11 @@ import { Service } from '@n8n/di'; -import { DataSource, In, Repository } from '@n8n/typeorm'; +import { DataSource, In } from '@n8n/typeorm'; import type { QueryDeepPartialEntity } from '@n8n/typeorm/query-builder/QueryPartialEntity'; import type { IDataObject } from 'n8n-workflow'; import { AgentEvalRun, AgentEvalResult } from '../entities'; +import { TransactionRunner } from '../services/transaction'; +import { BaseRepository } from './base-repository'; import type { AgentEvalResultStatusCounts } from './agent-eval-result.repository.ee'; type CreateAgentEvalRunAttrs = { @@ -13,9 +15,9 @@ type CreateAgentEvalRunAttrs = { }; @Service() -export class AgentEvalRunRepository extends Repository { - constructor(dataSource: DataSource) { - super(AgentEvalRun, dataSource.manager); +export class AgentEvalRunRepository extends BaseRepository { + constructor(dataSource: DataSource, transactionRunner: TransactionRunner) { + super(AgentEvalRun, dataSource.manager, transactionRunner); } async createRun(attrs: CreateAgentEvalRunAttrs): Promise { @@ -109,26 +111,32 @@ export class AgentEvalRunRepository extends Repository { counts?: AgentEvalResultStatusCounts, ) { if (!counts) return await this.update(id, attributes); - const query = this.createQueryBuilder() - .update() - .set(attributes) - .where({ id, status: 'running' }); - if (attributes.status !== 'cancelled') query.andWhere({ cancelRequested: false }); - for (const status of ['new', 'running', 'success', 'error', 'cancelled'] as const) { - const countQuery = this.manager - .createQueryBuilder(AgentEvalResult, 'result') - .select('COUNT(*)') - .where('result.runId = :runId') - .andWhere(`result.status = :status_${status}`) - .getQuery(); - const expected = status === 'new' || status === 'running' ? 0 : counts[status]; - query.andWhere(`(${countQuery}) = :count_${status}`, { - runId: id, - [`status_${status}`]: status, - [`count_${status}`]: expected, - }); - } - return await query.execute(); + return await this.runInTransaction({}, async (manager) => { + // Acquire the claim lock in a separate statement. Postgres must take a new + // snapshot for the count predicates after a concurrent claim commits. + await manager.update(AgentEvalRun, { id }, { id }); + const query = manager + .createQueryBuilder(AgentEvalRun, 'run') + .update() + .set(attributes) + .where({ id, status: 'running' }); + if (attributes.status !== 'cancelled') query.andWhere({ cancelRequested: false }); + for (const status of ['new', 'running', 'success', 'error', 'cancelled'] as const) { + const countQuery = manager + .createQueryBuilder(AgentEvalResult, 'result') + .select('COUNT(*)') + .where('result.runId = :runId') + .andWhere(`result.status = :status_${status}`) + .getQuery(); + const expected = status === 'new' || status === 'running' ? 0 : counts[status]; + query.andWhere(`(${countQuery}) = :count_${status}`, { + runId: id, + [`status_${status}`]: status, + [`count_${status}`]: expected, + }); + } + return await query.execute(); + }); } /** diff --git a/packages/cli/src/modules/agent-evals/__tests__/agent-eval-runner.service.integration.test.ts b/packages/cli/src/modules/agent-evals/__tests__/agent-eval-runner.service.integration.test.ts index 53832ad347d..dc828a9a430 100644 --- a/packages/cli/src/modules/agent-evals/__tests__/agent-eval-runner.service.integration.test.ts +++ b/packages/cli/src/modules/agent-evals/__tests__/agent-eval-runner.service.integration.test.ts @@ -5,6 +5,7 @@ import { AgentEvalDatasetRepository, AgentEvalResultRepository, AgentEvalRunRepository, + AgentEvalRun, GLOBAL_OWNER_ROLE, type User, } from '@n8n/db'; @@ -128,7 +129,7 @@ describe('AgentEvalRunnerService (integration)', () => { const [result] = await results.seedResults([{ runId: run.id }]); await results.markAsError(result.id, 'execution_failed'); const oldCounts = await results.countByStatus(run.id); - expect(await results.claimForRerun(result.id)).toBe(true); + expect(await results.claimForRerun(result.id, result.runId)).toBe(true); const oldMetrics = { total: 1, pending: 0, success: 0, error: 1, cancelled: 0 }; expect((await runs.markAsCompleted(run.id, oldMetrics, oldCounts)).affected).toBe(0); await results.markAsCompleted(result.id, { output: { finalText: 'Updated answer' } }); @@ -144,6 +145,88 @@ describe('AgentEvalRunnerService (integration)', () => { }); }); + it('repairs a failed original attempt without overwriting a newer rerun', async () => { + const project = await createTeamProject(); + const agent = await createAgent(project.id); + const dataset = await Container.get(AgentEvalDatasetRepository).createDataset({ + name: 'ds', + agentId: agent.id, + datasetSource: 'data_table', + datasetRef: { dataTableId: 'table' }, + columnMapping: { input: 'question' }, + }); + const run = await Container.get(AgentEvalRunRepository).createRun({ datasetId: dataset.id }); + const results = Container.get(AgentEvalResultRepository); + const [result, queued] = await results.seedResults([{ runId: run.id }, { runId: run.id }]); + const originalAt = new Date('2025-01-01T00:00:00.000Z'); + await results.markAsRunning(result.id, originalAt); + await results.markUnfinishedAsError(result.id, originalAt, 'Original write failed'); + expect((await results.findById(result.id))?.status).toBe('error'); + expect(await results.claimForRerun(result.id, run.id)).toBe(true); + await results.markUnfinishedAsError(result.id, originalAt, 'Late repair'); + expect((await results.findById(result.id))?.status).toBe('running'); + await results.markUnfinishedAsError(queued.id, undefined, 'Dispatch failed'); + expect((await results.findById(queued.id))?.status).toBe('error'); + }); + + it.runIf(process.env.DB_TYPE === 'postgresdb')( + 'serializes a rerun claim with a terminal statement already waiting on the run lock', + async () => { + const project = await createTeamProject(); + const agent = await createAgent(project.id); + const dataset = await Container.get(AgentEvalDatasetRepository).createDataset({ + name: 'ds', + agentId: agent.id, + datasetSource: 'data_table', + datasetRef: { dataTableId: 'table' }, + columnMapping: { input: 'question' }, + }); + const runs = Container.get(AgentEvalRunRepository); + const results = Container.get(AgentEvalResultRepository); + const run = await runs.createRun({ datasetId: dataset.id }); + await runs.markAsRunning(run.id); + const [result] = await results.seedResults([{ runId: run.id }]); + await results.markAsCompleted(result.id, { output: {} }); + const oldCounts = await results.countByStatus(run.id); + const connection = Container.get(DataSource); + const blocker = connection.createQueryRunner(); + const pending: Array> = []; + await blocker.startTransaction(); + try { + await blocker.manager.update(AgentEvalRun, { id: run.id }, { id: run.id }); + const claim = results.claimForRerun(result.id, run.id); + pending.push(claim); + const waitForLocks = async (count: number) => { + await vi.waitFor( + async () => { + const [row] = await connection.query(`SELECT count(*)::int AS count + FROM pg_stat_activity WHERE datname = current_database() + AND wait_event_type = 'Lock' AND query LIKE 'UPDATE %agent_eval_run%'`); + expect(row.count).toBe(count); + }, + { timeout: 2_000 }, + ); + }; + // The claim must hold this same lock before it can change the result. + await waitForLocks(1); + const settling = runs.markAsCompleted(run.id, { success: 1 }, oldCounts); + pending.push(settling); + await waitForLocks(2); + await blocker.commitTransaction(); + expect(await claim).toBe(true); + // Its first statement started before the claim committed. The count + // statement must use a fresh snapshot after it acquires the lock. + expect((await settling).affected).toBe(0); + expect((await runs.findById(run.id))?.status).toBe('running'); + expect((await results.findById(result.id))?.status).toBe('running'); + } finally { + if (blocker.isTransactionActive) await blocker.rollbackTransaction(); + await blocker.release(); + await Promise.allSettled(pending); + } + }, + ); + it('keeps an original batch running until a targeted rerun has settled', async () => { const project = await createTeamProject(); const agent = await createAgent(project.id); diff --git a/packages/cli/src/modules/agent-evals/__tests__/agent-eval-runner.service.test.ts b/packages/cli/src/modules/agent-evals/__tests__/agent-eval-runner.service.test.ts index 1881030fae9..003e02876a9 100644 --- a/packages/cli/src/modules/agent-evals/__tests__/agent-eval-runner.service.test.ts +++ b/packages/cli/src/modules/agent-evals/__tests__/agent-eval-runner.service.test.ts @@ -407,6 +407,39 @@ describe('AgentEvalRunnerService', () => { ); }); + it.each(['cancellation poll', 'original write'])( + 'waits for active reruns after a %s failure', + async (failure) => { + seedFor([{ id: 'row-1', question: 'First' }], { running: 1 }); + evalAgentExecutionService.executeWithLlmMock.mockResolvedValue(successExec() as never); + if (failure === 'cancellation poll') { + runRepository.isCancellationRequested + .mockResolvedValueOnce(false) + .mockResolvedValueOnce(false) + .mockRejectedValueOnce(new Error('Poll failed')) + .mockResolvedValue(false); + } else { + resultRepository.markAsCompleted.mockRejectedValueOnce(new Error('Write failed')); + resultRepository.markAsError.mockRejectedValueOnce(new Error('Error write failed')); + } + const { finished } = await service.startRun('ds-1', 'proj-1', user); + await vi.waitFor(() => expect(resultRepository.countByStatus).toHaveBeenCalled()); + // A failed original write may already have been repaired. This running + // count is a newer targeted attempt and must not be ignored. + expect(runRepository.markAsError).not.toHaveBeenCalled(); + expect(runRepository.markAsCompleted).not.toHaveBeenCalled(); + resultRepository.countByStatus.mockResolvedValue(counts({ success: 1 })); + await finished; + expect(runRepository.markAsError).toHaveBeenCalledWith( + 'run-1', + 'case_dispatch_failed', + expect.anything(), + expect.objectContaining({ success: 1, pending: 0 }), + counts({ success: 1 }), + ); + }, + ); + it('honors a cancellation received while a targeted rerun is finishing', async () => { seedFor([{ id: 'row-1', question: 'First' }], { running: 1 }); evalAgentExecutionService.executeWithLlmMock.mockResolvedValue(successExec() as never); @@ -427,14 +460,25 @@ describe('AgentEvalRunnerService', () => { seedFor([{ id: 'row-1', question: 'First' }], { running: 1 }); evalAgentExecutionService.executeWithLlmMock.mockRejectedValue(new Error('Execution failed')); resultRepository.markAsError.mockRejectedValue(new Error('Result write failed')); + resultRepository.markUnfinishedAsError + .mockRejectedValueOnce(new Error('Repair failed')) + .mockImplementationOnce(async () => { + resultRepository.countByStatus.mockResolvedValue(counts({ error: 1 })); + }); const { finished } = await service.startRun('ds-1', 'proj-1', user); await finished; expect(runRepository.markAsError).toHaveBeenCalledWith( 'run-1', 'case_dispatch_failed', expect.anything(), - expect.objectContaining({ pending: 1 }), - undefined, + expect.objectContaining({ pending: 0, error: 1 }), + counts({ error: 1 }), + ); + expect(resultRepository.markUnfinishedAsError).toHaveBeenCalledTimes(2); + expect(resultRepository.markUnfinishedAsError).toHaveBeenLastCalledWith( + 'res-0', + expect.any(Date), + 'Result write failed', ); expect(runRepository.markAsCompleted).not.toHaveBeenCalled(); }); @@ -631,7 +675,7 @@ describe('AgentEvalRunnerService', () => { 'case_dispatch_failed', expect.anything(), expect.anything(), - undefined, + counts({ success: 1, error: 1 }), ); }); @@ -1010,6 +1054,9 @@ describe('AgentEvalRunnerService', () => { .mockRejectedValueOnce(new Error('db down')) .mockResolvedValue(undefined as never); + resultRepository.markUnfinishedAsError.mockImplementationOnce(async () => { + resultRepository.countByStatus.mockResolvedValue(counts({ cancelled: 1, error: 1 })); + }); const { finished } = await service.startRun('ds-1', 'proj-1', user); await finished; @@ -1017,8 +1064,8 @@ describe('AgentEvalRunnerService', () => { expect(resultRepository.markAsCancelled).toHaveBeenCalledTimes(2); expect(runRepository.markAsCancelled).toHaveBeenCalledWith( 'run-1', - expect.objectContaining({ total: 2, cancelled: 1, pending: 1 }), - undefined, + expect.objectContaining({ total: 2, cancelled: 1, error: 1, pending: 0 }), + counts({ cancelled: 1, error: 1 }), ); expect(runRepository.markAsError).not.toHaveBeenCalled(); }); @@ -1038,6 +1085,9 @@ describe('AgentEvalRunnerService', () => { .mockResolvedValue(false); evalAgentExecutionService.executeWithLlmMock.mockResolvedValue(successExec() as never); + resultRepository.markUnfinishedAsError.mockImplementationOnce(async () => { + resultRepository.countByStatus.mockResolvedValue(counts({ success: 1, error: 1 })); + }); const { finished } = await service.startRun('ds-1', 'proj-1', user); await finished; @@ -1046,8 +1096,8 @@ describe('AgentEvalRunnerService', () => { 'run-1', 'case_dispatch_failed', expect.objectContaining({ errors: ['db down'] }), - expect.objectContaining({ total: 2, success: 1 }), - undefined, + expect.objectContaining({ total: 2, success: 1, error: 1, pending: 0 }), + counts({ success: 1, error: 1 }), ); expect(runRepository.markAsCompleted).not.toHaveBeenCalled(); }); @@ -1070,7 +1120,7 @@ describe('AgentEvalRunnerService', () => { 'case_dispatch_failed', expect.objectContaining({ errors: ['db down'] }), expect.objectContaining({ total: 1, success: 1 }), - undefined, + counts({ success: 1 }), ); // Not the tally-less `run_failed` path. expect(runRepository.markAsError).not.toHaveBeenCalledWith( @@ -1410,7 +1460,7 @@ describe('AgentEvalRunnerService', () => { 'already running', ); - expect(resultRepository.claimForRerun).toHaveBeenCalledWith('res-1'); + expect(resultRepository.claimForRerun).toHaveBeenCalledWith('res-1', 'run-1'); expect(evalAgentExecutionService.executeWithLlmMock).not.toHaveBeenCalled(); }); @@ -1484,7 +1534,7 @@ describe('AgentEvalRunnerService', () => { 'What is 2+2?', { onProgress: expect.any(Function) }, ); - expect(resultRepository.markAsRunning).toHaveBeenCalledWith('res-1'); + expect(resultRepository.markAsRunning).toHaveBeenCalledWith('res-1', expect.any(Date)); expect(resultRepository.markAsCompleted).toHaveBeenCalledWith( 'res-1', expect.objectContaining({ output: expect.objectContaining({ finalText: 'the answer' }) }), diff --git a/packages/cli/src/modules/agent-evals/agent-eval-runner.service.ts b/packages/cli/src/modules/agent-evals/agent-eval-runner.service.ts index f59e8dc19ea..1af55458889 100644 --- a/packages/cli/src/modules/agent-evals/agent-eval-runner.service.ts +++ b/packages/cli/src/modules/agent-evals/agent-eval-runner.service.ts @@ -246,7 +246,7 @@ export class AgentEvalRunnerService { // Atomic, unlike the caller's snapshot status check: of two concurrent // requests for the same result, only one wins the claim and executes. - if (!(await this.resultRepository.claimForRerun(result.id))) { + if (!(await this.resultRepository.claimForRerun(result.id, result.runId))) { throw new BadRequestError(`Agent eval result ${result.id} is already running.`); } @@ -371,6 +371,7 @@ export class AgentEvalRunnerService { }; const deadline = this.startRunDeadline(abort); + const originalAttempts = new Map(); // The stop reads/writes around `runCase` sit outside its safety net, so // collect their throws instead of letting one reject the whole pool. @@ -406,7 +407,10 @@ export class AgentEvalRunnerService { await stopCase(resultRow); return; } - const usage = await this.runCase(resultRow, resolvedCase, ctx); + const usage = await this.runCase(resultRow, resolvedCase, { + ...ctx, + onAttemptStarted: (runAt) => originalAttempts.set(resultRow.id, runAt), + }); if (usage) { totalUsage.inputTokens += usage.inputTokens; totalUsage.outputTokens += usage.outputTokens; @@ -421,10 +425,13 @@ export class AgentEvalRunnerService { deadline.clear(); const dispatchFailures: string[] = []; - for (const settlement of settlements) { + const unfinishedOriginals = new Map(); + for (const [index, settlement] of settlements.entries()) { if (settlement.status !== 'rejected') continue; const reason: unknown = settlement.reason; - dispatchFailures.push(reason instanceof Error ? reason.message : String(reason)); + const message = reason instanceof Error ? reason.message : String(reason); + dispatchFailures.push(message); + unfinishedOriginals.set(seeded[index].id, message); } let wasCancelled = cancelObserved; @@ -439,6 +446,20 @@ export class AgentEvalRunnerService { // Fence the terminal write against those attempts, including other mains. const settlementDeadline = Date.now() + RESULT_SETTLEMENT_TIMEOUT_MS; while (true) { + for (const [resultId, message] of unfinishedOriginals) { + try { + await this.resultRepository.markUnfinishedAsError( + resultId, + originalAttempts.get(resultId), + message, + ); + unfinishedOriginals.delete(resultId); + } catch (error) { + this.logger.warn(`[AgentEvalRunner] Could not settle original case ${resultId}`, { + error, + }); + } + } if (!wasCancelled) { try { wasCancelled = await this.runRepository.isCancellationRequested(runId); @@ -463,12 +484,12 @@ export class AgentEvalRunnerService { } break; } - if (dispatchFailures.length === 0 && (counts.new > 0 || counts.running > 0)) { + if (counts.new > 0 || counts.running > 0) { await sleep(RESULT_SETTLEMENT_POLL_MS); continue; } - // Failed writes can leave an original row pending. Report that failure instead of waiting for it. - const finalCounts = dispatchFailures.length === 0 ? counts : undefined; + // Dispatch failures use the same claim fence as successful completion. + const finalCounts = counts; let settled; // Every branch records the tally under `metrics`, so pollers never find a @@ -646,10 +667,13 @@ export class AgentEvalRunnerService { user: User; timeoutMs?: number; source?: AgentRunSource; + onAttemptStarted?: (runAt: Date) => void; }, ): Promise { try { - const runAt = await this.resultRepository.markAsRunning(resultRow.id); + const attemptStartedAt = new Date(); + ctx.onAttemptStarted?.(attemptStartedAt); + const runAt = await this.resultRepository.markAsRunning(resultRow.id, attemptStartedAt); const execResult = await this.evalAgentExecutionService.executeWithLlmMock( ctx.agentId, From 8573bba029b42818f105d9e83aab2349c4431d3c Mon Sep 17 00:00:00 2001 From: Luka Zivkovic Date: Thu, 15 Oct 2026 08:46:28 +0200 Subject: [PATCH 11/18] test(core): Isolate eval concurrency test control connections --- .../agent-eval-runner.service.integration.test.ts | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/packages/cli/src/modules/agent-evals/__tests__/agent-eval-runner.service.integration.test.ts b/packages/cli/src/modules/agent-evals/__tests__/agent-eval-runner.service.integration.test.ts index dc828a9a430..f63db054380 100644 --- a/packages/cli/src/modules/agent-evals/__tests__/agent-eval-runner.service.integration.test.ts +++ b/packages/cli/src/modules/agent-evals/__tests__/agent-eval-runner.service.integration.test.ts @@ -189,17 +189,24 @@ describe('AgentEvalRunnerService (integration)', () => { await results.markAsCompleted(result.id, { output: {} }); const oldCounts = await results.countByStatus(run.id); const connection = Container.get(DataSource); - const blocker = connection.createQueryRunner(); + if (connection.options.type !== 'postgres') throw new Error('Postgres is required'); + // Keep the blocker and observer outside the two-connection application + // pool so both repository transactions can acquire a connection. + const control = await new DataSource({ + ...connection.options, + poolSize: 2, + }).initialize(); + const blocker = control.createQueryRunner(); const pending: Array> = []; - await blocker.startTransaction(); try { + await blocker.startTransaction(); await blocker.manager.update(AgentEvalRun, { id: run.id }, { id: run.id }); const claim = results.claimForRerun(result.id, run.id); pending.push(claim); const waitForLocks = async (count: number) => { await vi.waitFor( async () => { - const [row] = await connection.query(`SELECT count(*)::int AS count + const [row] = await control.query(`SELECT count(*)::int AS count FROM pg_stat_activity WHERE datname = current_database() AND wait_event_type = 'Lock' AND query LIKE 'UPDATE %agent_eval_run%'`); expect(row.count).toBe(count); @@ -223,6 +230,7 @@ describe('AgentEvalRunnerService (integration)', () => { if (blocker.isTransactionActive) await blocker.rollbackTransaction(); await blocker.release(); await Promise.allSettled(pending); + await control.destroy(); } }, ); From de4d8a72191f06173f030a5971ce4bdb318d5cb4 Mon Sep 17 00:00:00 2001 From: Luka Zivkovic Date: Thu, 15 Oct 2026 08:46:52 +0200 Subject: [PATCH 12/18] fix(core): Preserve eval tool call IDs when execution completes --- .../__tests__/agent-execution.service.test.ts | 70 +++++++++ .../eval/agent-execution.service.ts | 134 +++++++++++------- .../__tests__/AgentEvalToolCalls.test.ts | 31 ++++ 3 files changed, 184 insertions(+), 51 deletions(-) diff --git a/packages/cli/src/modules/instance-ai/eval/__tests__/agent-execution.service.test.ts b/packages/cli/src/modules/instance-ai/eval/__tests__/agent-execution.service.test.ts index 1bf07cfdafc..c0db886c0ff 100644 --- a/packages/cli/src/modules/instance-ai/eval/__tests__/agent-execution.service.test.ts +++ b/packages/cli/src/modules/instance-ai/eval/__tests__/agent-execution.service.test.ts @@ -160,6 +160,63 @@ describe('EvalAgentExecutionService.executeWithLlmMock', () => { const request = { projectId: 'proj-1' }; + it.each([false, true])( + 'keeps runtime IDs for repeated calls at completion (identical: %s)', + async (identical) => { + const handlers = new Map(); + const first = { tool: 'Slack_Tool', input: { text: 'first' }, output: { sent: 'first' } }; + const second = identical + ? { ...first } + : { ...first, input: { text: 'second' }, output: { sent: 'second' } }; + const skipped = { ...first, output: 'Skipped', canceled: true }; + reconstructFromAgentEntity.mockResolvedValue({ + agent: { + on: (event: AgentEvent, handler: AgentEventHandler) => handlers.set(event, handler), + close: vi.fn(), + generate: vi.fn(async () => { + for (const [toolCallId, entry] of [ + ['first', first], + ['second', second], + ] as const) { + handlers.get(AgentEvent.ToolExecutionStart)?.({ + type: AgentEvent.ToolExecutionStart, + toolCallId, + toolName: entry.tool, + args: entry.input, + }); + } + // Completion order can differ from the original tool-call order. + for (const [toolCallId, entry] of [ + ['second', second], + ['first', first], + ] as const) { + handlers.get(AgentEvent.ToolExecutionEnd)?.({ + type: AgentEvent.ToolExecutionEnd, + toolCallId, + toolName: entry.tool, + result: entry.output, + isError: false, + }); + } + return makeGenerateResult({ toolCalls: [first, second, skipped] }); + }), + }, + }); + const result = await buildService().executeWithLlmMock( + 'agent-1', + user, + request, + 'Send twice', + { onProgress: vi.fn() }, + ); + expect(result.toolCalls).toEqual([ + expect.objectContaining({ ...first, toolCallId: 'first', status: 'completed' }), + expect.objectContaining({ ...second, toolCallId: 'second', status: 'completed' }), + expect.objectContaining({ tool: 'Slack_Tool', output: 'Skipped', error: 'canceled' }), + ]); + }, + ); + it('reports tool progress before generation finishes and flushes writes before returning', async () => { const handlers = new Map(); const onProgress = vi.fn().mockResolvedValue(undefined); @@ -385,6 +442,19 @@ describe('EvalAgentExecutionService.executeWithLlmMock', () => { ); expect(result.success).toBe(false); expect(result.errors).toContain('Agent run failed: Generation stopped'); + expect(result.toolCalls).toEqual([ + expect.objectContaining({ + toolCallId: 'failed', + status: 'error', + error: 'tool_execution_failed', + output: 'Tool failed', + }), + expect.objectContaining({ + toolCallId: 'interrupted', + status: 'error', + error: 'execution_stopped', + }), + ]); expect(onProgress).toHaveBeenLastCalledWith([ expect.objectContaining({ toolCallId: 'failed', diff --git a/packages/cli/src/modules/instance-ai/eval/agent-execution.service.ts b/packages/cli/src/modules/instance-ai/eval/agent-execution.service.ts index 209666ce3c0..1889f98ee40 100644 --- a/packages/cli/src/modules/instance-ai/eval/agent-execution.service.ts +++ b/packages/cli/src/modules/instance-ai/eval/agent-execution.service.ts @@ -1,3 +1,4 @@ +import { isDeepStrictEqual } from 'node:util'; import { AgentEvent, sanitizeToolName as sanitizeMcpToolName, @@ -497,6 +498,69 @@ export class EvalAgentExecutionService { if (!segmentToolCalls.includes(entry)) segmentToolCalls.push(entry); } }; + const terminalToolCalls = (): InstanceAiEvalAgentToolCallRecord[] => { + const kindByToolName = new Map(toolSummaries.map((tool) => [tool.name, tool.kind])); + const kindForTool = (tool: string): InstanceAiEvalAgentToolCallRecord['kind'] => + kindByToolName.get(tool) ?? + (mcpServers.some((server) => tool.startsWith(sanitizeMcpToolName(`${server.name}_`))) || + (toolLedger.get(tool) ?? []).some((request) => request.nodeType?.startsWith('mcp:')) + ? 'mcp' + : 'other'); + const reported = segmentToolCalls.map( + (entry): InstanceAiEvalAgentToolCallRecord => ({ + tool: entry.tool, + kind: kindForTool(entry.tool), + input: truncateRecordedValue(entry.input), + output: truncateRecordedValue(entry.output), + ...(entry.canceled ? { error: 'canceled' } : {}), + mocked: false, + interceptedRequests: [], + }), + ); + // Events own the IDs shown during progress. Consume matching SDK entries + // once, so repeated calls remain distinct even when they finish out of order. + const calls = [...progressCalls.values()].map((call) => { + const index = reported.findIndex( + (entry) => + call.status === 'completed' && + entry.tool === call.tool && + (call.input === undefined || isDeepStrictEqual(entry.input, call.input)) && + isDeepStrictEqual(entry.output, call.output), + ); + const entry = index >= 0 ? reported.splice(index, 1)[0] : undefined; + return { + ...entry, + ...call, + kind: kindForTool(call.tool), + ...(call.status === 'running' + ? { status: 'error' as const, error: 'execution_stopped' } + : {}), + }; + }); + // Some cancelled calls never emit execution events. Keep those records too. + calls.push(...reported); + const recordedTools = new Set(calls.map((call) => call.tool)); + for (const [tool] of toolLedger) { + if (recordedTools.has(tool)) continue; + calls.push({ + tool, + kind: kindForTool(tool), + error: + 'Not attributed to a reported tool call (a delegated sub-agent call, or an errored call) — see interceptedRequests', + mocked: false, + interceptedRequests: [], + }); + } + return calls.map((call) => { + const interceptedRequests = toolLedger.get(call.tool) ?? []; + return { + ...call, + mocked: interceptedRequests.length > 0, + interceptedRequests, + ...(autoApprovedToolNames.has(call.tool) ? { autoApproved: true } : {}), + }; + }); + }; const budgetSignal = AbortSignal.timeout(timeoutMs); try { result = await agent.generate(seed.openingMessage, { @@ -540,13 +604,19 @@ export class EvalAgentExecutionService { `Agent run exceeded its ${seconds}s eval budget and was stopped`, seed, skippedFeatures, - { modelTurns: recorder.turns, toolLedger, credentialHelpers }, + { + modelTurns: recorder.turns, + toolLedger, + credentialHelpers, + toolCalls: terminalToolCalls(), + }, ); } return this.errorResult(`Agent run failed: ${message}`, seed, skippedFeatures, { modelTurns: recorder.turns, toolLedger, credentialHelpers, + toolCalls: terminalToolCalls(), }); } finally { onLedgerChange = undefined; @@ -573,47 +643,6 @@ export class EvalAgentExecutionService { ); } - const kindByToolName = new Map(toolSummaries.map((tool) => [tool.name, tool.kind])); - // MCP summaries are keyed by SERVER name; the model calls the client-side - // prefixed `_` names. Sub-agent-brought servers aren't in - // this agent's summaries — their ledger entries carry an `mcp:` nodeType. - const kindForTool = (tool: string): InstanceAiEvalAgentToolCallRecord['kind'] => - kindByToolName.get(tool) ?? - (mcpServers.some((server) => tool.startsWith(sanitizeMcpToolName(`${server.name}_`))) || - (toolLedger.get(tool) ?? []).some((request) => request.nodeType?.startsWith('mcp:')) - ? 'mcp' - : 'other'); - const toolCalls = segmentToolCalls.map((entry): InstanceAiEvalAgentToolCallRecord => { - const interceptedRequests = toolLedger.get(entry.tool) ?? []; - return { - tool: entry.tool, - kind: kindForTool(entry.tool), - input: truncateRecordedValue(entry.input), - output: truncateRecordedValue(entry.output), - ...(entry.canceled ? { error: 'canceled' } : {}), - mocked: interceptedRequests.length > 0, - interceptedRequests, - ...(autoApprovedToolNames.has(entry.tool) ? { autoApproved: true } : {}), - }; - }); - // Ledger entries with no matching toolCalls record are either errored - // calls (GenerateResult omits those) or a delegated sub-agent's calls - // (never reported on the parent) — surface them without asserting - // failure so the judge can weigh the intercepted traffic itself. - const reportedTools = new Set(toolCalls.map((entry) => entry.tool)); - for (const [tool, interceptedRequests] of toolLedger) { - if (reportedTools.has(tool)) continue; - toolCalls.push({ - tool, - kind: kindForTool(tool), - error: - 'Not attributed to a reported tool call (a delegated sub-agent call, or an errored call) — see interceptedRequests', - mocked: interceptedRequests.length > 0, - interceptedRequests, - ...(autoApprovedToolNames.has(tool) ? { autoApproved: true } : {}), - }); - } - return { runId: result.runId, success: errors.length === 0 && result.finishReason !== 'error', @@ -621,7 +650,7 @@ export class EvalAgentExecutionService { finalText: extractFinalAssistantText(result), model: result.model, finishReason: result.finishReason, - toolCalls, + toolCalls: terminalToolCalls(), modelTurns: recorder.turns, ...(result.usage ? { @@ -678,6 +707,7 @@ export class EvalAgentExecutionService { modelTurns: InstanceAiEvalAgentExecutionResult['modelTurns']; toolLedger: ToolLedger; credentialHelpers: EvalMockedCredentialsHelper[]; + toolCalls?: InstanceAiEvalAgentToolCallRecord[]; }, ): InstanceAiEvalAgentExecutionResult { this.logger.error(`[EvalAgentMock] ${message}`); @@ -686,14 +716,16 @@ export class EvalAgentExecutionService { success: false, errors: [message], finalText: '', - toolCalls: partial - ? [...partial.toolLedger.entries()].map(([tool, interceptedRequests]) => ({ - tool, - kind: salvagedToolKind(interceptedRequests), - mocked: interceptedRequests.length > 0, - interceptedRequests, - })) - : [], + toolCalls: + partial?.toolCalls ?? + (partial + ? [...partial.toolLedger.entries()].map(([tool, interceptedRequests]) => ({ + tool, + kind: salvagedToolKind(interceptedRequests), + mocked: interceptedRequests.length > 0, + interceptedRequests, + })) + : []), modelTurns: partial?.modelTurns ?? [], seed: seed ?? { openingMessage: '', globalContext: '', toolHints: {}, warnings: [] }, skippedFeatures, diff --git a/packages/frontend/editor-ui/src/features/agents/__tests__/AgentEvalToolCalls.test.ts b/packages/frontend/editor-ui/src/features/agents/__tests__/AgentEvalToolCalls.test.ts index c1cf5ec05ff..f69e1b497c2 100644 --- a/packages/frontend/editor-ui/src/features/agents/__tests__/AgentEvalToolCalls.test.ts +++ b/packages/frontend/editor-ui/src/features/agents/__tests__/AgentEvalToolCalls.test.ts @@ -2,6 +2,7 @@ import { createTestingPinia } from '@pinia/testing'; import userEvent from '@testing-library/user-event'; import { createComponentRenderer } from '@/__tests__/render'; +import { TOOL_CALL_STATE } from '@/features/ai/shared/agentsChat/constants'; import AgentEvalToolCalls from '../components/AgentEvalToolCalls.vue'; import { toDisplayToolCalls } from '../utils/agent-eval-tool-calls'; @@ -64,6 +65,36 @@ it('keeps legacy calls free of mock claims', () => { expect(queryByText(/Mocked/)).not.toBeInTheDocument(); }); +it.each([false, true])( + 'keeps the tool disclosure open when the result completes (grouped: %s)', + async (grouped) => { + const user = userEvent.setup(); + const { getByText, getByRole, rerender } = renderComponent({ + props: { toolCalls: calls(true, grouped) }, + }); + if (grouped) await user.click(getByText('2 tool calls')); + const trigger = getByRole('button', { name: /Send and wait/ }); + await user.click(trigger); + const contentId = trigger.getAttribute('aria-controls'); + expect(getByText('Input')).toBeInTheDocument(); + await rerender({ + toolCalls: calls(true, grouped).map((call) => ({ + ...call, + state: TOOL_CALL_STATE.DONE, + output: { ok: true }, + })), + }); + expect(getByRole('button', { name: /Send and wait/ })).toHaveAttribute( + 'aria-controls', + contentId, + ); + expect(getByRole('button', { name: /Send and wait/ })).toHaveAttribute('aria-expanded', 'true'); + expect(getByText('Input')).toBeInTheDocument(); + expect(getByText('Output')).toBeInTheDocument(); + expect(getByText('Mocked requests')).toBeInTheDocument(); + }, +); + it('opens recorded mock details when the tool has no input or output', async () => { const user = userEvent.setup(); const { getByText } = renderComponent({ From f690debf069f47f1b9fb875d01155d9666a2914a Mon Sep 17 00:00:00 2001 From: Luka Zivkovic Date: Thu, 15 Oct 2026 09:01:30 +0200 Subject: [PATCH 13/18] test(core): Allow concurrent eval transactions in CI --- .../__tests__/agent-eval-runner.service.integration.test.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/cli/src/modules/agent-evals/__tests__/agent-eval-runner.service.integration.test.ts b/packages/cli/src/modules/agent-evals/__tests__/agent-eval-runner.service.integration.test.ts index f63db054380..1ef843d7f52 100644 --- a/packages/cli/src/modules/agent-evals/__tests__/agent-eval-runner.service.integration.test.ts +++ b/packages/cli/src/modules/agent-evals/__tests__/agent-eval-runner.service.integration.test.ts @@ -85,6 +85,9 @@ async function createAgent(projectId: string): Promise { } beforeAll(async () => { + // The contention test needs two concurrent repository transactions. CI + // otherwise uses one connection to detect accidental nested transactions. + Container.get(GlobalConfig).database.postgresdb.poolSize = 2; await testModules.loadModules(['data-table', 'agents']); await testDb.init(); owner = await createUserShell(GLOBAL_OWNER_ROLE); From 1ed1538e0023a05fc814f4254bb2c3a9276766b8 Mon Sep 17 00:00:00 2001 From: Luka Zivkovic Date: Thu, 15 Oct 2026 09:05:54 +0200 Subject: [PATCH 14/18] fix(core): Keep agent eval settlement results in domain terms --- .../agent-eval-run.repository.test.ts | 56 +++++++++++++++++++ .../agent-eval-run.repository.ee.ts | 16 ++++-- .../agent-eval-runner.service.test.ts | 22 ++++++-- .../agent-evals/agent-eval-runner.service.ts | 2 +- 4 files changed, 85 insertions(+), 11 deletions(-) diff --git a/packages/@n8n/db/src/repositories/__tests__/agent-eval-run.repository.test.ts b/packages/@n8n/db/src/repositories/__tests__/agent-eval-run.repository.test.ts index 0109558eec3..5244389a059 100644 --- a/packages/@n8n/db/src/repositories/__tests__/agent-eval-run.repository.test.ts +++ b/packages/@n8n/db/src/repositories/__tests__/agent-eval-run.repository.test.ts @@ -5,6 +5,7 @@ import type { Mock } from 'vitest'; import { AgentEvalRun } from '../../entities/agent-eval-run.ee'; import { mockEntityManager } from '../../utils/test-utils/mock-entity-manager'; import { AgentEvalRunRepository } from '../agent-eval-run.repository.ee'; +import type { AgentEvalResultStatusCounts } from '../agent-eval-result.repository.ee'; describe('AgentEvalRunRepository', () => { const entityManager = mockEntityManager(AgentEvalRun); @@ -17,6 +18,61 @@ describe('AgentEvalRunRepository', () => { vi.resetAllMocks(); }); + describe.each([ + [ + 'completed', + async (counts?: AgentEvalResultStatusCounts) => + await repo.markAsCompleted('run-1', {}, counts), + ], + [ + 'error', + async (counts?: AgentEvalResultStatusCounts) => + await repo.markAsError('run-1', 'failed', null, null, counts), + ], + [ + 'cancelled', + async (counts?: AgentEvalResultStatusCounts) => + await repo.markAsCancelled('run-1', null, counts), + ], + ] as const)('%s settlement', (_status, settle) => { + it.each([ + { affected: 1, expected: true }, + { affected: 0, expected: false }, + { affected: undefined, expected: false }, + ])( + 'returns $expected for an update affecting $affected rows', + async ({ affected, expected }) => { + entityManager.update.mockResolvedValueOnce({ affected, generatedMaps: [], raw: [] }); + + expect(await settle()).toBe(expected); + }, + ); + + it.each([ + { affected: 1, expected: true }, + { affected: 0, expected: false }, + { affected: undefined, expected: false }, + ])( + 'returns $expected for a count-guarded update affecting $affected rows', + async ({ affected, expected }) => { + const query = { + update: vi.fn().mockReturnThis(), + set: vi.fn().mockReturnThis(), + select: vi.fn().mockReturnThis(), + where: vi.fn().mockReturnThis(), + andWhere: vi.fn().mockReturnThis(), + getQuery: vi.fn().mockReturnValue('SELECT COUNT(*)'), + execute: vi.fn().mockResolvedValue({ affected, generatedMaps: [], raw: [] }), + }; + (entityManager.createQueryBuilder as Mock).mockReturnValue(query); + + expect(await settle({ new: 0, running: 0, success: 1, error: 0, cancelled: 0 })).toBe( + expected, + ); + }, + ); + }); + describe('createRun', () => { it('starts a run as "new" with cancellation cleared', async () => { (entityManager.create as Mock).mockImplementation( diff --git a/packages/@n8n/db/src/repositories/agent-eval-run.repository.ee.ts b/packages/@n8n/db/src/repositories/agent-eval-run.repository.ee.ts index 7950ff1e4fd..fdd0b50bf03 100644 --- a/packages/@n8n/db/src/repositories/agent-eval-run.repository.ee.ts +++ b/packages/@n8n/db/src/repositories/agent-eval-run.repository.ee.ts @@ -44,7 +44,7 @@ export class AgentEvalRunRepository extends BaseRepository { id: string, metrics: IDataObject | null, counts?: AgentEvalResultStatusCounts, - ) { + ): Promise { return await this.settleWithResultCounts( id, { @@ -67,7 +67,7 @@ export class AgentEvalRunRepository extends BaseRepository { errorDetails?: IDataObject | null, metrics?: IDataObject | null, counts?: AgentEvalResultStatusCounts, - ) { + ): Promise { return await this.settleWithResultCounts( id, { @@ -91,7 +91,7 @@ export class AgentEvalRunRepository extends BaseRepository { id: string, metrics: IDataObject | null = null, counts?: AgentEvalResultStatusCounts, - ) { + ): Promise { return await this.settleWithResultCounts( id, { @@ -109,8 +109,11 @@ export class AgentEvalRunRepository extends BaseRepository { id: string, attributes: QueryDeepPartialEntity, counts?: AgentEvalResultStatusCounts, - ) { - if (!counts) return await this.update(id, attributes); + ): Promise { + if (!counts) { + const result = await this.update(id, attributes); + return (result.affected ?? 0) > 0; + } return await this.runInTransaction({}, async (manager) => { // Acquire the claim lock in a separate statement. Postgres must take a new // snapshot for the count predicates after a concurrent claim commits. @@ -135,7 +138,8 @@ export class AgentEvalRunRepository extends BaseRepository { [`count_${status}`]: expected, }); } - return await query.execute(); + const result = await query.execute(); + return (result.affected ?? 0) > 0; }); } diff --git a/packages/cli/src/modules/agent-evals/__tests__/agent-eval-runner.service.test.ts b/packages/cli/src/modules/agent-evals/__tests__/agent-eval-runner.service.test.ts index 003e02876a9..92a32bc0773 100644 --- a/packages/cli/src/modules/agent-evals/__tests__/agent-eval-runner.service.test.ts +++ b/packages/cli/src/modules/agent-evals/__tests__/agent-eval-runner.service.test.ts @@ -164,10 +164,9 @@ describe('AgentEvalRunnerService', () => { moduleRegistry.isActive.mockReturnValue(true); datasetRepository = mock(); runRepository = mock(); - const updated = { affected: 1, raw: [], generatedMaps: [] }; - runRepository.markAsCompleted.mockResolvedValue(updated); - runRepository.markAsCancelled.mockResolvedValue(updated); - runRepository.markAsError.mockResolvedValue(updated); + runRepository.markAsCompleted.mockResolvedValue(true); + runRepository.markAsCancelled.mockResolvedValue(true); + runRepository.markAsError.mockResolvedValue(true); resultRepository = mock(); agentRepository = mock(); dataTableService = mock(); @@ -345,6 +344,21 @@ describe('AgentEvalRunnerService', () => { }); describe('running cases', () => { + it('retries stale settlement until the repository confirms completion', async () => { + seedFor([{ id: 'row-1', question: 'First' }], { success: 1 }); + evalAgentExecutionService.executeWithLlmMock.mockResolvedValue(successExec() as never); + runRepository.markAsCompleted.mockResolvedValueOnce(false).mockResolvedValue(true); + runRepository.findById + .mockResolvedValueOnce(mock({ id: 'run-1', status: 'running' })) + .mockResolvedValue(mock({ id: 'run-1', status: 'completed' })); + + const { finished } = await service.startRun('ds-1', 'proj-1', user); + await finished; + + expect(runRepository.markAsCompleted).toHaveBeenCalledTimes(2); + expect(runRepository.findById).toHaveBeenCalledOnce(); + }); + it('keeps one config snapshot when instructions change with a queued case', async () => { seedFor( [ diff --git a/packages/cli/src/modules/agent-evals/agent-eval-runner.service.ts b/packages/cli/src/modules/agent-evals/agent-eval-runner.service.ts index 1af55458889..28becdc504b 100644 --- a/packages/cli/src/modules/agent-evals/agent-eval-runner.service.ts +++ b/packages/cli/src/modules/agent-evals/agent-eval-runner.service.ts @@ -528,7 +528,7 @@ export class AgentEvalRunnerService { } settled = await this.runRepository.markAsCompleted(runId, metrics, finalCounts); } - if ((settled.affected ?? 0) > 0) break; + if (settled) break; const current = await this.runRepository.findById(runId); if (!current || current.status !== 'running') break; await sleep(RESULT_SETTLEMENT_POLL_MS); From b3912e45bf947be92fa350b3a77e4dbea8f65684 Mon Sep 17 00:00:00 2001 From: Luka Zivkovic Date: Thu, 15 Oct 2026 09:06:09 +0200 Subject: [PATCH 15/18] test(core): Assert eval terminal transition outcomes --- .../agent-eval-runner.service.integration.test.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/cli/src/modules/agent-evals/__tests__/agent-eval-runner.service.integration.test.ts b/packages/cli/src/modules/agent-evals/__tests__/agent-eval-runner.service.integration.test.ts index 1ef843d7f52..39b228fff4e 100644 --- a/packages/cli/src/modules/agent-evals/__tests__/agent-eval-runner.service.integration.test.ts +++ b/packages/cli/src/modules/agent-evals/__tests__/agent-eval-runner.service.integration.test.ts @@ -134,14 +134,14 @@ describe('AgentEvalRunnerService (integration)', () => { const oldCounts = await results.countByStatus(run.id); expect(await results.claimForRerun(result.id, result.runId)).toBe(true); const oldMetrics = { total: 1, pending: 0, success: 0, error: 1, cancelled: 0 }; - expect((await runs.markAsCompleted(run.id, oldMetrics, oldCounts)).affected).toBe(0); + expect(await runs.markAsCompleted(run.id, oldMetrics, oldCounts)).toBe(false); await results.markAsCompleted(result.id, { output: { finalText: 'Updated answer' } }); - expect((await runs.markAsCompleted(run.id, oldMetrics, oldCounts)).affected).toBe(0); + expect(await runs.markAsCompleted(run.id, oldMetrics, oldCounts)).toBe(false); const currentCounts = await results.countByStatus(run.id); const currentMetrics = { ...oldMetrics, success: 1, error: 0 }; await runs.requestCancellation(run.id); - expect((await runs.markAsCompleted(run.id, currentMetrics, currentCounts)).affected).toBe(0); - expect((await runs.markAsCancelled(run.id, currentMetrics, currentCounts)).affected).toBe(1); + expect(await runs.markAsCompleted(run.id, currentMetrics, currentCounts)).toBe(false); + expect(await runs.markAsCancelled(run.id, currentMetrics, currentCounts)).toBe(true); expect(await runs.findById(run.id)).toMatchObject({ status: 'cancelled', metrics: currentMetrics, @@ -200,7 +200,7 @@ describe('AgentEvalRunnerService (integration)', () => { poolSize: 2, }).initialize(); const blocker = control.createQueryRunner(); - const pending: Array> = []; + const pending: Promise[] = []; try { await blocker.startTransaction(); await blocker.manager.update(AgentEvalRun, { id: run.id }, { id: run.id }); @@ -226,7 +226,7 @@ describe('AgentEvalRunnerService (integration)', () => { expect(await claim).toBe(true); // Its first statement started before the claim committed. The count // statement must use a fresh snapshot after it acquires the lock. - expect((await settling).affected).toBe(0); + expect(await settling).toBe(false); expect((await runs.findById(run.id))?.status).toBe('running'); expect((await results.findById(result.id))?.status).toBe('running'); } finally { From 4d17b47c38a3dda6ad729c7881856e6c5e67b83e Mon Sep 17 00:00:00 2001 From: Luka Zivkovic Date: Fri, 16 Oct 2026 13:13:47 +0200 Subject: [PATCH 16/18] test(core): Cover agent eval progress boundaries --- .../__tests__/agent-execution.service.test.ts | 109 ++++++++++++++++++ .../eval/agent-execution.service.ts | 68 ++++------- .../agents/__tests__/AgentEvalTryRow.test.ts | 11 ++ .../agents/__tests__/agentEvals.store.test.ts | 73 ++++++++++++ .../src/features/agents/agentEvals.store.ts | 7 +- .../agents/components/AgentEvalTryRow.vue | 2 +- .../__tests__/agent-eval-tool-calls.test.ts | 6 + 7 files changed, 223 insertions(+), 53 deletions(-) diff --git a/packages/cli/src/modules/instance-ai/eval/__tests__/agent-execution.service.test.ts b/packages/cli/src/modules/instance-ai/eval/__tests__/agent-execution.service.test.ts index c0db886c0ff..a851326fb56 100644 --- a/packages/cli/src/modules/instance-ai/eval/__tests__/agent-execution.service.test.ts +++ b/packages/cli/src/modules/instance-ai/eval/__tests__/agent-execution.service.test.ts @@ -29,6 +29,7 @@ import { import { generateAgentScenarioSeed } from '../agent-scenario-seed'; import { createMcpMockFetch } from '../mcp-mock-fetch'; import { createLlmMockHandler } from '../mock-handler'; +import { createWebSearchMock } from '../web-search-mock'; // The service resolves the agents-module surface lazily; top-level vi.mock // keeps those specifiers from loading the real module graph (and avoids the @@ -54,6 +55,7 @@ vi.mock('@/utils/ai-proxy-fetch', () => ({ createAiProxyFetch: vi.fn(() => vi.fn vi.mock('../agent-scenario-seed', () => ({ generateAgentScenarioSeed: vi.fn() })); vi.mock('../mcp-mock-fetch', () => ({ createMcpMockFetch: vi.fn(() => vi.fn()) })); vi.mock('../mock-handler', () => ({ createLlmMockHandler: vi.fn() })); +vi.mock('../web-search-mock', () => ({ createWebSearchMock: vi.fn(() => vi.fn()) })); const logger = mock(); const settingsService = mock(); @@ -217,6 +219,113 @@ describe('EvalAgentExecutionService.executeWithLlmMock', () => { }, ); + it('records search progress only for the matching active call and ignores late endings', async () => { + const handlers = new Map(); + const onProgress = vi.fn().mockResolvedValue(undefined); + const search = { query: 'shipping' }; + const searchResult = { query: 'shipping', results: [] }; + reconstructFromAgentEntity.mockResolvedValue({ + agent: { + on: (event: AgentEvent, handler: AgentEventHandler) => handlers.set(event, handler), + close: vi.fn(), + generate: vi.fn(async () => { + for (const toolName of ['Slack_Tool', 'web_search']) { + handlers.get(AgentEvent.ToolExecutionStart)?.({ + type: AgentEvent.ToolExecutionStart, + toolName, + toolCallId: toolName, + args: search, + }); + } + const { onSearch } = vi.mocked(createWebSearchMock).mock.calls[0][0]; + onSearch(search, searchResult); + handlers.get(AgentEvent.ToolExecutionEnd)?.({ + type: AgentEvent.ToolExecutionEnd, + toolName: 'web_search', + toolCallId: 'web_search', + result: searchResult, + isError: false, + }); + // A delegated search has no active parent call to update. + onSearch({ query: 'returns' }, { query: 'returns', results: [] }); + // A runtime can report completion without a prior start event. + handlers.get(AgentEvent.ToolExecutionEnd)?.({ + type: AgentEvent.ToolExecutionEnd, + toolName: 'lookup', + toolCallId: 'orphan', + result: 'found', + isError: false, + }); + return makeGenerateResult({ + toolCalls: [{ tool: 'lookup', input: {}, output: 'found' }], + }); + }), + }, + }); + const result = await buildService().executeWithLlmMock('agent-1', user, request, 'Search', { + onProgress, + }); + expect(onProgress.mock.calls[2][0]).toEqual([ + expect.objectContaining({ tool: 'Slack_Tool', mocked: false, interceptedRequests: [] }), + expect.objectContaining({ + tool: 'web_search', + status: 'running', + mocked: true, + interceptedRequests: [expect.objectContaining({ requestBody: search })], + }), + ]); + expect(onProgress).toHaveBeenCalledTimes(6); + expect( + result.toolCalls.find((call) => call.tool === 'web_search')?.interceptedRequests, + ).toHaveLength(2); + expect(result.toolCalls.find((call) => call.toolCallId === 'orphan')).toMatchObject({ + kind: 'other', + output: 'found', + }); + handlers.get(AgentEvent.ToolExecutionEnd)?.({ + type: AgentEvent.ToolExecutionEnd, + toolName: 'lookup', + toolCallId: 'late', + result: 'ignored', + isError: false, + }); + await Promise.resolve(); + expect(onProgress).toHaveBeenCalledTimes(6); + }); + + it('classifies delegated MCP records from their intercepted request metadata', async () => { + reconstructFromAgentEntity.mockResolvedValue({ + agent: { + close: vi.fn(), + generate: vi.fn(async () => { + vi.mocked(createMcpMockFetch).mock.calls[0][0].onToolCall({ + serverName: 'child', + toolName: 'lookup', + args: {}, + result: { text: 'found', isError: false }, + }); + const instrumentation = reconstructFromAgentEntity.mock.calls[0][5] as { + onMcpToolCallSettled: (event: { + serverName: string; + toolName: string; + success: boolean; + }) => void; + }; + instrumentation.onMcpToolCallSettled({ + serverName: 'child', + toolName: 'lookup', + success: true, + }); + return makeGenerateResult({ toolCalls: [] }); + }), + }, + }); + const result = await buildService().executeWithLlmMock('agent-1', user, request); + expect(result.toolCalls).toEqual([ + expect.objectContaining({ tool: 'child_lookup', kind: 'mcp', mocked: true }), + ]); + }); + it('reports tool progress before generation finishes and flushes writes before returning', async () => { const handlers = new Map(); const onProgress = vi.fn().mockResolvedValue(undefined); diff --git a/packages/cli/src/modules/instance-ai/eval/agent-execution.service.ts b/packages/cli/src/modules/instance-ai/eval/agent-execution.service.ts index 1889f98ee40..860947bc9e9 100644 --- a/packages/cli/src/modules/instance-ai/eval/agent-execution.service.ts +++ b/packages/cli/src/modules/instance-ai/eval/agent-execution.service.ts @@ -265,7 +265,9 @@ export class EvalAgentExecutionService { }); const toolLedger: ToolLedger = new Map(); - let onLedgerChange: ((toolName: string) => void) | undefined; + let onLedgerChange: + | ((toolName: string, entries: InstanceAiEvalInterceptedRequest[]) => void) + | undefined; const credentialHelpers: EvalMockedCredentialsHelper[] = []; const recorder = createAgentModelTurnRecorder( createAiProxyFetch(this.outboundHttp), @@ -324,7 +326,7 @@ export class EvalAgentExecutionService { requestBody: call.args, mockResponse: call.result, }); - onLedgerChange?.(key); + onLedgerChange?.(key, entries); }; // Fallback web_search mock — always created (sub-agents may enable web @@ -348,7 +350,7 @@ export class EvalAgentExecutionService { requestBody: args, mockResponse: result, }); - onLedgerChange?.('web_search'); + onLedgerChange?.('web_search', entries); }, }); @@ -403,7 +405,7 @@ export class EvalAgentExecutionService { mockHandler, toolContext.toolName, toolLedger, - () => onLedgerChange?.(toolContext.toolName), + (entries) => onLedgerChange?.(toolContext.toolName, entries), ); }, }, @@ -420,24 +422,24 @@ export class EvalAgentExecutionService { const progressCalls = new Map(); let progressWrites = Promise.resolve(); let progressClosed = false; - const publishProgress = () => { - if (progressClosed || !hooks?.onProgress) return; + const onProgress = hooks?.onProgress; + const publishProgress = ( + writeProgress: (calls: InstanceAiEvalAgentToolCallRecord[]) => Promise, + ) => { const calls = [...progressCalls.values()].map((call) => ({ ...call })); progressWrites = progressWrites - .then(async () => await hooks.onProgress?.(calls)) + .then(async () => await writeProgress(calls)) .catch((error: unknown) => { this.logger.warn('[EvalAgentMock] Could not record tool progress', { error }); }); }; - if (hooks?.onProgress) { + if (onProgress) { // A send-and-wait tool records its mocked send before the tool ends. // Notify synchronously; the existing queue owns persistence and ordering. - onLedgerChange = (toolName) => { - if (progressClosed) return; + onLedgerChange = (toolName, interceptedRequests) => { let changed = false; for (const [id, call] of progressCalls) { if (call.tool !== toolName || call.status !== 'running') continue; - const interceptedRequests = toolLedger.get(toolName) ?? []; progressCalls.set(id, { ...call, mocked: interceptedRequests.length > 0, @@ -445,7 +447,7 @@ export class EvalAgentExecutionService { }); changed = true; } - if (changed) publishProgress(); + if (changed) publishProgress(onProgress); }; agent.on(AgentEvent.ToolExecutionStart, (event) => { if (progressClosed || event.type !== AgentEvent.ToolExecutionStart) return; @@ -458,7 +460,7 @@ export class EvalAgentExecutionService { mocked: false, interceptedRequests: [], }); - publishProgress(); + publishProgress(onProgress); }); agent.on(AgentEvent.ToolExecutionEnd, (event) => { if (progressClosed || event.type !== AgentEvent.ToolExecutionEnd) return; @@ -475,7 +477,7 @@ export class EvalAgentExecutionService { mocked: interceptedRequests.length > 0, interceptedRequests: [...interceptedRequests], }); - publishProgress(); + publishProgress(onProgress); }); } @@ -606,7 +608,6 @@ export class EvalAgentExecutionService { skippedFeatures, { modelTurns: recorder.turns, - toolLedger, credentialHelpers, toolCalls: terminalToolCalls(), }, @@ -614,7 +615,6 @@ export class EvalAgentExecutionService { } return this.errorResult(`Agent run failed: ${message}`, seed, skippedFeatures, { modelTurns: recorder.turns, - toolLedger, credentialHelpers, toolCalls: terminalToolCalls(), }); @@ -624,7 +624,7 @@ export class EvalAgentExecutionService { if (call.status === 'running') progressCalls.set(id, { ...call, status: 'error', error: 'execution_stopped' }); } - if (progressCalls.size > 0) publishProgress(); + if (onProgress && progressCalls.size > 0) publishProgress(onProgress); progressClosed = true; await progressWrites; await recorder.flush(); @@ -675,7 +675,7 @@ export class EvalAgentExecutionService { mockHandler: EvalLlmMockHandler, toolName: string, ledger: ToolLedger, - onRecorded?: () => void, + onRecorded?: (entries: InstanceAiEvalInterceptedRequest[]) => void, ): EvalLlmMockHandler { return async (requestOptions, node) => { const response = await mockHandler(requestOptions, node); @@ -691,7 +691,7 @@ export class EvalAgentExecutionService { requestBody: requestOptions.body, mockResponse: snapshotLedgerBody(response?.body), }); - onRecorded?.(); + onRecorded?.(entries); this.logger.debug( `[EvalAgentMock] Intercepted ${requestOptions.method ?? 'GET'} ${requestOptions.url} from tool "${toolName}" (${node.type})`, ); @@ -705,9 +705,8 @@ export class EvalAgentExecutionService { skippedFeatures: InstanceAiEvalAgentSkippedFeature[] = [], partial?: { modelTurns: InstanceAiEvalAgentExecutionResult['modelTurns']; - toolLedger: ToolLedger; credentialHelpers: EvalMockedCredentialsHelper[]; - toolCalls?: InstanceAiEvalAgentToolCallRecord[]; + toolCalls: InstanceAiEvalAgentToolCallRecord[]; }, ): InstanceAiEvalAgentExecutionResult { this.logger.error(`[EvalAgentMock] ${message}`); @@ -716,16 +715,7 @@ export class EvalAgentExecutionService { success: false, errors: [message], finalText: '', - toolCalls: - partial?.toolCalls ?? - (partial - ? [...partial.toolLedger.entries()].map(([tool, interceptedRequests]) => ({ - tool, - kind: salvagedToolKind(interceptedRequests), - mocked: interceptedRequests.length > 0, - interceptedRequests, - })) - : []), + toolCalls: partial?.toolCalls ?? [], modelTurns: partial?.modelTurns ?? [], seed: seed ?? { openingMessage: '', globalContext: '', toolHints: {}, warnings: [] }, skippedFeatures, @@ -800,22 +790,6 @@ export function pruneConfigForEval(original: AgentJsonConfig): { * tools, `sanitizeToolName` for workflow tools, the stored descriptor name * for custom tools. */ -/** - * Best-effort kind for a salvaged ledger entry on the failure path, where the - * summary map is gone: MCP and web-search entries are self-describing via - * nodeType; anything else with a nodeType came through the node/workflow - * engine seam. - */ -function salvagedToolKind( - interceptedRequests: InstanceAiEvalInterceptedRequest[], -): InstanceAiEvalAgentToolCallRecord['kind'] { - const nodeType = interceptedRequests.find((request) => request.nodeType)?.nodeType; - if (!nodeType) return 'other'; - if (nodeType.startsWith('mcp:')) return 'mcp'; - if (nodeType === 'web-search:fallback') return 'other'; - return 'node'; -} - export function summarizeTools( config: AgentJsonConfig, customTools: NonNullable, diff --git a/packages/frontend/editor-ui/src/features/agents/__tests__/AgentEvalTryRow.test.ts b/packages/frontend/editor-ui/src/features/agents/__tests__/AgentEvalTryRow.test.ts index 871295daa60..546585765fe 100644 --- a/packages/frontend/editor-ui/src/features/agents/__tests__/AgentEvalTryRow.test.ts +++ b/packages/frontend/editor-ui/src/features/agents/__tests__/AgentEvalTryRow.test.ts @@ -30,6 +30,17 @@ describe('AgentEvalTryRow', () => { }); describe('view="complete"', () => { + it('announces both disclosure states on the complete row', async () => { + const user = userEvent.setup(); + const { getByTestId } = renderComponent({ props: { testId: 'row' } }); + const toggle = getByTestId('row-toggle'); + expect(toggle).toHaveAttribute('aria-expanded', 'false'); + await user.click(toggle); + expect(toggle).toHaveAttribute('aria-expanded', 'true'); + await user.click(toggle); + expect(toggle).toHaveAttribute('aria-expanded', 'false'); + }); + it.each([ ['pass', 'Passes'], ['strong', 'Passes'], diff --git a/packages/frontend/editor-ui/src/features/agents/__tests__/agentEvals.store.test.ts b/packages/frontend/editor-ui/src/features/agents/__tests__/agentEvals.store.test.ts index 577ca34f433..6a0b79633ef 100644 --- a/packages/frontend/editor-ui/src/features/agents/__tests__/agentEvals.store.test.ts +++ b/packages/frontend/editor-ui/src/features/agents/__tests__/agentEvals.store.test.ts @@ -1722,6 +1722,79 @@ describe('useAgentEvalsStore', () => { }); describe('rerunResult', () => { + it('ignores a timer task already queued when a rerun finishes', async () => { + vi.useFakeTimers(); + mockRun({ results: [result('c1')], count: 1, ratings: [] }); + const store = useAgentEvalsStore(); + await store.openRun(PROJECT_ID, AGENT_ID, RUN_ID); + let finish!: (value: AgentEvalResultRecord) => void; + rerunResult.mockImplementationOnce( + async () => + await new Promise((resolve) => { + finish = resolve; + }), + ); + const timer = vi.spyOn(globalThis, 'setTimeout'); + const pending = store.rerunResult(PROJECT_ID, AGENT_ID, 'c1'); + const queued = timer.mock.calls.find(([, delay]) => delay === 5000)?.[0]; + expect(typeof queued).toBe('function'); + finish(result('c1')); + await pending; + getRunDetail.mockClear(); + if (typeof queued !== 'function') throw new Error('Missing progress timer'); + await queued(); + expect(getRunDetail).not.toHaveBeenCalled(); + expect(store.getReview(RUN_ID).results[0]).toEqual(result('c1')); + expect(vi.getTimerCount()).toBe(0); + timer.mockRestore(); + }); + + it.each([false, true])('handles an uncached rerun (request fails: %s)', async (fails) => { + const store = useAgentEvalsStore(); + if (fails) rerunResult.mockRejectedValueOnce(new Error('Unavailable')); + else rerunResult.mockResolvedValueOnce(result('uncached')); + const pending = store.rerunResult(PROJECT_ID, AGENT_ID, 'uncached'); + if (fails) await expect(pending).rejects.toThrow('Unavailable'); + else expect(await pending).toEqual(result('uncached')); + expect(getRunDetail).not.toHaveBeenCalled(); + }); + + it('preserves a newer attempt when a later refresh returns an older runAt', async () => { + const newer = { ...result('c1'), runAt: '2026-01-01T01:00:00.000Z' }; + mockRun({ results: [newer], count: 1, ratings: [] }); + const store = useAgentEvalsStore(); + await store.openRun(PROJECT_ID, AGENT_ID, RUN_ID); + getRunDetail.mockResolvedValueOnce(runDetail([result('c1')], 1)); + await store.openRun(PROJECT_ID, AGENT_ID, RUN_ID); + expect(store.getReview(RUN_ID).results[0]).toEqual(newer); + }); + + it('pauses normal progress reads while the document is hidden', async () => { + vi.useFakeTimers(); + const visibility = vi.spyOn(document, 'hidden', 'get').mockReturnValue(true); + mockRun({ results: [result('c1')], count: 1, ratings: [] }); + const store = useAgentEvalsStore(); + await store.openRun(PROJECT_ID, AGENT_ID, RUN_ID); + let finish!: (value: AgentEvalResultRecord) => void; + rerunResult.mockImplementationOnce( + async () => + await new Promise((resolve) => { + finish = resolve; + }), + ); + const pending = store.rerunResult(PROJECT_ID, AGENT_ID, 'c1'); + getRunDetail.mockClear(); + await vi.advanceTimersByTimeAsync(5000); + expect(getRunDetail).not.toHaveBeenCalled(); + visibility.mockReturnValue(false); + await vi.advanceTimersByTimeAsync(5000); + expect(getRunDetail).toHaveBeenCalledOnce(); + finish(result('c1')); + await pending; + expect(vi.getTimerCount()).toBe(0); + visibility.mockRestore(); + }); + it('patches the cached result to running before the request lands, then to the real response', async () => { mockRun({ results: [{ ...result('c1'), status: 'error' }], count: 1, ratings: [] }); const store = useAgentEvalsStore(); diff --git a/packages/frontend/editor-ui/src/features/agents/agentEvals.store.ts b/packages/frontend/editor-ui/src/features/agents/agentEvals.store.ts index b55dac36b69..90514432d7a 100644 --- a/packages/frontend/editor-ui/src/features/agents/agentEvals.store.ts +++ b/packages/frontend/editor-ui/src/features/agents/agentEvals.store.ts @@ -1182,14 +1182,13 @@ export const useAgentEvalsStore = defineStore(STORES.AGENT_EVALS, () => { const ownsProgress = () => rerunningResults.get(result.id) === pending; revisionByResult.set(result.id, ++resultRevision); let timer: ReturnType | undefined; - let stopped = false; let errors = 0; const deadline = Date.now() + RUN_POLL_TIMEOUT_MS; const confirmationDeadline = Date.now() + RESULT_CONFIRMATION_TIMEOUT_MS; const schedule = () => { - if (stopped || !ownsProgress()) return; + if (!ownsProgress()) return; timer = setTimeout(async () => { - if (stopped || !ownsProgress()) return; + if (!ownsProgress()) return; const current = findCachedResult(result.id)?.result; const confirmationExpired = recovering && current?.runAt === result.runAt && Date.now() >= confirmationDeadline; @@ -1217,8 +1216,6 @@ export const useAgentEvalsStore = defineStore(STORES.AGENT_EVALS, () => { }, RUN_POLL_INTERVAL_MS); }; const stop = (exhausted = false) => { - if (stopped) return; - stopped = true; clearTimeout(timer); if (!ownsProgress()) return; const current = findCachedResult(result.id)?.result; diff --git a/packages/frontend/editor-ui/src/features/agents/components/AgentEvalTryRow.vue b/packages/frontend/editor-ui/src/features/agents/components/AgentEvalTryRow.vue index 8de4c5794ec..b975f084e11 100644 --- a/packages/frontend/editor-ui/src/features/agents/components/AgentEvalTryRow.vue +++ b/packages/frontend/editor-ui/src/features/agents/components/AgentEvalTryRow.vue @@ -240,7 +240,7 @@ watch( type="button" :class="$style.expandToggle" :data-test-id="testId && `${testId}-toggle`" - :aria-expanded="view === 'complete' ? expanded : undefined" + :aria-expanded="expanded" :aria-label="i18n.baseText('instanceAi.testAgentPreview.conversation')" > diff --git a/packages/frontend/editor-ui/src/features/agents/utils/__tests__/agent-eval-tool-calls.test.ts b/packages/frontend/editor-ui/src/features/agents/utils/__tests__/agent-eval-tool-calls.test.ts index 256a12bd671..3592314bdd0 100644 --- a/packages/frontend/editor-ui/src/features/agents/utils/__tests__/agent-eval-tool-calls.test.ts +++ b/packages/frontend/editor-ui/src/features/agents/utils/__tests__/agent-eval-tool-calls.test.ts @@ -130,3 +130,9 @@ it('filters malformed intercepted requests without discarding the tool call', () }); expect(call).toMatchObject({ mocked: false, interceptedRequests: [] }); }); + +it('renders a recorded error status without requiring a diagnostic string', () => { + const [call] = toDisplayToolCalls({ calls: [{ tool: 'lookup', status: 'error' }] }); + expect(call.state).toBe(TOOL_CALL_STATE.ERROR); + expect(call.output).toBeUndefined(); +}); From 874cb237132c29481ae409353c1a0546342357b8 Mon Sep 17 00:00:00 2001 From: Luka Zivkovic Date: Fri, 16 Oct 2026 13:15:33 +0200 Subject: [PATCH 17/18] test(agent-evals): cover active run settlement and suggestion guards --- .../agent-eval-result.repository.test.ts | 35 ++++++++++ .../agent-eval-runner.service.test.ts | 67 +++++++++++++++++++ .../__tests__/AgentEvalChecksPanel.test.ts | 4 ++ .../components/AgentEvalChecksPanel.vue | 2 +- .../InstanceAiTestAgentExamplesPanel.test.ts | 14 ++++ 5 files changed, 121 insertions(+), 1 deletion(-) diff --git a/packages/@n8n/db/src/repositories/__tests__/agent-eval-result.repository.test.ts b/packages/@n8n/db/src/repositories/__tests__/agent-eval-result.repository.test.ts index aef6928625a..35203649302 100644 --- a/packages/@n8n/db/src/repositories/__tests__/agent-eval-result.repository.test.ts +++ b/packages/@n8n/db/src/repositories/__tests__/agent-eval-result.repository.test.ts @@ -219,6 +219,41 @@ describe('AgentEvalResultRepository', () => { await expect(repo.claimForRerun('res-1', 'run-1')).resolves.toBe(false); }); + + it('does not claim a rerun when the driver omits the affected row count', async () => { + entityManager.update.mockResolvedValue({ generatedMaps: [], raw: [] }); + + await expect(repo.claimForRerun('res-1', 'run-1')).resolves.toBe(false); + }); + }); + + describe('markUnfinishedAsError', () => { + it.each([undefined, new Date('2026-10-15T00:00:00Z')])( + 'repairs only an unstarted row or the original attempt at %s', + async (runAt) => { + const qb = { + update: vi.fn().mockReturnThis(), + set: vi.fn().mockReturnThis(), + where: vi.fn().mockReturnThis(), + execute: vi.fn().mockResolvedValue({ affected: 1 }), + }; + (entityManager.createQueryBuilder as Mock).mockReturnValue(qb); + + await repo.markUnfinishedAsError('res-1', runAt, 'Original write failed'); + + expect(qb.set).toHaveBeenCalledWith({ + status: 'error', + completedAt: expect.any(Date), + errorCode: 'case_dispatch_failed', + errorDetails: { message: 'Original write failed' }, + }); + type UnfinishedRow = { id: string; status: string; runAt?: Date }; + const expectedRows: UnfinishedRow[] = [{ id: 'res-1', status: 'new' }]; + if (runAt) expectedRows.push({ id: 'res-1', status: 'running', runAt }); + expect(qb.where).toHaveBeenCalledWith(expectedRows); + expect(qb.execute).toHaveBeenCalledOnce(); + }, + ); }); describe('markAsCancelled', () => { diff --git a/packages/cli/src/modules/agent-evals/__tests__/agent-eval-runner.service.test.ts b/packages/cli/src/modules/agent-evals/__tests__/agent-eval-runner.service.test.ts index 92a32bc0773..df3710e417a 100644 --- a/packages/cli/src/modules/agent-evals/__tests__/agent-eval-runner.service.test.ts +++ b/packages/cli/src/modules/agent-evals/__tests__/agent-eval-runner.service.test.ts @@ -517,6 +517,73 @@ describe('AgentEvalRunnerService', () => { } }); + it('preserves cancellation when an active rerun exceeds the settlement deadline', async () => { + const now = vi.spyOn(Date, 'now').mockReturnValue(1_000); + try { + seedFor([{ id: 'row-1', question: 'First' }], { running: 1 }); + evalAgentExecutionService.executeWithLlmMock.mockResolvedValue(successExec() as never); + const { finished } = await service.startRun('ds-1', 'proj-1', user); + await vi.waitFor(() => expect(resultRepository.countByStatus).toHaveBeenCalled()); + runRepository.isCancellationRequested.mockResolvedValue(true); + now.mockReturnValue(1_000 + 65 * 60_000); + await finished; + expect(runRepository.markAsCancelled).toHaveBeenCalledWith( + 'run-1', + expect.objectContaining({ pending: 1 }), + ); + expect(runRepository.markAsError).not.toHaveBeenCalled(); + expect(runRepository.markAsCompleted).not.toHaveBeenCalled(); + } finally { + now.mockRestore(); + } + }); + + it.each(['cancellation poll', 'original write'])( + 'reports a non-Error rejection from the %s', + async (failure) => { + seedFor([{ id: 'row-1', question: failure === 'original write' ? '' : 'First' }], { + error: 1, + }); + evalAgentExecutionService.executeWithLlmMock.mockResolvedValue(successExec() as never); + const reason = `${failure} rejected`; + if (failure === 'cancellation poll') { + runRepository.isCancellationRequested + .mockResolvedValueOnce(false) + .mockResolvedValueOnce(false) + .mockRejectedValueOnce(reason); + } else { + resultRepository.markAsError.mockRejectedValueOnce(reason); + } + const { finished } = await service.startRun('ds-1', 'proj-1', user); + await finished; + expect(runRepository.markAsError).toHaveBeenCalledWith( + 'run-1', + 'case_dispatch_failed', + expect.objectContaining({ errors: [reason] }), + expect.objectContaining({ error: 1, pending: 0 }), + counts({ error: 1 }), + ); + expect(runRepository.markAsCompleted).not.toHaveBeenCalled(); + }, + ); + + it.each(['removed', 'completed'] as const)( + 'stops settlement when the run was %s by another worker', + async (state) => { + seedFor([{ id: 'row-1', question: 'First' }], { success: 1 }); + evalAgentExecutionService.executeWithLlmMock.mockResolvedValue(successExec() as never); + runRepository.markAsCompleted.mockResolvedValue(false); + runRepository.findById.mockResolvedValue( + state === 'removed' ? null : mock({ id: 'run-1', status: 'completed' }), + ); + const { finished } = await service.startRun('ds-1', 'proj-1', user); + await finished; + expect(runRepository.markAsCompleted).toHaveBeenCalledOnce(); + expect(runRepository.findById).toHaveBeenCalledWith('run-1'); + expect(runRepository.markAsError).not.toHaveBeenCalled(); + }, + ); + it('runs each case with its input verbatim, persists results, aggregates the run', async () => { seedFor( [ diff --git a/packages/frontend/editor-ui/src/features/agents/__tests__/AgentEvalChecksPanel.test.ts b/packages/frontend/editor-ui/src/features/agents/__tests__/AgentEvalChecksPanel.test.ts index 43e9addf7cf..e2de1d530ac 100644 --- a/packages/frontend/editor-ui/src/features/agents/__tests__/AgentEvalChecksPanel.test.ts +++ b/packages/frontend/editor-ui/src/features/agents/__tests__/AgentEvalChecksPanel.test.ts @@ -810,6 +810,10 @@ describe('AgentEvalChecksPanel', () => { // A click that still reaches the composable is ignored, not sent as a second write. await user.click(within(getByTestId('agent-eval-check-c2')).getByText('apply suggestion')); expect(store.applySuggestions).toHaveBeenCalledTimes(1); + // A queued button event must also respect the active write. + getByTestId(BUTTON).dispatchEvent(new MouseEvent('click', { bubbles: true })); + await nextTick(); + expect(store.applySuggestions).toHaveBeenCalledTimes(1); finish({ configHash: 'h', results: [] }); }); diff --git a/packages/frontend/editor-ui/src/features/agents/components/AgentEvalChecksPanel.vue b/packages/frontend/editor-ui/src/features/agents/components/AgentEvalChecksPanel.vue index bced5dab63d..c611418c8ff 100644 --- a/packages/frontend/editor-ui/src/features/agents/components/AgentEvalChecksPanel.vue +++ b/packages/frontend/editor-ui/src/features/agents/components/AgentEvalChecksPanel.vue @@ -327,7 +327,7 @@ watch( }, ); -function onCheckAdded(deferRun = false) { +function onCheckAdded(deferRun: boolean) { if (deferRun || inFlight.value || props.rerunning) { savedForNextRunCount.value++; return; diff --git a/packages/frontend/editor-ui/src/features/ai/instanceAi/__tests__/InstanceAiTestAgentExamplesPanel.test.ts b/packages/frontend/editor-ui/src/features/ai/instanceAi/__tests__/InstanceAiTestAgentExamplesPanel.test.ts index ed2fda88012..9b26008a447 100644 --- a/packages/frontend/editor-ui/src/features/ai/instanceAi/__tests__/InstanceAiTestAgentExamplesPanel.test.ts +++ b/packages/frontend/editor-ui/src/features/ai/instanceAi/__tests__/InstanceAiTestAgentExamplesPanel.test.ts @@ -540,6 +540,20 @@ describe('InstanceAiTestAgentExamplesPanel', () => { expect(getByTestId(BUTTON)).toBeDisabled(); }); + it('ignores a queued apply-all click after a suggestion starts applying', async () => { + const { getByTestId, emitted, rerender } = renderComponent({ + props: { caseRuns: [failed(1), failed(2)] }, + }); + const button = getByTestId(BUTTON); + expect(button).toBeEnabled(); + await rerender({ applyingSuggestionIds: ['result-1'] }); + + button.dispatchEvent(new MouseEvent('click', { bubbles: true })); + + expect(emitted('apply-suggestions')).toBeUndefined(); + expect(button).toBeDisabled(); + }); + it('applies settled suggestions and offers the checks view while another check is running', async () => { const user = userEvent.setup(); const { getByTestId, emitted } = renderComponent({ From 3d7164d7d273bf1d18a58aa03377581b571ce62d Mon Sep 17 00:00:00 2001 From: Luka Zivkovic Date: Fri, 16 Oct 2026 19:38:54 +0200 Subject: [PATCH 18/18] refactor(core): Isolate agent eval run settlement --- packages/@n8n/api-types/src/index.ts | 1 + .../src/schemas/instance-ai.schema.ts | 4 +- .../agent-evals/agent-eval-runner.service.ts | 239 ++++++++++-------- 3 files changed, 143 insertions(+), 101 deletions(-) diff --git a/packages/@n8n/api-types/src/index.ts b/packages/@n8n/api-types/src/index.ts index c30c3a6e3aa..0b745d664e1 100644 --- a/packages/@n8n/api-types/src/index.ts +++ b/packages/@n8n/api-types/src/index.ts @@ -502,6 +502,7 @@ export { InstanceAiSendMessageRequest, InstanceAiEvalExecutionRequest, InstanceAiEvalAgentExecutionRequest, + INSTANCE_AI_EVAL_AGENT_MAX_TIMEOUT_MS, InstanceAiEvalCredentialAllowlistRequest, InstanceAiEvalCredentialTestTargetRequest, INSTANCE_AI_MEMORY_TASK_WAIT_TIMEOUT_MS, diff --git a/packages/@n8n/api-types/src/schemas/instance-ai.schema.ts b/packages/@n8n/api-types/src/schemas/instance-ai.schema.ts index 6a5441040c3..622fad95057 100644 --- a/packages/@n8n/api-types/src/schemas/instance-ai.schema.ts +++ b/packages/@n8n/api-types/src/schemas/instance-ai.schema.ts @@ -3013,6 +3013,8 @@ export interface InstanceAiEvalAgentExecutionResult { llmUsage?: InstanceAiEvalLlmUsage[]; } +export const INSTANCE_AI_EVAL_AGENT_MAX_TIMEOUT_MS = 3_600_000; + export class InstanceAiEvalAgentExecutionRequest extends Z.class({ /** Project the agent lives in (agent routes are project-scoped). */ projectId: z.string().min(1), @@ -3021,7 +3023,7 @@ export class InstanceAiEvalAgentExecutionRequest extends Z.class({ * Overall run budget. Server default applies when omitted. Shares the workflow * variant's ceiling — the old 900_000 cap truncated a `complex` case's budget. */ - timeoutMs: z.number().int().min(30_000).max(3_600_000).optional(), + timeoutMs: z.number().int().min(30_000).max(INSTANCE_AI_EVAL_AGENT_MAX_TIMEOUT_MS).optional(), }) {} export class InstanceAiEvalCredentialAllowlistRequest extends Z.class({ diff --git a/packages/cli/src/modules/agent-evals/agent-eval-runner.service.ts b/packages/cli/src/modules/agent-evals/agent-eval-runner.service.ts index 28becdc504b..92576e33824 100644 --- a/packages/cli/src/modules/agent-evals/agent-eval-runner.service.ts +++ b/packages/cli/src/modules/agent-evals/agent-eval-runner.service.ts @@ -1,5 +1,9 @@ import { setTimeout as sleep } from 'node:timers/promises'; -import type { AgentEvalVerdict, AgentEvalRunSummary } from '@n8n/api-types'; +import { + INSTANCE_AI_EVAL_AGENT_MAX_TIMEOUT_MS, + type AgentEvalVerdict, + type AgentEvalRunSummary, +} from '@n8n/api-types'; import { Logger, ModuleRegistry } from '@n8n/backend-common'; import { GlobalConfig } from '@n8n/config'; import type { AgentEvalDataset, AgentEvalResult, User } from '@n8n/db'; @@ -48,8 +52,8 @@ 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; -// Reruns allow a one-hour execution budget. Leave five minutes for judging. -const RESULT_SETTLEMENT_TIMEOUT_MS = 65 * 60_000; +// Cover the maximum agent execution budget, plus five minutes for judging. +const RESULT_SETTLEMENT_TIMEOUT_MS = INSTANCE_AI_EVAL_AGENT_MAX_TIMEOUT_MS + 5 * 60_000; const RESULT_SETTLEMENT_POLL_MS = 1_000; /** A dataset row resolved into a runnable case via the dataset's column mapping. */ @@ -67,6 +71,12 @@ interface CaseUsage { outputTokens: number; } +interface RunDeadline { + hasExpired: () => boolean; + clear: () => void; + deadlineMinutes: number; +} + /** * Executes an agent-eval dataset against a real agent and persists per-case * results, then aggregates run-level status. @@ -433,7 +443,6 @@ export class AgentEvalRunnerService { dispatchFailures.push(message); unfinishedOriginals.set(seeded[index].id, message); } - let wasCancelled = cancelObserved; if (dispatchFailures.length > 0) { this.logger.error( @@ -442,112 +451,142 @@ export class AgentEvalRunnerService { ); } - // A settled sibling can be rerun while the original pool is active. - // Fence the terminal write against those attempts, including other mains. - const settlementDeadline = Date.now() + RESULT_SETTLEMENT_TIMEOUT_MS; - while (true) { - for (const [resultId, message] of unfinishedOriginals) { - try { - await this.resultRepository.markUnfinishedAsError( - resultId, - originalAttempts.get(resultId), - message, - ); - unfinishedOriginals.delete(resultId); - } catch (error) { - this.logger.warn(`[AgentEvalRunner] Could not settle original case ${resultId}`, { - error, - }); - } - } - if (!wasCancelled) { - try { - wasCancelled = await this.runRepository.isCancellationRequested(runId); - } catch (error) { - dispatchFailures.push(error instanceof Error ? error.message : String(error)); - } - } - const counts = await this.resultRepository.countByStatus(runId); - const metrics: IDataObject = { ...toSummaryCounts(counts), usage: { ...totalUsage } }; - if (Date.now() >= settlementDeadline) { - if (wasCancelled) { - await this.runRepository.markAsCancelled(runId, metrics); - } else { - await this.runRepository.markAsError( - runId, - 'result_settlement_timeout', - { - message: 'A result did not settle after the original batch finished.', - }, - metrics, - ); - } - break; - } - if (counts.new > 0 || counts.running > 0) { - await sleep(RESULT_SETTLEMENT_POLL_MS); - continue; - } - // Dispatch failures use the same claim fence as successful completion. - const finalCounts = counts; - - let settled; - // 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. - settled = await this.runRepository.markAsCancelled(runId, metrics, finalCounts); - } else if (deadline.hasExpired() && stoppedCases > 0) { - settled = await this.runRepository.markAsError( - runId, - 'timeout', - { - message: `Run exceeded its ${deadline.deadlineMinutes}-minute deadline; ${stoppedCases} case(s) were not started.`, - }, - metrics, - finalCounts, - ); - } else if (dispatchFailures.length > 0) { - settled = await this.runRepository.markAsError( - runId, - 'case_dispatch_failed', - { - message: `${dispatchFailures.length} failure(s) occurred outside case execution.`, - errors: dispatchFailures, - }, - metrics, - finalCounts, - ); - } 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`, - ); - } - settled = await this.runRepository.markAsCompleted(runId, metrics, finalCounts); - } - if (settled) break; - const current = await this.runRepository.findById(runId); - if (!current || current.status !== 'running') break; - await sleep(RESULT_SETTLEMENT_POLL_MS); - } + await this.settleRun({ + runId, + unfinishedOriginals, + originalAttempts, + dispatchFailures, + cancelObserved, + totalUsage, + deadline, + stoppedCases, + }); } catch (error) { const message = error instanceof Error ? error.message : String(error); await this.failRun(runId, message); } } + private async settleRun(ctx: { + runId: string; + unfinishedOriginals: Map; + originalAttempts: Map; + dispatchFailures: string[]; + cancelObserved: boolean; + totalUsage: CaseUsage; + deadline: RunDeadline; + stoppedCases: number; + }): Promise { + const { + runId, + unfinishedOriginals, + originalAttempts, + dispatchFailures, + cancelObserved, + totalUsage, + deadline, + stoppedCases, + } = ctx; + let wasCancelled = cancelObserved; + + // A settled sibling can be rerun while the original pool is active. + // Fence the terminal write against those attempts, including other mains. + const settlementDeadline = Date.now() + RESULT_SETTLEMENT_TIMEOUT_MS; + while (true) { + for (const [resultId, message] of unfinishedOriginals) { + try { + await this.resultRepository.markUnfinishedAsError( + resultId, + originalAttempts.get(resultId), + message, + ); + unfinishedOriginals.delete(resultId); + } catch (error) { + this.logger.warn(`[AgentEvalRunner] Could not settle original case ${resultId}`, { + error, + }); + } + } + if (!wasCancelled) { + try { + wasCancelled = await this.runRepository.isCancellationRequested(runId); + } catch (error) { + dispatchFailures.push(error instanceof Error ? error.message : String(error)); + } + } + const counts = await this.resultRepository.countByStatus(runId); + const metrics: IDataObject = { ...toSummaryCounts(counts), usage: { ...totalUsage } }; + if (Date.now() >= settlementDeadline) { + if (wasCancelled) { + await this.runRepository.markAsCancelled(runId, metrics); + } else { + await this.runRepository.markAsError( + runId, + 'result_settlement_timeout', + { + message: 'A result did not settle after the original batch finished.', + }, + metrics, + ); + } + break; + } + if (counts.new > 0 || counts.running > 0) { + await sleep(RESULT_SETTLEMENT_POLL_MS); + continue; + } + // Dispatch failures use the same claim fence as successful completion. + const finalCounts = counts; + + let settled; + // 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. + settled = await this.runRepository.markAsCancelled(runId, metrics, finalCounts); + } else if (deadline.hasExpired() && stoppedCases > 0) { + settled = await this.runRepository.markAsError( + runId, + 'timeout', + { + message: `Run exceeded its ${deadline.deadlineMinutes}-minute deadline; ${stoppedCases} case(s) were not started.`, + }, + metrics, + finalCounts, + ); + } else if (dispatchFailures.length > 0) { + settled = await this.runRepository.markAsError( + runId, + 'case_dispatch_failed', + { + message: `${dispatchFailures.length} failure(s) occurred outside case execution.`, + errors: dispatchFailures, + }, + metrics, + finalCounts, + ); + } 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`, + ); + } + settled = await this.runRepository.markAsCompleted(runId, metrics, finalCounts); + } + if (settled) break; + const current = await this.runRepository.findById(runId); + if (current?.status !== 'running') break; + await sleep(RESULT_SETTLEMENT_POLL_MS); + } + } + /** * 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; - } { + private startRunDeadline(abort: AbortController): RunDeadline { const deadlineMinutes = this.globalConfig.evaluation.agentEvalsRunTimeoutMinutes; if (deadlineMinutes <= 0) { return { hasExpired: () => false, clear: () => undefined, deadlineMinutes };