From 146d7804af2fe347325a40becea74fae33a9b2da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Braulio=20Gonz=C3=A1lez=20Valido?= Date: Tue, 13 Oct 2026 21:29:00 +0000 Subject: [PATCH] fix(ai-builder): Count eval runs that read another build's workflow as framework issues (no-changelog) (#40717) Co-authored-by: Claude Opus 5.5 (1M context) --- .../@n8n/instance-ai/evaluations/README.md | 2 + .../__tests__/build-orchestrator.test.ts | 39 ++++++++ .../build-workflow-build-project.test.ts | 23 +++++ .../__tests__/case-pipeline.test.ts | 46 +++++++++ .../__tests__/foreign-workflow-reads.test.ts | 99 +++++++++++++++++++ .../evaluations/harness/build-workflow.ts | 37 +++++++ .../evaluations/outcome/event-parser.ts | 3 +- .../evaluations/outcome/foreign-reads.ts | 49 +++++++++ .../evaluations/outcome/workflow-discovery.ts | 7 +- .../evaluations/run/build-orchestrator.ts | 12 ++- .../evaluations/run/case-pipeline.ts | 19 ++-- 11 files changed, 317 insertions(+), 19 deletions(-) create mode 100644 packages/@n8n/instance-ai/evaluations/__tests__/foreign-workflow-reads.test.ts create mode 100644 packages/@n8n/instance-ai/evaluations/outcome/foreign-reads.ts diff --git a/packages/@n8n/instance-ai/evaluations/README.md b/packages/@n8n/instance-ai/evaluations/README.md index cc7d522fba8..c1cffe26de5 100644 --- a/packages/@n8n/instance-ai/evaluations/README.md +++ b/packages/@n8n/instance-ai/evaluations/README.md @@ -877,6 +877,8 @@ A direction governs only what it covers; otherwise the proxy answers every quest Each orchestrator build runs in its own empty project: the personal project of a freshly invited member, named like the lane owner. The owner still drives the build; the member only owns the project. An unlicensed instance cannot create team projects, so a new user is the only way to get a new project. Cleanup deletes the member, which also deletes its project and anything a timed-out build left there; under `--keep-workflows` the members stay, with their projects. Like the MCP lane, this needs a lane without SMTP, so invites return their accept token. +If the agent still reads a workflow that its build did not create or seed, the run fails as a harness fault: its rows and expectations fail with `framework_issue`. The pass rate shows the leak, builder analyses can filter it out by attribution, and the log names the workflow. A timeout still wins: if the build also timed out, the run stays neutral as `timeout`, like every timed-out run. + ### Credentials By default a build sees **no credentials**: the harness pins every build thread's credential view to the case's declared set (empty unless declared), so concurrent cases — and whatever happens to live on the instance — can never leak into a build. Every node mocks during verification. diff --git a/packages/@n8n/instance-ai/evaluations/__tests__/build-orchestrator.test.ts b/packages/@n8n/instance-ai/evaluations/__tests__/build-orchestrator.test.ts index e2d05b32235..fe8a8904949 100644 --- a/packages/@n8n/instance-ai/evaluations/__tests__/build-orchestrator.test.ts +++ b/packages/@n8n/instance-ai/evaluations/__tests__/build-orchestrator.test.ts @@ -644,6 +644,45 @@ describe('createBuildOrchestrator', () => { expect(verdicts?.[0].attribution).toBe('framework_issue'); }); + it("fails, without judging, the expectations of a build that read another build's workflow", async () => { + vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ ok: true })); + const building = vi.fn().mockResolvedValue(okBuild({ foreignWorkflowReads: ['other-wf'] })); + const deps = makeDeps([makeLane(1, building)], { + testCaseByFileSlug: new Map([ + ['case-a', baseCase({ processExpectations: ['builds the digest'] })], + ]), + }); + const orchestrator = createBuildOrchestrator(deps); + + await orchestrator.getOrBuild(0, 'case-a'); + + expect(vi.mocked(verifyBuildExpectations)).not.toHaveBeenCalled(); + const verdicts = await deps.buildExpectationsByKey.get('0:case-a'); + expect(verdicts?.[0]).toMatchObject({ pass: false, attribution: 'framework_issue' }); + // Counted, like the row: only `incomplete` keeps a verdict out of the pass rate. + expect(verdicts?.[0].incomplete).toBeFalsy(); + expect(verdicts?.[0].reason).toContain('another build'); + }); + + it("keeps a timed-out build that read another build's workflow neutral", async () => { + vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ ok: true })); + const timeout = { kind: 'turn' as const, turn: 1, elapsedMs: 900_001 }; + const building = vi + .fn() + .mockResolvedValue(okBuild({ foreignWorkflowReads: ['other-wf'], timeout })); + const deps = makeDeps([makeLane(1, building)], { + testCaseByFileSlug: new Map([ + ['case-a', baseCase({ processExpectations: ['builds the digest'] })], + ]), + }); + const orchestrator = createBuildOrchestrator(deps); + + await orchestrator.getOrBuild(0, 'case-a'); + + const verdicts = await deps.buildExpectationsByKey.get('0:case-a'); + expect(verdicts?.[0]).toMatchObject({ incomplete: true, attribution: 'timeout' }); + }); + it('serves prebuilt workflows by fetching them, never invoking the builder', async () => { const tracedBuild = vi.fn().mockResolvedValue(okBuild()); const lane = makeLane(1, tracedBuild); diff --git a/packages/@n8n/instance-ai/evaluations/__tests__/build-workflow-build-project.test.ts b/packages/@n8n/instance-ai/evaluations/__tests__/build-workflow-build-project.test.ts index bc6915a6009..edadf4bbf09 100644 --- a/packages/@n8n/instance-ai/evaluations/__tests__/build-workflow-build-project.test.ts +++ b/packages/@n8n/instance-ai/evaluations/__tests__/build-workflow-build-project.test.ts @@ -126,4 +126,27 @@ describe('buildWorkflow build project', () => { { type: 'agent', id: 'agent-1', projectId: 'build-project' }, ]); }); + + it("reports the agent's reads of another build's workflow, even when the build fails", async () => { + const actual = + await vi.importActual('../outcome/event-parser'); + vi.mocked(extractOutcomeFromEvents).mockImplementationOnce((events) => ({ + ...actual.extractOutcomeFromEvents(events), + toolCalls: [ + { + toolCallId: 'call-1', + toolName: 'workflows', + args: { action: 'get', workflowId: 'other-wf' }, + result: { id: 'other-wf' }, + durationMs: 0, + }, + ], + })); + const { client } = makeClient(); + + const result = await buildWorkflow({ client, ...baseConfig }); + + expect(result.success).toBe(false); + expect(result.foreignWorkflowReads).toEqual(['other-wf']); + }); }); diff --git a/packages/@n8n/instance-ai/evaluations/__tests__/case-pipeline.test.ts b/packages/@n8n/instance-ai/evaluations/__tests__/case-pipeline.test.ts index 44664ecf613..a93b1063fa6 100644 --- a/packages/@n8n/instance-ai/evaluations/__tests__/case-pipeline.test.ts +++ b/packages/@n8n/instance-ai/evaluations/__tests__/case-pipeline.test.ts @@ -232,6 +232,52 @@ describe('createCasePipeline', () => { expect(vi.mocked(lane.tracedExecute)).not.toHaveBeenCalled(); }); + // Unlike a missing premise, a run that did not start clean still counts, so a leak + // shows in the pass rate. The attribution keeps it out of the builder's numbers. + it("fails a build that read another build's workflow as framework_issue, and counts it", async () => { + const lane = makeLane(); + const cached: CachedBuild = { + build: { ...okBuild(), foreignWorkflowReads: ['other-wf'] }, + lane, + buildDurationMs: 42, + }; + const pipeline = createCasePipeline(makeDeps(makeOrchestrator(cached))); + + const output = await pipeline.runRow(rowInputs('happy-path')); + + expect(output).toMatchObject({ + passed: false, + failureCategory: 'framework_issue', + attribution: 'framework_issue', + }); + expect(output.incomplete).toBeFalsy(); + expect(String(output.reasoning)).toContain('another build'); + expect(vi.mocked(lane.tracedExecute)).not.toHaveBeenCalled(); + }); + + // A timeout outranks the fault, as on every other timed-out build. + it("keeps a timed-out build that read another build's workflow out of the pass rate", async () => { + const lane = makeLane(); + const timeout = { kind: 'turn' as const, turn: 2, elapsedMs: 900_400 }; + const cached: CachedBuild = { + build: okBuild({ timeout, foreignWorkflowReads: ['other-wf'] }), + lane, + buildDurationMs: 42, + }; + const pipeline = createCasePipeline(makeDeps(makeOrchestrator(cached))); + + const output = await pipeline.runRow(rowInputs('happy-path')); + + expect(output).toMatchObject({ + passed: false, + incomplete: true, + attribution: 'timeout', + failureCategory: 'build_timeout', + buildTimeout: timeout, + }); + expect(vi.mocked(lane.tracedExecute)).not.toHaveBeenCalled(); + }); + it('preserves agent metadata when a prior run failed', async () => { const lane = makeLane(); const build = okBuild({ diff --git a/packages/@n8n/instance-ai/evaluations/__tests__/foreign-workflow-reads.test.ts b/packages/@n8n/instance-ai/evaluations/__tests__/foreign-workflow-reads.test.ts new file mode 100644 index 00000000000..fa90bcfd6ec --- /dev/null +++ b/packages/@n8n/instance-ai/evaluations/__tests__/foreign-workflow-reads.test.ts @@ -0,0 +1,99 @@ +import { findForeignWorkflowReads } from '../outcome/foreign-reads'; +import type { CapturedToolCall } from '../types'; + +function call( + toolName: string, + args: Record, + outcome: { result?: unknown; error?: string } = { result: { success: true } }, +): CapturedToolCall { + return { + toolCallId: `${toolName}-${String(Math.random())}`, + toolName, + args, + durationMs: 0, + ...outcome, + }; +} + +const created = call( + 'build-workflow', + { filePath: 'src/workflows/digest.workflow.ts', name: 'Daily Email Digest' }, + { result: { success: true, workflowId: 'mine' } }, +); + +describe('findForeignWorkflowReads', () => { + it('flags a successful read of a workflow this build did not create or seed', () => { + const reads = findForeignWorkflowReads( + [call('workflows', { action: 'get', workflowId: 'other' }, { result: { id: 'other' } })], + [], + ); + + expect(reads).toEqual(['other']); + }); + + it('ignores the workflows the build created, and later reads of them', () => { + const reads = findForeignWorkflowReads( + [created, call('workflows', { action: 'get', workflowId: 'mine' })], + [], + ); + + expect(reads).toEqual([]); + }); + + it('ignores seeded workflows', () => { + const reads = findForeignWorkflowReads( + [call('workflows', { action: 'get', workflowId: 'seeded' })], + ['seeded'], + ); + + expect(reads).toEqual([]); + }); + + it('ignores reads that failed, so an invented id is not a leak', () => { + const reads = findForeignWorkflowReads( + [ + call('workflows', { action: 'get', workflowId: 'ghost' }, { error: 'Workflow not found' }), + call( + 'workflows', + { action: 'setup', workflowId: 'ghost-2' }, + { result: { success: false, error: 'not found' } }, + ), + ], + [], + ); + + expect(reads).toEqual([]); + }); + + it("flags another build's workflows in a list result", () => { + const reads = findForeignWorkflowReads( + [ + created, + call( + 'workflows', + { action: 'list', scope: 'instance' }, + { result: JSON.stringify({ workflows: [{ id: 'mine' }, { id: 'other' }] }) }, + ), + ], + [], + ); + + expect(reads).toEqual(['other']); + }); + + it("flags an edit of another build's workflow, once", () => { + const reads = findForeignWorkflowReads( + [ + call('workflows', { action: 'get', workflowId: 'other' }), + call( + 'build-workflow', + { filePath: 'src/workflows/digest.workflow.ts', workflowId: 'other' }, + { result: { success: true, workflowId: 'other' } }, + ), + ], + [], + ); + + expect(reads).toEqual(['other']); + }); +}); diff --git a/packages/@n8n/instance-ai/evaluations/harness/build-workflow.ts b/packages/@n8n/instance-ai/evaluations/harness/build-workflow.ts index 3aef627aa29..8f29b3d6170 100644 --- a/packages/@n8n/instance-ai/evaluations/harness/build-workflow.ts +++ b/packages/@n8n/instance-ai/evaluations/harness/build-workflow.ts @@ -71,6 +71,7 @@ import { extractOutcomeFromEvents, mergeSeededConversationMetrics, } from '../outcome/event-parser'; +import { findForeignWorkflowReads } from '../outcome/foreign-reads'; import { buildTranscriptFromEvents } from '../outcome/transcript-from-events'; import { buildAgentOutcome, extractWorkflowIdsFromMessages } from '../outcome/workflow-discovery'; import type { @@ -78,6 +79,7 @@ import type { BuildTimeout, BuildTrace, CapturedEvent, + CapturedToolCall, ConversationMetrics, ConversationTurn, ExecutionScenario, @@ -333,6 +335,8 @@ export interface BuildResult { * one applies even when the BUILD SUCCEEDED, which is exactly the case that would * otherwise be scored as an agent failure. */ priorRunFailed?: string; + /** Workflows the agent read that this build neither created nor seeded. Not scored. */ + foreignWorkflowReads?: string[]; /** Evidence that the MODEL PROVIDER, not the builder, failed this build (a * 5xx/429 upstream of the n8n instance). Set only after the retry budget is * spent. Routed to `framework_issue` with `PROVIDER_OUTAGE_ROOT_CAUSE`, so an @@ -344,6 +348,24 @@ export interface BuildResult { credentialSetup?: CredentialSetupRunFacts; } +/** A harness fault that decides the iteration without grading. `counted` faults fail it as + * `framework_issue`; the others keep it out of the pass rate. */ +export function harnessFault(build: BuildResult): { reason: string; counted: boolean } | undefined { + if (build.priorRunFailed) { + return { + reason: `prior run staging did not land, so the case premise is missing: ${build.priorRunFailed}`, + counted: false, + }; + } + if (build.foreignWorkflowReads?.length) { + return { + reason: `the agent read workflows another build made (${build.foreignWorkflowReads.join(', ')})`, + counted: true, + }; + } + return undefined; +} + /** * True when the build failed for a reason the agent doesn't own — seeding, * transport or the model provider. Everything downstream that has to attribute @@ -599,6 +621,16 @@ export async function buildWorkflow(config: BuildWorkflowConfig): Promise { + const toolCalls = parsedToolCalls ?? extractOutcomeFromEvents(events).toolCalls; + const foreignWorkflowReads = findForeignWorkflowReads(toolCalls, restoredWorkflowIds); + if (foreignWorkflowReads.length === 0) return {}; + logger.warn( + ` The agent read workflows another build made: ${foreignWorkflowReads.join(', ')}`, + ); + return { foreignWorkflowReads }; + }; /** The agent the seeded history last targeted — graded and executed first. */ let seedActiveAgentId: string | undefined; // Scenario seed tables are created empty before the build @@ -1263,6 +1295,7 @@ export async function buildWorkflow(config: BuildWorkflowConfig): Promise | undefined { + if (isRecord(result)) return result; + if (typeof result !== 'string') return undefined; + try { + const parsed: unknown = JSON.parse(result); + return isRecord(parsed) ? parsed : undefined; + } catch { + return undefined; + } +} + +function succeeded(call: CapturedToolCall): boolean { + const result = toRecord(call.result); + return call.error === undefined && result?.success !== false && typeof result?.error !== 'string'; +} + +/** Workflow ids the agent successfully read or changed that this build neither created + * nor seeded: another build's work, reachable through a shared view. */ +export function findForeignWorkflowReads( + toolCalls: CapturedToolCall[], + seededWorkflowIds: string[], +): string[] { + const calls = toolCalls.filter(succeeded); + const own = new Set(seededWorkflowIds); + for (const call of calls) { + // A save without a target workflow creates one. + if (!WORKFLOW_TOOLS.has(call.toolName) || call.args.workflowId !== undefined) continue; + const result = toRecord(call.result); + const id = result?.workflowId ?? result?.id; + if (typeof id === 'string') own.add(id); + } + + const seen = calls.flatMap((call) => { + const target = typeof call.args.workflowId === 'string' ? [call.args.workflowId] : []; + const listed = toRecord(call.result)?.workflows; + const listedIds = Array.isArray(listed) + ? listed.flatMap((workflow) => + isRecord(workflow) && typeof workflow.id === 'string' ? [workflow.id] : [], + ) + : []; + return [...target, ...listedIds]; + }); + return [...new Set(seen.filter((id) => !own.has(id)))]; +} diff --git a/packages/@n8n/instance-ai/evaluations/outcome/workflow-discovery.ts b/packages/@n8n/instance-ai/evaluations/outcome/workflow-discovery.ts index 6799ca02365..a5364376d58 100644 --- a/packages/@n8n/instance-ai/evaluations/outcome/workflow-discovery.ts +++ b/packages/@n8n/instance-ai/evaluations/outcome/workflow-discovery.ts @@ -7,12 +7,7 @@ import type { InstanceAiMessage } from '@n8n/api-types'; import { N8nApiError, type N8nClient, type WorkflowResponse } from '../clients/n8n-client'; import type { AgentOutcome, EventOutcome, ExecutionSummary, WorkflowSummary } from '../types'; import { collectArtifactRefIds } from './collect-refs'; - -// --------------------------------------------------------------------------- -// Tool names whose results contain workflow IDs -// --------------------------------------------------------------------------- - -const WORKFLOW_TOOLS = new Set(['build-workflow', 'submit-workflow', 'patch-workflow']); +import { WORKFLOW_TOOLS } from './event-parser'; // --------------------------------------------------------------------------- // snapshotWorkflowIds -- call before the run to know what existed prior diff --git a/packages/@n8n/instance-ai/evaluations/run/build-orchestrator.ts b/packages/@n8n/instance-ai/evaluations/run/build-orchestrator.ts index f626d57522b..2cd937463dd 100644 --- a/packages/@n8n/instance-ai/evaluations/run/build-orchestrator.ts +++ b/packages/@n8n/instance-ai/evaluations/run/build-orchestrator.ts @@ -38,6 +38,7 @@ import { resolveArtifactContext } from '../harness/artifacts/artifact-context'; import { attributionForExpectation } from '../harness/attribution'; import { buildFailedOnInfra, + harnessFault, leakHaystackFor, redactLocalRunSecrets, searchableBuildText, @@ -476,10 +477,11 @@ export function createBuildOrchestrator(deps: BuildOrchestratorDeps): BuildOrche // would still score on them. const timedOut = (verdicts: BuildExpectationResult[]): BuildExpectationResult[] => verdicts.map((v) => ({ ...v, incomplete: true, attribution: 'timeout' as const })); - if (build.priorRunFailed) { + const fault = harnessFault(build); + if (fault) { const unjudged = allFailVerdicts( collectExpectations(testCase), - `not judged — prior run staging did not land, so the case premise is missing: ${build.priorRunFailed}`, + `not judged — ${fault.reason}`, ); // The row for this build is re-stamped `timeout` too, so the two agree. buildExpectationsByKey.set( @@ -487,7 +489,11 @@ export function createBuildOrchestrator(deps: BuildOrchestratorDeps): BuildOrche Promise.resolve( build.timeout ? timedOut(unjudged) - : unjudged.map((verdict) => ({ ...verdict, attribution: 'framework_issue' as const })), + : unjudged.map((verdict) => ({ + ...verdict, + incomplete: !fault.counted, + attribution: 'framework_issue' as const, + })), ), ); return; diff --git a/packages/@n8n/instance-ai/evaluations/run/case-pipeline.ts b/packages/@n8n/instance-ai/evaluations/run/case-pipeline.ts index 5e28e8b4d5d..b70b23b1bc2 100644 --- a/packages/@n8n/instance-ai/evaluations/run/case-pipeline.ts +++ b/packages/@n8n/instance-ai/evaluations/run/case-pipeline.ts @@ -22,7 +22,7 @@ import { findAgentArtifactRef, type AgentScenarioContext, } from '../harness/agent-execution'; -import { buildFailedOnInfra } from '../harness/build-workflow'; +import { buildFailedOnInfra, harnessFault } from '../harness/build-workflow'; import { cleanupBuild, effectiveTimeoutMs } from '../harness/cleanup'; import type { EvalLogger } from '../harness/logger'; import { selectScenarioWorkflowId } from '../harness/scenario-execution'; @@ -257,8 +257,10 @@ export function createCasePipeline(deps: CasePipelineDeps): CasePipeline { // grading against history the instance does not have. Checked BEFORE every other // branch, and independently of `build.success`, because the biting case is a build // that succeeded: `buildFailedOnInfra` returns false there, so the case would - // otherwise be scored as an agent failure on a premise that never existed. - if (build.priorRunFailed) { + // otherwise be scored as an agent failure on a premise that never existed. A read of + // another build's workflow takes the same route: the run did not start clean. + const fault = harnessFault(build); + if (fault) { const capturedAgent = agentRef ? await agentContextByKey.get(cacheKey) : undefined; return await attachExpectations({ buildSuccess: build.success, @@ -266,15 +268,14 @@ export function createCasePipeline(deps: CasePipelineDeps): CasePipeline { ...(capturedAgent ? { agentContext: capturedAgent.rendered } : {}), ...(capturedAgent?.artifact ? { agentArtifact: capturedAgent.artifact } : {}), passed: false, - // Not graded rather than failed — the same treatment an ungraded expectation - // gets, so this stays out of the pass rate instead of landing in the - // builder's baseline as a red. - incomplete: true, + // A missing premise is not graded, so it stays out of the pass rate. A counted + // fault fails the run, but as framework_issue, not in the builder's baseline. + ...(fault.counted ? {} : { incomplete: true }), score: 0, - reasoning: `Prior run staging did not land, so the case premise is missing: ${build.priorRunFailed}`, + reasoning: `${fault.counted ? 'Harness fault' : 'Not scored'}: ${fault.reason}`, failureCategory: 'framework_issue', attribution: 'framework_issue', - execErrors: [build.priorRunFailed], + execErrors: [fault.reason], buildDurationMs, ...buildSpendFields, execDurationMs: 0,