diff --git a/packages/@n8n/instance-ai/evaluations/README.md b/packages/@n8n/instance-ai/evaluations/README.md index 1618b12d604..12ad2fde739 100644 --- a/packages/@n8n/instance-ai/evaluations/README.md +++ b/packages/@n8n/instance-ai/evaluations/README.md @@ -436,8 +436,19 @@ How it differs from the manifest flow: place, so every iteration gets a genuinely fresh build (clean `pass@k`/`pass^k` variance) instead of rotating a fixed list of prebuilt IDs. - **Multi-lane.** Unlike `--prebuilt-workflows` (single instance), `--build-via-mcp` - accepts a comma-separated `--base-url`. Each lane enables MCP, mints its own API - key, and stages its own `claude` MCP config — the CLI does this setup for you. + accepts a comma-separated `--base-url`. Each lane enables MCP; the CLI does this + setup for you. +- **Per-case build users + credential seeding.** Every build runs `claude` as its + own freshly-invited member user (invited in batches through the lane owner, + accepted lazily per build), with that user's MCP API key staged into a per-build + `claude` MCP config. MCP credential/workflow visibility is user-scoped, so each + build sees an isolated view: exactly the case's declared `credentials` (created + in the member's personal project — the MCP analog of the orchestrator's + per-thread credential pinning), and nothing from concurrent builds. Requires the + lanes to have no SMTP configured (otherwise invites are emailed and the CLI + can't obtain the accept token — eval instances never have SMTP). Build users are + deleted with the built workflows after the run; under `--keep-workflows` they + stay, since deleting a user deletes their remaining data. - **Throwaway cleanup.** Built workflows are deleted after the run unless you pass `--keep-workflows`. Known limitation: cleanup keys off the `WORKFLOW_ID` trailer `claude` prints, so a build that times out or never emits the trailer can leave diff --git a/packages/@n8n/instance-ai/evaluations/__tests__/fixtures.ts b/packages/@n8n/instance-ai/evaluations/__tests__/fixtures.ts index 223922fa0ec..9cd5d9209d1 100644 --- a/packages/@n8n/instance-ai/evaluations/__tests__/fixtures.ts +++ b/packages/@n8n/instance-ai/evaluations/__tests__/fixtures.ts @@ -1,10 +1,20 @@ import type { EvaluationConfigDto, InstanceAiAgentNode, InstanceAiMessage } from '@n8n/api-types'; import type { WorkflowResponse } from '../clients/n8n-client'; +import type { EvalLogger } from '../harness/logger'; import type { WorkflowTestCase } from '../types'; /** Shared fixture builders for artifact-handler and outcome tests — see individual test files for usage. */ +export const silentLogger: EvalLogger = { + info: () => {}, + verbose: () => {}, + success: () => {}, + warn: () => {}, + error: () => {}, + isVerbose: false, +}; + export function agentNode(overrides: Partial = {}): InstanceAiAgentNode { return { agentId: 'agent-1', diff --git a/packages/@n8n/instance-ai/evaluations/__tests__/lane-users.test.ts b/packages/@n8n/instance-ai/evaluations/__tests__/lane-users.test.ts new file mode 100644 index 00000000000..98dc4b448da --- /dev/null +++ b/packages/@n8n/instance-ai/evaluations/__tests__/lane-users.test.ts @@ -0,0 +1,186 @@ +import { silentLogger } from './fixtures'; +import type { N8nClient } from '../clients/n8n-client'; +import { cleanupLaneUsers, LaneUserPool, provisionCaseBuildUser } from '../run/lane-users'; +import type { TestCaseCredential } from '../types'; + +type InvitedUser = { id: string; email: string; acceptToken: string }; + +/** Owner-client stand-in that fulfills every invite and records the batches. */ +function fakeOwnerClient() { + const inviteBatches: string[][] = []; + let nextId = 0; + const inviteMembers = vi.fn(async (emails: string[]): Promise => { + inviteBatches.push(emails); + return await Promise.resolve( + emails.map((email) => { + nextId += 1; + return { id: `u${String(nextId)}`, email, acceptToken: `token-${email}` }; + }), + ); + }); + return { client: { inviteMembers } as unknown as N8nClient, inviteMembers, inviteBatches }; +} + +describe('LaneUserPool', () => { + it('invites a chunk on first claim and serves later claims from the pool', async () => { + const { client, inviteMembers } = fakeOwnerClient(); + const pool = new LaneUserPool(client, 4); + + const first = await pool.claim(); + const second = await pool.claim(); + + expect(inviteMembers).toHaveBeenCalledTimes(1); + expect(first.email).not.toBe(second.email); + expect(pool.createdUserIds).toHaveLength(4); + }); + + it('shares one in-flight refill across concurrent claims on a drained pool', async () => { + const { client, inviteMembers } = fakeOwnerClient(); + const pool = new LaneUserPool(client, 4); + + const users = await Promise.all([pool.claim(), pool.claim(), pool.claim(), pool.claim()]); + + expect(inviteMembers).toHaveBeenCalledTimes(1); + expect(new Set(users.map((u) => u.email)).size).toBe(4); + }); + + it('refills again when concurrent claims outnumber one chunk', async () => { + const { client, inviteMembers } = fakeOwnerClient(); + const pool = new LaneUserPool(client, 2); + + const users = await Promise.all(Array.from({ length: 5 }, async () => await pool.claim())); + + expect(new Set(users.map((u) => u.email)).size).toBe(5); + expect(inviteMembers.mock.calls.length).toBeGreaterThanOrEqual(3); + expect(pool.createdUserIds.length).toBe(inviteMembers.mock.calls.length * 2); + }); + + it('mints unique emails across refills', async () => { + const { client, inviteBatches } = fakeOwnerClient(); + const pool = new LaneUserPool(client, 2); + + for (let i = 0; i < 5; i++) await pool.claim(); + + const emails = inviteBatches.flat(); + expect(new Set(emails).size).toBe(emails.length); + }); + + it('records shells for cleanup and explains itself when a batch yields no accept token', async () => { + // Mirrors an SMTP-configured lane: the shells exist server-side, but the + // accept URL (and so the token) is withheld. + const inviteMembers = vi.fn().mockResolvedValue([ + { id: 'u1', email: 'a@b.invalid' }, + { id: 'u2', email: 'c@d.invalid' }, + ]); + const pool = new LaneUserPool({ inviteMembers } as unknown as N8nClient, 2); + + await expect(pool.claim()).rejects.toThrow('none returned an accept token'); + expect(pool.createdUserIds).toEqual(['u1', 'u2']); + }); + + it('serves the token-bearing invites when only some of a batch carry one', async () => { + const inviteMembers = vi.fn().mockResolvedValue([ + { id: 'u1', email: 'a@b.invalid', error: 'already invited' }, + { id: 'u2', email: 'c@d.invalid', acceptToken: 't2' }, + ]); + const pool = new LaneUserPool({ inviteMembers } as unknown as N8nClient, 2); + + await expect(pool.claim()).resolves.toMatchObject({ id: 'u2' }); + expect(pool.createdUserIds).toEqual(['u1', 'u2']); + }); + + it('rejects waiting claims when the invite fails, then recovers', async () => { + const inviteMembers = vi + .fn() + .mockRejectedValueOnce(new Error('invite exploded')) + .mockResolvedValue([{ id: 'u1', email: 'a@b.invalid', acceptToken: 't' }]); + const pool = new LaneUserPool({ inviteMembers } as unknown as N8nClient, 1); + + await expect(pool.claim()).rejects.toThrow('invite exploded'); + await expect(pool.claim()).resolves.toMatchObject({ id: 'u1' }); + }); +}); + +describe('provisionCaseBuildUser', () => { + function fakeMemberClient() { + const created: Array<{ name: string; type: string }> = []; + const member = { + acceptInvitation: vi.fn().mockResolvedValue(undefined), + rotateMcpApiKey: vi.fn().mockResolvedValue('mcp-key-1'), + createCredential: vi.fn(async (name: string, type: string) => { + created.push({ name, type }); + return await Promise.resolve({ id: `cred-${String(created.length)}` }); + }), + }; + return { member, client: member as unknown as N8nClient, created }; + } + + it("accepts the pooled invite as the member and returns that user's MCP key", async () => { + const { client: ownerClient } = fakeOwnerClient(); + const pool = new LaneUserPool(ownerClient, 1); + const { member, client } = fakeMemberClient(); + + const mcpApiKey = await provisionCaseBuildUser({ + pool, + memberClient: client, + onCredentialCreated: () => {}, + }); + + expect(member.acceptInvitation).toHaveBeenCalledTimes(1); + const acceptArgs = member.acceptInvitation.mock.calls[0][0] as { + token: string; + password: string; + }; + expect(acceptArgs.password).toBe(pool.password); + expect(acceptArgs.token).toContain('token-'); + expect(mcpApiKey).toBe('mcp-key-1'); + expect(member.createCredential).not.toHaveBeenCalled(); + }); + + it('seeds declared credentials via the member client and reports each id', async () => { + const { client: ownerClient } = fakeOwnerClient(); + const pool = new LaneUserPool(ownerClient, 1); + const { client, created } = fakeMemberClient(); + const credentials: TestCaseCredential[] = [{ type: 'slackApi' }, { type: 'notionApi' }]; + const reported: string[] = []; + + await provisionCaseBuildUser({ + pool, + memberClient: client, + credentials, + onCredentialCreated: (id) => reported.push(id), + }); + + expect(created.map((c) => c.type)).toEqual(['slackApi', 'notionApi']); + expect(reported).toEqual(['cred-1', 'cred-2']); + }); + + it('propagates credential-seeding failures so the build fails instead of skewing', async () => { + const { client: ownerClient } = fakeOwnerClient(); + const pool = new LaneUserPool(ownerClient, 1); + const { member, client } = fakeMemberClient(); + member.createCredential.mockRejectedValueOnce(new Error('credential POST failed')); + + await expect( + provisionCaseBuildUser({ + pool, + memberClient: client, + credentials: [{ type: 'slackApi' }], + onCredentialCreated: () => {}, + }), + ).rejects.toThrow('credential POST failed'); + }); +}); + +describe('cleanupLaneUsers', () => { + it('deletes every invited user best-effort, surviving individual failures', async () => { + const { client: ownerClient } = fakeOwnerClient(); + const pool = new LaneUserPool(ownerClient, 3); + await pool.claim(); + + const deleteUser = vi.fn().mockRejectedValueOnce(new Error('409')).mockResolvedValue(undefined); + await cleanupLaneUsers({ deleteUser } as unknown as N8nClient, pool, silentLogger); + + expect(deleteUser).toHaveBeenCalledTimes(3); + }); +}); diff --git a/packages/@n8n/instance-ai/evaluations/__tests__/mcp-builder.test.ts b/packages/@n8n/instance-ai/evaluations/__tests__/mcp-builder.test.ts index 3f222b1b780..b4cb97a16e9 100644 --- a/packages/@n8n/instance-ai/evaluations/__tests__/mcp-builder.test.ts +++ b/packages/@n8n/instance-ai/evaluations/__tests__/mcp-builder.test.ts @@ -129,6 +129,12 @@ describe('unsupportedMcpBuildSetupFields', () => { expect(unsupportedMcpBuildSetupFields(testCase({ credentials: [] }))).toEqual([]); }); + it('does not flag declared credentials (the fused path seeds them per-case)', () => { + expect( + unsupportedMcpBuildSetupFields(testCase({ credentials: [{ type: 'slackApi' }] })), + ).toEqual([]); + }); + it('does not flag messageBudget (inapplicable to a single-shot claude build)', () => { expect(unsupportedMcpBuildSetupFields(testCase({ messageBudget: 6 }))).toEqual([]); }); @@ -136,7 +142,6 @@ describe('unsupportedMcpBuildSetupFields', () => { // One `seed` entry covers every mode: the classification keys off the slot, not // the mode, so a new arm needs no edit here. it.each<[string, string, Partial]>([ - ['credentials', 'credentials', { credentials: [{ type: 'slackApi' }] }], [ 'an inline seed', 'seed', @@ -163,7 +168,7 @@ describe('unsupportedMcpBuildSetupFields', () => { expect(unsupportedMcpBuildSetupFields(testCase(overrides))).toEqual([field]); }); - it('flags multiple declared fields together', () => { + it('flags only the seed when credentials and a seed are declared together', () => { expect( unsupportedMcpBuildSetupFields( testCase({ @@ -171,7 +176,7 @@ describe('unsupportedMcpBuildSetupFields', () => { seed: { mode: 'replay', threadId: 't1' }, }), ), - ).toEqual(['credentials', 'seed']); + ).toEqual(['seed']); }); }); diff --git a/packages/@n8n/instance-ai/evaluations/cli/build-mcp-manifest.ts b/packages/@n8n/instance-ai/evaluations/cli/build-mcp-manifest.ts index e1c058d7aac..2fd9cdb0a63 100644 --- a/packages/@n8n/instance-ai/evaluations/cli/build-mcp-manifest.ts +++ b/packages/@n8n/instance-ai/evaluations/cli/build-mcp-manifest.ts @@ -5,7 +5,7 @@ // loader uses, so shape regressions surface here rather than at eval time. import { execSync } from 'child_process'; -import { existsSync, mkdirSync, readFileSync, readdirSync, unlinkSync, writeFileSync } from 'fs'; +import { existsSync, mkdirSync, readFileSync, readdirSync, writeFileSync } from 'fs'; import { basename, join, resolve } from 'path'; import { z } from 'zod'; @@ -523,14 +523,8 @@ async function main(): Promise { repoRoot, repoRoot ? undefined : process.cwd(), ]); + // Removed on process exit by the staging module itself. const mcpConfigPath = stageMcpConfigFromClaudeJson(args.mcpServerName, projectScopes); - process.on('exit', () => { - try { - unlinkSync(mcpConfigPath); - } catch { - // best-effort - } - }); const tasks: Array<{ slug: string; iteration: number }> = []; for (const slug of args.slugs) { diff --git a/packages/@n8n/instance-ai/evaluations/cli/index.ts b/packages/@n8n/instance-ai/evaluations/cli/index.ts index 6c6725fedda..1815b7ad89a 100644 --- a/packages/@n8n/instance-ai/evaluations/cli/index.ts +++ b/packages/@n8n/instance-ai/evaluations/cli/index.ts @@ -42,7 +42,7 @@ async function main(): Promise { // One lane per base URL; the drivers dispatch builds across them via the // work-stealing allocator inside the shared eval session. - const { lanes, cleanupStagedMcpConfigs } = await setupLanes(args, logger); + const lanes = await setupLanes(args, logger); const startTime = Date.now(); // Delete workflows after the run when they're throwaway: prebuilt opt-in @@ -131,7 +131,6 @@ async function main(): Promise { }); } finally { await cleanupLanes(lanes, cleanupBuiltWorkflows, logger); - cleanupStagedMcpConfigs(); } } diff --git a/packages/@n8n/instance-ai/evaluations/cli/mcp-builder.ts b/packages/@n8n/instance-ai/evaluations/cli/mcp-builder.ts index a6c1959e9ea..ec5085bb927 100644 --- a/packages/@n8n/instance-ai/evaluations/cli/mcp-builder.ts +++ b/packages/@n8n/instance-ai/evaluations/cli/mcp-builder.ts @@ -15,7 +15,7 @@ // --------------------------------------------------------------------------- import { spawn } from 'child_process'; -import { existsSync, readFileSync, writeFileSync } from 'fs'; +import { existsSync, readFileSync, unlinkSync, writeFileSync } from 'fs'; import { setTimeout as delay } from 'node:timers/promises'; import { homedir, tmpdir } from 'os'; import { join } from 'path'; @@ -61,6 +61,24 @@ export function uniqueProjectScopes(scopes: Array): string[] return uniqueDefined(scopes); } +// Every staged config embeds a bearer token, so exit cleanup is intrinsic to +// staging: writeMcpConfig tracks each path and the exit hook unlinks whatever +// is left. Callers that finish normally unlink eagerly via unlinkStagedMcpConfig. +const stagedConfigPaths = new Set(); +process.on('exit', () => { + for (const staged of stagedConfigPaths) unlinkStagedMcpConfig(staged); +}); + +/** Eagerly remove a staged MCP config (and drop it from exit cleanup). */ +export function unlinkStagedMcpConfig(path: string): void { + stagedConfigPaths.delete(path); + try { + unlinkSync(path); + } catch { + // best-effort + } +} + function writeMcpConfig(serverName: string, block: unknown, filePrefix: string): string { // A random suffix (not just pid+time) keeps concurrent lane stages from // colliding when several are written in the same millisecond. @@ -69,6 +87,7 @@ function writeMcpConfig(serverName: string, block: unknown, filePrefix: string): `${filePrefix}-${String(process.pid)}-${String(Date.now())}-${Math.random().toString(36).slice(2, 8)}.json`, ); writeFileSync(tmpPath, JSON.stringify({ mcpServers: { [serverName]: block } }), { mode: 0o600 }); + stagedConfigPaths.add(tmpPath); return tmpPath; } @@ -102,10 +121,11 @@ export function stageMcpConfigFromClaudeJson( } /** - * Stage an MCP config for one eval lane directly from its URL + API key, with no + * Stage an MCP config for one build directly from a lane URL + API key, with no * dependency on `~/.claude.json`. Used by the fused `--build-via-mcp` path where - * the eval CLI mints an MCP key per lane and points `claude` at that lane's MCP - * server. Returns the temp config path; the caller owns cleanup. + * the eval CLI mints an MCP key per build user and points `claude` at that + * lane's MCP server. Returns the temp config path; removed on process exit, or + * earlier via unlinkStagedMcpConfig. */ export function stageLaneMcpConfig(opts: { serverName: string; @@ -134,12 +154,13 @@ export function buildAllowedTools(serverName: string): readonly string[] { type McpBuildKeySupport = 'supported' | 'orchestrator-only'; /** - * Classification of EVERY test-case schema key for the `claude -p` MCP build - * path. `orchestrator-only` keys are build-side setup the orchestrator seeds - * before driving the in-product agent (credential creation, conversation/thread - * seeding), while `claude` receives only the flattened conversation prompt — a - * case relying on them would build without its prerequisites and fail - * misleadingly, so callers skip cases that declare them. + * Classification of EVERY test-case schema key for the fused `--build-via-mcp` + * build path (the standalone manifest builder does not consult this filter). + * `orchestrator-only` keys are build-side setup the orchestrator seeds + * before driving the in-product agent (conversation/thread seeding), while + * `claude` receives only the flattened conversation prompt — a case relying on + * them would build without its prerequisites and fail misleadingly, so callers + * skip cases that declare them. * * Deliberately exhaustive rather than a blocklist: the type covers the * WorkflowTestCase interface (plus the schema's forbidden legacy key), and a @@ -167,7 +188,10 @@ export const MCP_BUILD_KEY_SUPPORT: Record< // Forbidden legacy key — the schema rejects it at load, so it can never // reach this check; classified only to keep the map schema-complete. buildExpectations: 'supported', - credentials: 'orchestrator-only', + // The fused --build-via-mcp path seeds these into the per-build member + // user's personal project (the standalone manifest builder cannot — it has + // no n8n session). + credentials: 'supported', seed: 'orchestrator-only', datasets: 'supported', }; @@ -189,7 +213,7 @@ export function unsupportedMcpBuildSetupFields(testCase: WorkflowTestCase): stri return ORCHESTRATOR_ONLY_KEYS.filter((key) => { const value = values[key]; if (value === undefined || value === null || value === '') return false; - // An empty array (e.g. credentials: []) declares nothing to seed. + // An empty array declares nothing to seed. return !Array.isArray(value) || value.length > 0; }); } diff --git a/packages/@n8n/instance-ai/evaluations/clients/n8n-client.ts b/packages/@n8n/instance-ai/evaluations/clients/n8n-client.ts index 227def41917..8fc84d429d4 100644 --- a/packages/@n8n/instance-ai/evaluations/clients/n8n-client.ts +++ b/packages/@n8n/instance-ai/evaluations/clients/n8n-client.ts @@ -47,6 +47,21 @@ function serverBudgetFor(timeoutMs: number): number { return Math.max(timeoutMs - CLIENT_ABORT_MARGIN_MS, 30_000); } +// -- Invitation response shapes ------------------------------------------------ + +const InvitedUsersEnvelope = z.object({ + data: z.array( + z.object({ + user: z.object({ + id: z.string(), + email: z.string(), + inviteAcceptUrl: z.string().optional(), + }), + error: z.string().optional(), + }), + ), +}); + // -- Conversation seeding response shapes ------------------------------------- const RestoreThreadEnvelope = z.object({ @@ -660,6 +675,61 @@ export class N8nClient { await this.fetch(`/rest/credentials/${id}`, { method: 'DELETE' }); } + /** + * Invite member users in one batched request. Requires an owner session. + * Returns one row per invitee, reporting rather than throwing on failure: + * n8n creates the user shells before it reports per-invite errors, so the + * caller needs every id back to clean up. `acceptToken` is present only when + * the invite was not emailed (`inviteAcceptUrl` is the token's only carrier, + * and it is withheld when SMTP is configured or N8N_INVITE_LINKS_EMAIL_ONLY + * is set). + * POST /rest/invitations body: [{ email, role: 'global:member' }, ...] + */ + async inviteMembers( + emails: string[], + ): Promise> { + if (emails.length === 0) return []; + const response = InvitedUsersEnvelope.parse( + await this.fetch('/rest/invitations', { + method: 'POST', + body: emails.map((email) => ({ email, role: 'global:member' })), + }), + ); + return response.data.map(({ user, error }) => ({ + id: user.id, + email: user.email, + acceptToken: user.inviteAcceptUrl + ? (new URL(user.inviteAcceptUrl).searchParams.get('token') ?? undefined) + : undefined, + error: error === '' ? undefined : error, + })); + } + + /** + * Accept an invitation. The response issues the new user's session cookie, + * so on a fresh N8nClient this doubles as their login. + * POST /rest/invitations/accept + */ + async acceptInvitation(opts: { + token: string; + firstName: string; + lastName: string; + password: string; + }): Promise { + await this.fetch('/rest/invitations/accept', { method: 'POST', body: opts }); + if (!this.sessionCookie) { + throw new Error('Invitation accepted but no session cookie received'); + } + } + + /** + * Delete a user, including the data remaining in their personal project. + * DELETE /rest/users/:id + */ + async deleteUser(id: string): Promise { + await this.fetch(`/rest/users/${id}`, { method: 'DELETE' }); + } + /** * Pin a build thread's credential view to exactly these IDs (empty array = * the thread sees no credentials). diff --git a/packages/@n8n/instance-ai/evaluations/run/build-orchestrator.ts b/packages/@n8n/instance-ai/evaluations/run/build-orchestrator.ts index fdec3f3c1e6..42ba6634778 100644 --- a/packages/@n8n/instance-ai/evaluations/run/build-orchestrator.ts +++ b/packages/@n8n/instance-ai/evaluations/run/build-orchestrator.ts @@ -12,11 +12,18 @@ import type { InstanceAiRunDebugResponse } from '@n8n/api-types'; import { sleep } from '@n8n/utils/sleep'; import type { LaneAllocator } from './lane-allocator'; +import { provisionCaseBuildUser, type LaneUserPool } from './lane-users'; import { selectAuthorExpectations } from '../build-expectations/select'; import { allFailVerdicts, verifyBuildExpectations } from '../build-expectations/verifier'; import type { CliArgs } from '../cli/args'; -import { buildWorkflowViaMcp, type McpBuildSettings } from '../cli/mcp-builder'; -import type { N8nClient } from '../clients/n8n-client'; +import { + buildWorkflowViaMcp, + stageLaneMcpConfig, + unlinkStagedMcpConfig, + type McpBuildResult, + type McpBuildSettings, +} from '../cli/mcp-builder'; +import { N8nClient } from '../clients/n8n-client'; import { fetchAgentScenarioContext, findAgentArtifactRef, @@ -36,6 +43,7 @@ import { import type { executeScenario } from '../harness/scenario-execution'; import type { ScenarioSeedContext } from '../harness/seed-tables'; import { + extractErrorMessage, findProviderOutage, isRequestAbort, isTransientNetworkError, @@ -45,6 +53,7 @@ import { import type { BuildExpectationResult, ExecutionScenario, + TestCaseCredential, TranscriptTurn, WorkflowTestCase, } from '../types'; @@ -68,9 +77,9 @@ export interface Lane { * per-lane because prebuilt/MCP-built workflows only exist on their own lane — * deleting them via another lane's client would 404. */ workflowIdsToDelete: Set; - /** Staged `claude` MCP config for this lane (`--build-via-mcp` only). Points - * `claude -p` at this lane's own MCP server + minted API key. Cleaned up on exit. */ - mcpConfigPath?: string; + /** Pool of invited member users for `--build-via-mcp` — each build claims one + * so its MCP credential/workflow view is isolated to that user. */ + mcpUserPool?: LaneUserPool; } /** One `claude` build's Anthropic spend (`--build-via-mcp` only). Mirrors @@ -136,10 +145,15 @@ function mcpBuildSettingsFromArgs(args: CliArgs): McpBuildSettings { * verify path is identical to `--prebuilt-workflows`. Never throws: a failed * build resolves to an unsuccessful BuildResult. The workflow lives on `lane`, * so the caller must verify it on that same lane. + * + * Each build runs as its own member user with the declared credentials seeded + * into that user's personal project (see lane-users.ts); the lane owner's + * global scopes cover the fetch-back and verification. */ async function buildWorkflowViaMcpOnLane(config: { lane: Lane; conversation: WorkflowTestCase['conversation']; + credentials?: TestCaseCredential[]; slug: string; iteration: number; args: CliArgs; @@ -148,27 +162,53 @@ async function buildWorkflowViaMcpOnLane(config: { /** Run-wide spend collector; every attempt is recorded, success or not. */ buildSpend: McpBuildSpend[]; }): Promise { - const { lane, conversation, slug, iteration, args, logDir, logger, buildSpend } = config; - if (!lane.mcpConfigPath) { - return { - success: false, - error: `Lane ${lane.baseUrl} has no staged MCP config — cannot build via MCP`, - workflowJsons: [], - createdWorkflowIds: [], - createdDataTableIds: [], - }; + const { lane, conversation, credentials, slug, iteration, args, logDir, logger, buildSpend } = + config; + const failure = (error: string): BuildResult => ({ + success: false, + error, + workflowJsons: [], + createdWorkflowIds: [], + createdDataTableIds: [], + }); + if (!lane.mcpUserPool) { + return failure(`Lane ${lane.baseUrl} has no MCP build user pool — cannot build via MCP`); } - const result = await buildWorkflowViaMcp({ - conversation: conversation ?? [], - slug, - iteration, - mcpConfigPath: lane.mcpConfigPath, - settings: mcpBuildSettingsFromArgs(args), - logDir, - log: (message) => logger.info(message), + let mcpApiKey: string; + try { + mcpApiKey = await provisionCaseBuildUser({ + pool: lane.mcpUserPool, + memberClient: new N8nClient(lane.baseUrl), + credentials, + onCredentialCreated: (id) => lane.createdCredentialIds.add(id), + logger, + }); + } catch (error) { + return failure(`MCP build user/credential setup failed: ${extractErrorMessage(error)}`); + } + + const mcpConfigPath = stageLaneMcpConfig({ + serverName: args.mcpServerName, + url: `${lane.baseUrl}/mcp-server/http`, + apiKey: mcpApiKey, }); + let result: McpBuildResult; + try { + result = await buildWorkflowViaMcp({ + conversation: conversation ?? [], + slug, + iteration, + mcpConfigPath, + settings: mcpBuildSettingsFromArgs(args), + logDir, + log: (message) => logger.info(message), + }); + } finally { + unlinkStagedMcpConfig(mcpConfigPath); + } + // Record spend whether or not the build produced a workflow — failed builds // cost money too, and this is the run's spend record. buildSpend.push({ costUsd: result.cost, turns: result.turns }); @@ -183,13 +223,7 @@ async function buildWorkflowViaMcpOnLane(config: { } if (!result.workflowId) { - return { - success: false, - error: `MCP build produced no workflow (${result.failureReason ?? 'unknown'})`, - workflowJsons: [], - createdWorkflowIds: [], - createdDataTableIds: [], - }; + return failure(`MCP build produced no workflow (${result.failureReason ?? 'unknown'})`); } return await fetchPrebuiltBuild(lane.client, result.workflowId, logger); @@ -416,6 +450,7 @@ export function createBuildOrchestrator(deps: BuildOrchestratorDeps): BuildOrche build = await buildWorkflowViaMcpOnLane({ lane: lane.runner, conversation: entry.conversation, + credentials: entry.credentials, slug: fileSlug, iteration, args, diff --git a/packages/@n8n/instance-ai/evaluations/run/case-selection.ts b/packages/@n8n/instance-ai/evaluations/run/case-selection.ts index 93b0127ff15..41c76c3b36e 100644 --- a/packages/@n8n/instance-ai/evaluations/run/case-selection.ts +++ b/packages/@n8n/instance-ai/evaluations/run/case-selection.ts @@ -64,9 +64,10 @@ export function selectCases( } // `claude -p` builds get only the flattened conversation — build-side setup - // (credentials, seeds) is orchestrator-only. Skip cases that declare it - // rather than building them without prerequisites and reporting misleading - // failures (mirrors the prebuilt-coverage partition above). + // the fused path can't provide (conversation/thread seeds) is + // orchestrator-only. Skip cases that declare it rather than building them + // without prerequisites and reporting misleading failures (mirrors the + // prebuilt-coverage partition above). if (args.buildViaMcp) { const skippedMcp: string[] = []; testCasesWithFiles = testCasesWithFiles.filter(({ testCase, fileSlug }) => { diff --git a/packages/@n8n/instance-ai/evaluations/run/lane-setup.ts b/packages/@n8n/instance-ai/evaluations/run/lane-setup.ts index 02d77a9a1ba..a1f68dc8e93 100644 --- a/packages/@n8n/instance-ai/evaluations/run/lane-setup.ts +++ b/packages/@n8n/instance-ai/evaluations/run/lane-setup.ts @@ -1,14 +1,12 @@ // --------------------------------------------------------------------------- // Lane setup — one authenticated n8n lane per --base-url (TRUST-261): -// login, MCP-registry seed, optional `claude` MCP config staging, and the -// pre-run workflow snapshot. Plus the end-of-run per-lane artifact cleanup. +// login, MCP-registry seed, optional MCP build-user pool, and the pre-run +// workflow snapshot. Plus the end-of-run per-lane artifact cleanup. // --------------------------------------------------------------------------- -import { unlinkSync } from 'fs'; - import type { Lane } from './build-orchestrator'; +import { cleanupLaneUsers, LaneUserPool } from './lane-users'; import type { CliArgs } from '../cli/args'; -import { stageLaneMcpConfig } from '../cli/mcp-builder'; import { N8nClient } from '../clients/n8n-client'; import { cleanupCredentials } from '../credentials/seeder'; import type { EvalLogger } from '../harness/logger'; @@ -16,30 +14,7 @@ import { cleanupPrebuiltWorkflows } from '../harness/prebuilt-workflows'; import { seedMcpRegistry } from '../mcp-registry/seeder'; import { snapshotDataTableIds, snapshotWorkflowIds } from '../outcome/workflow-discovery'; -export interface LaneSetup { - lanes: Lane[]; - /** Removes staged `claude` MCP configs (they embed lane bearer tokens). Also - * registered on process exit; calling it twice is safe. */ - cleanupStagedMcpConfigs: () => void; -} - -export async function setupLanes(args: CliArgs, logger: EvalLogger): Promise { - // Remove staged MCP configs (they embed the lane's bearer token) on exit, - // belt-and-suspenders alongside the explicit finally cleanup below. - // Registered before lane init and populated as configs are staged, so a - // lane failing setup can't strand another lane's already-staged token file. - const stagedMcpConfigPaths: string[] = []; - const cleanupStagedMcpConfigs = () => { - for (const path of stagedMcpConfigPaths) { - try { - unlinkSync(path); - } catch { - // best-effort - } - } - }; - if (args.buildViaMcp) process.on('exit', cleanupStagedMcpConfigs); - +export async function setupLanes(args: CliArgs, logger: EvalLogger): Promise { // One lane per base URL. The LangSmith path then uses a work-stealing // allocator (lane-allocator.ts) to dispatch builds across lanes; the direct // path partitions test cases statically per lane. @@ -62,21 +37,13 @@ export async function setupLanes(args: CliArgs, logger: EvalLogger): Promise {}); + // Deleting a user deletes their remaining data, so keep the build + // users when workflows are kept. + if (cleanupBuiltWorkflows && lane.mcpUserPool) { + await cleanupLaneUsers(lane.client, lane.mcpUserPool, logger); + } }), ); } diff --git a/packages/@n8n/instance-ai/evaluations/run/lane-users.ts b/packages/@n8n/instance-ai/evaluations/run/lane-users.ts new file mode 100644 index 00000000000..d6fc274d8f1 --- /dev/null +++ b/packages/@n8n/instance-ai/evaluations/run/lane-users.ts @@ -0,0 +1,133 @@ +// --------------------------------------------------------------------------- +// Per-case build users for `--build-via-mcp`. +// +// MCP credential/workflow visibility is user-scoped, so running each build as +// its own freshly-invited member gives it an isolated credential view holding +// exactly the case's declared credentials — the MCP analog of the +// orchestrator's per-thread credential pinning. +// --------------------------------------------------------------------------- + +import { randomBytes } from 'crypto'; + +import type { N8nClient } from '../clients/n8n-client'; +import { createDeclaredCredentials } from '../credentials/seeder'; +import type { EvalLogger } from '../harness/logger'; +import type { TestCaseCredential } from '../types'; + +export interface InvitedCaseUser { + id: string; + email: string; + acceptToken: string; +} + +const INVITE_CHUNK_SIZE = 8; + +export class LaneUserPool { + private readonly available: InvitedCaseUser[] = []; + + private refill?: Promise; + + /** Every user this pool invited (accepted or not), for post-run deletion. */ + readonly createdUserIds: string[] = []; + + private seq = 0; + + private readonly nonce = randomBytes(4).toString('hex'); + + /** Shared by this pool's throwaway members; random per run. */ + readonly password = `Eval1!${randomBytes(12).toString('hex')}`; + + constructor( + private readonly ownerClient: N8nClient, + private readonly chunkSize = INVITE_CHUNK_SIZE, + ) {} + + /** Claim an invited-but-unaccepted user, inviting a fresh chunk when the + * pool runs dry. Concurrent claims share one in-flight refill. */ + async claim(): Promise { + let user = this.available.pop(); + while (!user) { + this.refill ??= this.doRefill().finally(() => (this.refill = undefined)); + await this.refill; + user = this.available.pop(); + } + return user; + } + + private async doRefill(): Promise { + // One batched POST per chunk — /rest/invitations is IP-rate-limited in production. + const emails = Array.from({ length: this.chunkSize }, () => { + this.seq += 1; + return `eval-mcp-${this.nonce}-${String(this.seq)}@n8n-evals.invalid`; + }); + const invited = await this.ownerClient.inviteMembers(emails); + // Record every shell before judging usability: n8n creates them all up + // front, so even an unusable batch has to be deleted after the run. + for (const user of invited) this.createdUserIds.push(user.id); + + const usable = invited.filter( + (user): user is InvitedCaseUser => user.acceptToken !== undefined, + ); + this.available.push(...usable); + if (usable.length === 0) { + const reason = invited.map((user) => user.error).find(Boolean); + throw new Error( + `Invited ${String(invited.length)} user(s) but none returned an accept token${reason ? ` (${reason})` : ''} — the lane must run without SMTP and without N8N_INVITE_LINKS_EMAIL_ONLY for the eval build-user flow`, + ); + } + } +} + +/** + * Accept a pooled invitation (which logs the fresh client in as the member), + * mint and return the member's MCP API key, and create the case's declared + * credentials in their personal project. Throws on any step so a partial setup + * fails the build instead of skewing it. + */ +export async function provisionCaseBuildUser(opts: { + pool: LaneUserPool; + /** A fresh, logged-out client for the lane — accepting the invite logs it in. */ + memberClient: N8nClient; + credentials?: TestCaseCredential[]; + onCredentialCreated: (id: string) => void; + logger?: EvalLogger; +}): Promise { + const { pool, memberClient } = opts; + const user = await pool.claim(); + await memberClient.acceptInvitation({ + token: user.acceptToken, + firstName: 'Eval', + lastName: 'Builder', + password: pool.password, + }); + const mcpApiKey = await memberClient.rotateMcpApiKey(); + await createDeclaredCredentials(memberClient, opts.credentials ?? [], { + onCreated: opts.onCredentialCreated, + logger: opts.logger, + }); + return mcpApiKey; +} + +/** Best-effort deletion of the pool's users. Deleting a user also deletes + * what's left in their personal project, so only call this when built + * workflows are throwaway. */ +export async function cleanupLaneUsers( + ownerClient: N8nClient, + pool: LaneUserPool, + logger: EvalLogger, +): Promise { + const ids = pool.createdUserIds; + if (ids.length === 0) return; + let deleted = 0; + // Sequential, like cleanupCredentials: each delete cascades server-side and + // this runs after the last verdict, so nothing waits on it. + for (const id of ids) { + try { + await ownerClient.deleteUser(id); + deleted++; + } catch { + // best-effort + } + } + logger.verbose(`Deleted ${String(deleted)}/${String(ids.length)} MCP build user(s)`); +}