From 9087a5ac6ddbf8306d82524876370eaa2aaf637d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Milorad=20FIlipovi=C4=87?= Date: Tue, 2 Jun 2026 09:33:08 +0200 Subject: [PATCH] fix(core): Fix multi-turn evals for mcp (no-changelog) (#31470) --- .../evaluations/cli/build-mcp-manifest.ts | 22 +++++- packages/cli/src/credentials-helper.ts | 6 +- .../src/errors/credential-missing-id.error.ts | 10 +++ .../eval-mocked-credentials-helper.test.ts | 77 ++++++++++++++++++- .../eval/eval-mocked-credentials-helper.ts | 8 +- 5 files changed, 114 insertions(+), 9 deletions(-) create mode 100644 packages/cli/src/errors/credential-missing-id.error.ts 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 3bff0ed6f86..308d54ba29c 100644 --- a/packages/@n8n/instance-ai/evaluations/cli/build-mcp-manifest.ts +++ b/packages/@n8n/instance-ai/evaluations/cli/build-mcp-manifest.ts @@ -364,6 +364,26 @@ const testCaseSchema = z }) .passthrough(); +function buildPromptFromConversation( + conversation: z.infer['conversation'], +): string { + const [firstUserTurn, ...remainingUserTurns] = conversation + .filter((turn) => turn.role === 'user') + .map((turn) => turn.text.trim()) + .filter((text) => text.length > 0); + + if (!firstUserTurn) return conversation[0].text; + if (remainingUserTurns.length === 0) return firstUserTurn; + + return [ + firstUserTurn, + 'Additional details from the user:', + ...remainingUserTurns.map((turn, index) => `${String(index + 1)}. ${turn}`), + '', + "Use all details above as requirements. Configure all nodes as completely as possible and don't ask me for credentials; I'll set them up later.", + ].join('\n\n'); +} + function tailWorkflowId(text: string): string | null { const matches = [...text.matchAll(/WORKFLOW_ID=([A-Za-z0-9_-]+)/g)]; return matches.length > 0 ? matches[matches.length - 1][1] : null; @@ -388,7 +408,7 @@ async function buildOne( ? `\n\nWhen calling create_workflow_from_code, pass projectId: '${args.projectId}' so the workflow is created in that n8n project.` : ''; - const userMessage = `${testCase.conversation[0].text}${projectInstruction} + const userMessage = `${buildPromptFromConversation(testCase.conversation)}${projectInstruction} --- After you have created the workflow with create_workflow_from_code, print a final line of the exact form: diff --git a/packages/cli/src/credentials-helper.ts b/packages/cli/src/credentials-helper.ts index ba31ff2a997..c20b141658d 100644 --- a/packages/cli/src/credentials-helper.ts +++ b/packages/cli/src/credentials-helper.ts @@ -40,6 +40,7 @@ import { import { RESPONSE_ERROR_MESSAGES } from './constants'; import { DynamicCredentialsProxy } from './credentials/dynamic-credentials-proxy'; +import { CredentialMissingIdError } from './errors/credential-missing-id.error'; import { CredentialNotFoundError } from './errors/credential-not-found.error'; import { CredentialTypes } from '@/credential-types'; @@ -291,10 +292,7 @@ export class CredentialsHelper extends ICredentialsHelper { type: string, ): Promise { if (!nodeCredential.id) { - throw new UnexpectedError('Found credential with no ID.', { - extra: { credentialName: nodeCredential.name }, - tags: { credentialType: type }, - }); + throw new CredentialMissingIdError(nodeCredential.name, type); } let credential: CredentialsEntity; diff --git a/packages/cli/src/errors/credential-missing-id.error.ts b/packages/cli/src/errors/credential-missing-id.error.ts new file mode 100644 index 00000000000..bb1024656e8 --- /dev/null +++ b/packages/cli/src/errors/credential-missing-id.error.ts @@ -0,0 +1,10 @@ +import { UnexpectedError } from 'n8n-workflow'; + +export class CredentialMissingIdError extends UnexpectedError { + constructor(credentialName: string, credentialType: string) { + super('Found credential with no ID.', { + extra: { credentialName }, + tags: { credentialType }, + }); + } +} diff --git a/packages/cli/src/modules/instance-ai/eval/__tests__/eval-mocked-credentials-helper.test.ts b/packages/cli/src/modules/instance-ai/eval/__tests__/eval-mocked-credentials-helper.test.ts index 7f96c592917..d76832cbc15 100644 --- a/packages/cli/src/modules/instance-ai/eval/__tests__/eval-mocked-credentials-helper.test.ts +++ b/packages/cli/src/modules/instance-ai/eval/__tests__/eval-mocked-credentials-helper.test.ts @@ -11,7 +11,9 @@ import type { IWorkflowExecuteAdditionalData, Workflow, } from 'n8n-workflow'; +import { UnexpectedError } from 'n8n-workflow'; +import { CredentialMissingIdError } from '@/errors/credential-missing-id.error'; import { CredentialNotFoundError } from '@/errors/credential-not-found.error'; import { EvalMockedCredentialsHelper } from '../eval-mocked-credentials-helper'; @@ -88,6 +90,20 @@ describe('EvalMockedCredentialsHelper', () => { expect(helper.mockedCredentials).toEqual([]); }); + it('rethrows generic no-id UnexpectedError errors', async () => { + const inner = makeInner({ + getDecrypted: jest + .fn() + .mockRejectedValue(new UnexpectedError('Found credential with no ID.')), + }); + const helper = new EvalMockedCredentialsHelper(inner); + + await expect( + helper.getDecrypted(fakeAdditionalData, fakeNodeCreds, 'telegramApi', 'manual'), + ).rejects.toThrow('Found credential with no ID.'); + expect(helper.mockedCredentials).toEqual([]); + }); + it('records "unknown" nodeName when executeData is missing', async () => { const inner = makeInner({ getDecrypted: jest.fn().mockRejectedValue(new CredentialNotFoundError('id', 'telegramApi')), @@ -414,8 +430,8 @@ describe('EvalMockedCredentialsHelper', () => { }); }); - describe('getDecrypted — schema synthesis when id is null', () => { - // `{ id: null }` short-circuits to schema synthesis without delegating to the inner helper. + describe('getDecrypted — schema synthesis when id is falsy', () => { + // Falsy credential ids short-circuit to schema synthesis without delegating to the inner helper. const propsSchema = [ { name: 'apiKey', @@ -433,6 +449,8 @@ describe('EvalMockedCredentialsHelper', () => { ]; const nullNodeCreds: INodeCredentialsDetails = { id: null, name: 'openAiApi' }; + const emptyIdNodeCreds: INodeCredentialsDetails = { id: '', name: 'openAiApi' }; + const noIdNodeCreds = { name: 'openAiApi' } as unknown as INodeCredentialsDetails; function makeSynthesizingInner(): ICredentialsHelper { return makeInner({ @@ -442,6 +460,15 @@ describe('EvalMockedCredentialsHelper', () => { }); } + function makeNoIdInner(): ICredentialsHelper { + return makeInner({ + getCredentialsProperties: jest.fn().mockReturnValue(propsSchema), + getDecrypted: jest + .fn() + .mockRejectedValue(new CredentialMissingIdError('openAiApi', 'openAiApi')), + }); + } + it('synthesizes a credential from the schema and applies the URL rewrite', async () => { const subNodeToRoot = new Map([['OpenAI', 'Agent']]); const helper = new EvalMockedCredentialsHelper( @@ -563,6 +590,52 @@ describe('EvalMockedCredentialsHelper', () => { expect(result.url).toBe('https://api.openai.com/v1'); expect(helper.rewrittenCredentials).toEqual([]); }); + + it('synthesizes a credential when the inner helper reports a missing id', async () => { + const helper = new EvalMockedCredentialsHelper(makeNoIdInner()); + + const result = await helper.getDecrypted( + fakeAdditionalData, + noIdNodeCreds, + 'openAiApi', + 'manual', + { node: { name: 'OpenAI' } as INode } as IExecuteData, + ); + + expect(result.__evalMockedCredential).toBe(true); + expect(typeof result.apiKey).toBe('string'); + expect(result.url).toBe('https://api.openai.com/v1'); + expect(helper.mockedCredentials).toEqual([ + { + nodeName: 'OpenAI', + credentialType: 'openAiApi', + credentialId: undefined, + }, + ]); + }); + + it('synthesizes a credential when the credential id is empty', async () => { + const helper = new EvalMockedCredentialsHelper(makeNoIdInner()); + + const result = await helper.getDecrypted( + fakeAdditionalData, + emptyIdNodeCreds, + 'openAiApi', + 'manual', + { node: { name: 'OpenAI' } as INode } as IExecuteData, + ); + + expect(result.__evalMockedCredential).toBe(true); + expect(typeof result.apiKey).toBe('string'); + expect(result.url).toBe('https://api.openai.com/v1'); + expect(helper.mockedCredentials).toEqual([ + { + nodeName: 'OpenAI', + credentialType: 'openAiApi', + credentialId: undefined, + }, + ]); + }); }); describe('no-id credential references — regression for "Found credential with no ID."', () => { diff --git a/packages/cli/src/modules/instance-ai/eval/eval-mocked-credentials-helper.ts b/packages/cli/src/modules/instance-ai/eval/eval-mocked-credentials-helper.ts index 827cf42ce7f..cc071ad42a2 100644 --- a/packages/cli/src/modules/instance-ai/eval/eval-mocked-credentials-helper.ts +++ b/packages/cli/src/modules/instance-ai/eval/eval-mocked-credentials-helper.ts @@ -31,6 +31,10 @@ export const EVAL_PROVIDER_URL_FIELD: Record