mirror of
https://github.com/n8n-io/n8n.git
synced 2026-08-28 17:22:01 +08:00
fix(core): Fix multi-turn evals for mcp (no-changelog) (#31470)
This commit is contained in:
committed by
GitHub
parent
e3c14a4720
commit
9087a5ac6d
@@ -364,6 +364,26 @@ const testCaseSchema = z
|
||||
})
|
||||
.passthrough();
|
||||
|
||||
function buildPromptFromConversation(
|
||||
conversation: z.infer<typeof testCaseSchema>['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:
|
||||
|
||||
@@ -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<CredentialsEntity> {
|
||||
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;
|
||||
|
||||
@@ -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 },
|
||||
});
|
||||
}
|
||||
}
|
||||
+75
-2
@@ -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<string, string>([['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."', () => {
|
||||
|
||||
@@ -31,6 +31,10 @@ export const EVAL_PROVIDER_URL_FIELD: Record<string, { field: string; pathPrefix
|
||||
openAiApi: { field: 'url', pathPrefix: '/v1' },
|
||||
};
|
||||
|
||||
function getCredentialId(nodeCredentials: INodeCredentialsDetails): string | undefined {
|
||||
return nodeCredentials.id ? nodeCredentials.id : undefined;
|
||||
}
|
||||
|
||||
/** CredentialsHelper proxy for eval: tolerates missing credentials and (optionally) rewrites vendor URLs to the wire server. */
|
||||
export class EvalMockedCredentialsHelper extends ICredentialsHelper {
|
||||
readonly mockedCredentials: InstanceAiEvalMockedCredential[] = [];
|
||||
@@ -137,7 +141,7 @@ export class EvalMockedCredentialsHelper extends ICredentialsHelper {
|
||||
this.mockedCredentials.push({
|
||||
nodeName: executeData?.node?.name ?? 'unknown',
|
||||
credentialType: type,
|
||||
credentialId: nodeCredentials.id ?? undefined,
|
||||
credentialId: getCredentialId(nodeCredentials),
|
||||
});
|
||||
credentials = { [MOCK_MARKER]: true };
|
||||
}
|
||||
@@ -179,7 +183,7 @@ export class EvalMockedCredentialsHelper extends ICredentialsHelper {
|
||||
this.rewrittenCredentials.push({
|
||||
nodeName: subNodeName ?? 'unknown',
|
||||
credentialType: type,
|
||||
credentialId: nodeCredentials.id ?? undefined,
|
||||
credentialId: getCredentialId(nodeCredentials),
|
||||
field,
|
||||
});
|
||||
|
||||
|
||||
Reference in New Issue
Block a user