mirror of
https://github.com/n8n-io/n8n.git
synced 2026-10-17 05:29:44 +02:00
fix(ai-builder): Report un-intercepted vendor sub-nodes and stop blaming the mock layer for a root's own crash (no-changelog) (#38155)
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
9043a74e2d
commit
ecda0d4680
|
|
@ -541,6 +541,44 @@ describe('buildVerificationArtifact', () => {
|
|||
expect(artifact.scenarioContext).toContain('**Did not run** (no execution data): none');
|
||||
});
|
||||
|
||||
// TRUST-508: verifiers read a model sub-node's absence as "the harness did not
|
||||
// mock it" and charged the root's own crash to the mock layer.
|
||||
it('tells the verifier what a sub-node under "Did not run" means', () => {
|
||||
const wf: WorkflowResponse = {
|
||||
id: 'w1',
|
||||
name: 'extractor',
|
||||
active: false,
|
||||
versionId: 'v1',
|
||||
nodes: [
|
||||
{
|
||||
id: 'a',
|
||||
name: 'Extract Jobs',
|
||||
type: '@n8n/n8n-nodes-langchain.informationExtractor',
|
||||
typeVersion: 1.2,
|
||||
position: [0, 0],
|
||||
parameters: {},
|
||||
},
|
||||
{
|
||||
id: 'b',
|
||||
name: 'OpenAI Model',
|
||||
type: '@n8n/n8n-nodes-langchain.lmChatOpenAi',
|
||||
typeVersion: 1,
|
||||
position: [0, 0],
|
||||
parameters: {},
|
||||
},
|
||||
],
|
||||
connections: { 'OpenAI Model': { ai_languageModel: [[{ node: 'Extract Jobs', index: 0 }]] } },
|
||||
};
|
||||
const evalResult = makeEvalResult({
|
||||
'Extract Jobs': makeNodeResult({ executionMode: 'real', iterationCount: 1 }),
|
||||
});
|
||||
|
||||
const artifact = buildVerificationArtifact(scenario, evalResult, [wf]);
|
||||
|
||||
expect(artifact.scenarioContext).toContain('**Did not run** (no execution data): OpenAI Model');
|
||||
expect(artifact.scenarioContext).toContain('NOT that the harness declined to mock it');
|
||||
});
|
||||
|
||||
it('head/tail-truncates oversized JSON output blocks and reports chars saved', () => {
|
||||
const wf: WorkflowResponse = {
|
||||
id: 'w1',
|
||||
|
|
|
|||
|
|
@ -601,6 +601,11 @@ function buildScenarioContextBlock(
|
|||
`**Pinned nodes** (synthetic input): ${pinnedNodes.join(', ') || 'none'}`,
|
||||
`**Real nodes** (executed with actual logic): ${realNodes.join(', ') || 'none'}`,
|
||||
`**Did not run** (no execution data): ${didNotRun.join(', ') || 'none'}`,
|
||||
// Verifiers read a sub-node's absence from the mocked/pinned lists as "the
|
||||
// harness skipped mocking this model" and charged the root's own crash to
|
||||
// the mock layer (TRUST-508/510).
|
||||
'',
|
||||
'> An AI sub-node (language model, memory, tool, embeddings) only produces execution data when its root node reaches it, and is served through that root — it never appears under mocked or pinned. Listed above it means the root was pinned or the root failed before invoking it, NOT that the harness declined to mock it. A vendor call the harness could not intercept is always a FRAMEWORK ISSUE flag in Pre-analysis.',
|
||||
'',
|
||||
);
|
||||
|
||||
|
|
|
|||
|
|
@ -9,7 +9,9 @@ This is a test environment. No real credentials or API connections exist. ALL HT
|
|||
- **Real nodes**: Logic nodes (Code, Set, Merge, Filter, Sort, IF, Switch) that executed their actual code on data from mocked/pinned upstream nodes.
|
||||
|
||||
IMPORTANT: Nodes receiving mock responses instead of real API responses is EXPECTED. Missing or mock credentials is EXPECTED. Don't flag these as issues — they are the testing mechanism itself.
|
||||
IMPORTANT: When an AI root node such as an AI Agent is pinned, its connected AI subnodes (language model, memory, tools, retrievers, parsers) often do not run. This is expected. Evaluate those subnodes from the saved workflow structure, connections, and all-node configs instead of failing only because the subnode did not execute.
|
||||
IMPORTANT: An AI subnode (language model, memory, tool, retriever, parser, embeddings) produces execution data ONLY when its root node reaches it, so "Did not run" is its normal state in three cases: the root was never reached at all (an upstream branch sent no items, so the root is absent too), the root was pinned, or the root ran and FAILED BEFORE invoking it. None of these means the harness declined to mock the subnode. When a root errored and its model subnode is listed under "Did not run", read the root's OWN error first — a root that throws while building its request (an unparseable schema parameter, a missing required field, an invalid expression) never gets as far as the model, and that is a builder_issue on the root, not a missing mock. Never infer a mocking gap from a subnode's absence from the mocked/pinned lists: subnodes are served through their root and never appear there.
|
||||
IMPORTANT: A vendor call the harness genuinely could not intercept is ALWAYS reported as a FRAMEWORK ISSUE pre-analysis flag naming the subnode ("executed without interception"). That flag is the only evidence for an un-mocked model call. With no such flag, do not attribute a failure to the harness failing to mock a language model — and never split one root cause between mock_issue and framework_issue across scenarios of the same build: pick the bucket the evidence names and use it for every scenario showing that cause.
|
||||
Evaluate subnodes from the saved workflow structure, connections, and all-node configs instead of failing only because the subnode did not execute.
|
||||
IMPORTANT: AI root output shapes differ by node type, and pinned outputs follow the REAL node behavior. An Agent root wraps its result in \`{ "output": ... }\`. A Basic LLM Chain (\`chainLlm\`) WITH a structured output parser attached also wraps: the parser itself emits the \`{ "output": {...} }\` envelope, so the parsed fields live under \`output\`, never flat at the top level of \`json\`. Without a parser, chainLlm emits \`{ "text": "..." }\` and a retrieval QA chain emits \`{ "response": ... }\`. Attribution follows from this: \`$json.output.*\` read from an Agent or a chainLlm-with-parser root is the CORRECT expression — if it failed to resolve, check the pin first; a pin carrying the parsed fields flat (no \`output\` envelope) for such a root is a mock_issue (wrong fixture shape), NOT a builder_issue. Conversely, \`$json.output.*\` against a chainLlm WITHOUT a parser (real shape \`{ "text": ... }\`) can never resolve against the real node — that IS a builder_issue.
|
||||
IMPORTANT: When the harness resolved the table's real columns, pinned Data Table read outputs mirror that column schema — it reads the actual columns off the table the builder created and enforces them on the pinned rows. In that case, when a downstream expression reads a field that is not among the pinned rows' keys (e.g. \`$json.contact_email\` while the rows carry \`email\`), the same expression would resolve undefined against the real table too — that is a builder_issue (expression references a column the builder never created), NOT a mock/pin issue.
|
||||
|
||||
|
|
|
|||
|
|
@ -164,7 +164,11 @@ describe('EvalMockedCredentialsHelper', () => {
|
|||
describe('server URL rewrite', () => {
|
||||
const serverUrl = 'http://127.0.0.1:55555';
|
||||
const openAiCreds: INodeCredentialsDetails = { id: 'cred-1', name: 'OpenAI cred' };
|
||||
const openAiNode = { name: 'OpenAI Chat Model', id: 'node-9' } as INode;
|
||||
const openAiNode = {
|
||||
name: 'OpenAI Chat Model',
|
||||
id: 'node-9',
|
||||
type: '@n8n/n8n-nodes-langchain.lmChatOpenAi',
|
||||
} as INode;
|
||||
|
||||
it('rewrites the URL field on openAiApi credentials when serverUrl is set', async () => {
|
||||
const inner = makeInner({
|
||||
|
|
@ -849,6 +853,108 @@ describe('EvalMockedCredentialsHelper', () => {
|
|||
});
|
||||
});
|
||||
|
||||
// TRUST-508: an un-intercepted model call used to live only in a server log,
|
||||
// so the verifier scored the scenario as if the builder had wired it wrong.
|
||||
describe('interceptionGaps', () => {
|
||||
const lmNode = {
|
||||
name: 'OpenAI Model',
|
||||
id: 'node-lm',
|
||||
type: '@n8n/n8n-nodes-langchain.lmChatOpenAi',
|
||||
} as INode;
|
||||
const lmCreds: INodeCredentialsDetails = { id: 'cred-1', name: 'OpenAI cred' };
|
||||
|
||||
async function fetchFor(
|
||||
helper: EvalMockedCredentialsHelper,
|
||||
node: INode,
|
||||
type = 'openAiApi',
|
||||
): Promise<void> {
|
||||
await helper.getDecrypted(fakeAdditionalData, lmCreds, type, 'manual', {
|
||||
node,
|
||||
} as IExecuteData);
|
||||
}
|
||||
|
||||
function innerWithUrl(): ICredentialsHelper {
|
||||
return makeInner({
|
||||
getDecrypted: vi
|
||||
.fn()
|
||||
.mockResolvedValue({ apiKey: 'sk-real', url: 'https://api.openai.com/v1' }),
|
||||
});
|
||||
}
|
||||
|
||||
it('records a gap when the sub-node has no routing entry', async () => {
|
||||
const helper = new EvalMockedCredentialsHelper(
|
||||
innerWithUrl(),
|
||||
'http://127.0.0.1:55555',
|
||||
mockLogger,
|
||||
new Map([['Some Other Sub-Node', 'Some Agent']]),
|
||||
);
|
||||
|
||||
await fetchFor(helper, lmNode);
|
||||
|
||||
expect(helper.interceptionGaps).toHaveLength(1);
|
||||
expect(helper.interceptionGaps[0]).toContain('"OpenAI Model"');
|
||||
expect(helper.interceptionGaps[0]).toContain('no vendor LLM routing entry');
|
||||
});
|
||||
|
||||
it('records a gap when the credential type has no rewrite mapping', async () => {
|
||||
const helper = new EvalMockedCredentialsHelper(
|
||||
innerWithUrl(),
|
||||
'http://127.0.0.1:55555',
|
||||
mockLogger,
|
||||
new Map<string, string>(),
|
||||
);
|
||||
|
||||
await fetchFor(
|
||||
helper,
|
||||
{ ...lmNode, type: '@n8n/n8n-nodes-langchain.lmChatAnthropic' },
|
||||
'anthropicApi',
|
||||
);
|
||||
|
||||
expect(helper.interceptionGaps).toHaveLength(1);
|
||||
expect(helper.interceptionGaps[0]).toContain('no wire-server URL rewrite mapping');
|
||||
});
|
||||
|
||||
it('records a gap when interception is off entirely', async () => {
|
||||
const helper = new EvalMockedCredentialsHelper(innerWithUrl(), undefined, mockLogger);
|
||||
|
||||
await fetchFor(helper, lmNode);
|
||||
|
||||
expect(helper.interceptionGaps).toHaveLength(1);
|
||||
expect(helper.interceptionGaps[0]).toContain('interception is not active');
|
||||
});
|
||||
|
||||
it('stays empty for a routed sub-node and for non-vendor nodes', async () => {
|
||||
const routed = new EvalMockedCredentialsHelper(
|
||||
innerWithUrl(),
|
||||
'http://127.0.0.1:55555',
|
||||
mockLogger,
|
||||
new Map([['OpenAI Model', 'Extract Jobs']]),
|
||||
);
|
||||
await fetchFor(routed, lmNode);
|
||||
expect(routed.interceptionGaps).toEqual([]);
|
||||
|
||||
// A Gmail/HTTP node reaching the same branches is fine — its traffic
|
||||
// goes through the HTTP mock whatever its credential URL says.
|
||||
const httpNode = new EvalMockedCredentialsHelper(innerWithUrl(), undefined, mockLogger);
|
||||
await fetchFor(httpNode, { ...fakeNode, type: 'n8n-nodes-base.gmail' }, 'gmailOAuth2');
|
||||
expect(httpNode.interceptionGaps).toEqual([]);
|
||||
});
|
||||
|
||||
it('does not repeat one node/reason across repeated credential fetches', async () => {
|
||||
const helper = new EvalMockedCredentialsHelper(
|
||||
innerWithUrl(),
|
||||
'http://127.0.0.1:55555',
|
||||
mockLogger,
|
||||
new Map<string, string>(),
|
||||
);
|
||||
|
||||
await fetchFor(helper, lmNode);
|
||||
await fetchFor(helper, lmNode);
|
||||
|
||||
expect(helper.interceptionGaps).toHaveLength(1);
|
||||
});
|
||||
});
|
||||
|
||||
describe('passthrough methods', () => {
|
||||
it('delegates passthrough methods to inner', async () => {
|
||||
const inner = makeInner();
|
||||
|
|
|
|||
|
|
@ -453,6 +453,32 @@ describe('partitionAiRoots', () => {
|
|||
});
|
||||
});
|
||||
|
||||
it.each([
|
||||
'@n8n/n8n-nodes-langchain.embeddingsOpenAi',
|
||||
'@n8n/n8n-nodes-langchain.embeddingsCohere',
|
||||
'@n8n/n8n-nodes-langchain.embeddingsGoogleGemini',
|
||||
'@n8n/n8n-nodes-langchain.embeddingsAzureOpenAi',
|
||||
])('auto-pins a root backed by embeddings sub-node %s', (embeddingsType) => {
|
||||
// Embeddings speak the vendor SDK, so the HTTP mock never sees them, and
|
||||
// no `EVAL_PROVIDER_URL_FIELD` entry rewrites their credentials. Left
|
||||
// unpinned the root reaches the real provider on real credentials.
|
||||
const nodes = [
|
||||
makeNode({ name: 'Embeddings', type: embeddingsType }),
|
||||
makeNode({ name: 'Store', type: '@n8n/n8n-nodes-langchain.vectorStoreInMemory' }),
|
||||
];
|
||||
const connections: IConnections = {
|
||||
Embeddings: { ai_embedding: [[{ node: 'Store', type: 'ai_embedding', index: 0 }]] },
|
||||
};
|
||||
const result = partitionAiRoots(makeWorkflow(nodes, connections));
|
||||
expect(result.unpinNodes).toEqual([]);
|
||||
expect(result.pinNodes).toEqual(['Store']);
|
||||
expect(result.autoPinned[0]).toMatchObject({
|
||||
root: 'Store',
|
||||
subNodeType: embeddingsType,
|
||||
reason: 'unsupported_vendor_embeddings',
|
||||
});
|
||||
});
|
||||
|
||||
it('ignores disabled vendor LLM sub-nodes when partitioning', () => {
|
||||
const nodes = [
|
||||
makeNode({
|
||||
|
|
|
|||
|
|
@ -24,6 +24,8 @@ import { ICredentialsHelper } from 'n8n-workflow';
|
|||
|
||||
import { CredentialNotFoundError } from '@/errors/credential-not-found.error';
|
||||
|
||||
import { isVendorSdkSubNode } from './workflow-analysis';
|
||||
|
||||
const MOCK_MARKER = '__evalMockedCredential' as const;
|
||||
|
||||
// `pathPrefix` must match what the vendor SDK appends to `baseURL`. OpenAI SDK appends
|
||||
|
|
@ -40,6 +42,8 @@ function getCredentialId(nodeCredentials: INodeCredentialsDetails): string | und
|
|||
export class EvalMockedCredentialsHelper extends ICredentialsHelper {
|
||||
readonly mockedCredentials: InstanceAiEvalMockedCredential[] = [];
|
||||
readonly rewrittenCredentials: InstanceAiEvalRewrittenCredential[] = [];
|
||||
/** Vendor sub-node calls this run could not point at the wire server, one per (node, reason). */
|
||||
readonly interceptionGaps: string[] = [];
|
||||
|
||||
constructor(
|
||||
private readonly inner: ICredentialsHelper,
|
||||
|
|
@ -152,17 +156,36 @@ export class EvalMockedCredentialsHelper extends ICredentialsHelper {
|
|||
return this.applyServerUrlRewrite(credentials, type, nodeCredentials, executeData);
|
||||
}
|
||||
|
||||
// Only vendor sub-nodes: every other node's traffic reaches the HTTP mock whatever its credential URL.
|
||||
private recordInterceptionGap(node: INode | undefined, reason: string): void {
|
||||
if (!node || !isVendorSdkSubNode(node.type)) return;
|
||||
const message =
|
||||
`Vendor sub-node "${node.name}" (${node.type}) executed without interception — ${reason}. ` +
|
||||
'Its model/embedding call did not reach the mock layer, so this scenario did not exercise the built workflow.';
|
||||
if (!this.interceptionGaps.includes(message)) this.interceptionGaps.push(message);
|
||||
}
|
||||
|
||||
private applyServerUrlRewrite(
|
||||
credentials: ICredentialDataDecryptedObject,
|
||||
type: string,
|
||||
nodeCredentials: INodeCredentialsDetails,
|
||||
executeData: IExecuteData | undefined,
|
||||
): ICredentialDataDecryptedObject {
|
||||
if (!this.serverUrl) return credentials;
|
||||
if (!this.serverUrl) {
|
||||
this.recordInterceptionGap(
|
||||
executeData?.node,
|
||||
'vendor SDK interception is not active for this run',
|
||||
);
|
||||
return credentials;
|
||||
}
|
||||
const mapping = EVAL_PROVIDER_URL_FIELD[type];
|
||||
if (!mapping) {
|
||||
// No rewrite mapping — vendor SDK will hit its default URL. Refused upfront
|
||||
// by assertUnpinCompatibility for LLM sub-nodes; this branch is for non-LLM HTTP creds.
|
||||
this.recordInterceptionGap(
|
||||
executeData?.node,
|
||||
`credential type "${type}" has no wire-server URL rewrite mapping`,
|
||||
);
|
||||
this.logger.warn(
|
||||
`[EvalMock] No URL rewrite mapping for credential type "${type}" — ` +
|
||||
`vendor traffic from "${executeData?.node?.name ?? 'unknown'}" will hit the real provider.`,
|
||||
|
|
@ -177,6 +200,10 @@ export class EvalMockedCredentialsHelper extends ICredentialsHelper {
|
|||
if (subNodeName && !rootName && this.subNodeToRoot) {
|
||||
// Sub-node not in routing map — unexpected topology; wire server's
|
||||
// unrouted-/v1 handler will surface this loudly too.
|
||||
this.recordInterceptionGap(
|
||||
executeData?.node,
|
||||
'it has no vendor LLM routing entry, so its traffic reaches the wire server unrouted',
|
||||
);
|
||||
this.logger.warn(
|
||||
`[EvalMock] No vendor LLM routing entry for sub-node "${subNodeName}" — ` +
|
||||
'wire-server attribution will be unrouted. Check buildVendorLlmRouting coverage.',
|
||||
|
|
|
|||
|
|
@ -1019,7 +1019,7 @@ export class EvalExecutionService {
|
|||
success: false,
|
||||
nodeResults,
|
||||
errors: [`Execution failed: ${message}`],
|
||||
hints,
|
||||
hints: withInterceptionGaps(hints, credentialsHelper),
|
||||
mockedCredentials: credentialsHelper?.mockedCredentials ?? [],
|
||||
rewrittenCredentials: credentialsHelper?.rewrittenCredentials ?? [],
|
||||
};
|
||||
|
|
@ -1129,7 +1129,7 @@ export class EvalExecutionService {
|
|||
success: allErrors.length === 0,
|
||||
nodeResults,
|
||||
errors: allErrors,
|
||||
hints,
|
||||
hints: withInterceptionGaps(hints, credentialsHelper),
|
||||
mockedCredentials: credentialsHelper?.mockedCredentials ?? [],
|
||||
rewrittenCredentials: credentialsHelper?.rewrittenCredentials ?? [],
|
||||
};
|
||||
|
|
@ -1154,6 +1154,16 @@ export class EvalExecutionService {
|
|||
}
|
||||
}
|
||||
|
||||
/** `warnings` is the channel the verification artifact renders as FRAMEWORK ISSUE flags. */
|
||||
function withInterceptionGaps(
|
||||
hints: MockHints,
|
||||
credentialsHelper: EvalMockedCredentialsHelper | undefined,
|
||||
): MockHints {
|
||||
const gaps = credentialsHelper?.interceptionGaps ?? [];
|
||||
if (gaps.length === 0) return hints;
|
||||
return { ...hints, warnings: [...hints.warnings, ...gaps] };
|
||||
}
|
||||
|
||||
/** Synthesize a structurally valid binary entry (real bytes, base64-inlined). */
|
||||
function synthesizeBinaryEntry(contentType: string, filename: string): IBinaryData {
|
||||
const bytes = synthesizeBinaryFixture(contentType, filename);
|
||||
|
|
|
|||
|
|
@ -61,6 +61,17 @@ function isVendorLlmSubNode(nodeType: string): boolean {
|
|||
return nodeType.startsWith('@n8n/n8n-nodes-langchain.lm');
|
||||
}
|
||||
|
||||
/** `embeddings*` nodes bake the vendor base URL into the SDK, exactly as `lm*` do. */
|
||||
function isVendorEmbeddingsSubNode(nodeType: string): boolean {
|
||||
return nodeType.startsWith('@n8n/n8n-nodes-langchain.embeddings');
|
||||
}
|
||||
|
||||
/** Sub-nodes the HTTP mock never sees: only a credential URL rewrite intercepts them. */
|
||||
export function isVendorSdkSubNode(nodeType: string | undefined): boolean {
|
||||
if (!nodeType) return false;
|
||||
return isVendorLlmSubNode(nodeType) || isVendorEmbeddingsSubNode(nodeType);
|
||||
}
|
||||
|
||||
/** MCP registry nodes talk via the MCP SDK's own transport, not n8n's HTTP helper — the mock can't reach them, so their root must stay pinned. */
|
||||
function isMcpRegistryNode(nodeType: string): boolean {
|
||||
return nodeType.startsWith('@n8n/mcp-registry.');
|
||||
|
|
@ -291,6 +302,7 @@ export function detectBinaryDependencies(
|
|||
export type AutoPinReason =
|
||||
| 'protocol_binary'
|
||||
| 'unsupported_vendor_llm'
|
||||
| 'unsupported_vendor_embeddings'
|
||||
| 'unsafe_baseurl_override'
|
||||
| 'shared_vendor_llm_subnode';
|
||||
|
||||
|
|
@ -516,7 +528,7 @@ function trackSharedSupportedSubNodes(
|
|||
* Return the auto-pin reason for a sub-node, or null if it's safe to intercept.
|
||||
* Order: protocol-binary (HTTP can't reach it) → shared (attribution ambiguous) →
|
||||
* supported-vendor-with-baseURL-override (SDK bypasses the rewrite) → unsupported
|
||||
* vendor LLM (no URL-rewrite mapping yet).
|
||||
* vendor LLM or embeddings (no URL-rewrite mapping yet).
|
||||
*/
|
||||
function categorizeSubNodeIncompatibility(
|
||||
sourceNode: INode,
|
||||
|
|
@ -529,6 +541,12 @@ function categorizeSubNodeIncompatibility(
|
|||
return hasUnsafeBaseUrlOverride(sourceNode) ? 'unsafe_baseurl_override' : null;
|
||||
}
|
||||
if (isVendorLlmSubNode(sourceNode.type)) return 'unsupported_vendor_llm';
|
||||
// No `EVAL_PROVIDER_URL_FIELD` entry rewrites an embeddings credential, so
|
||||
// `applyServerUrlRewrite` hands back the original one and the SDK reaches
|
||||
// the real provider on real credentials. The un-intercepted warning is
|
||||
// raised only after that request, which is too late to stop the spend.
|
||||
// Pin the root until `/v1/embeddings` has a wire-server route.
|
||||
if (isVendorEmbeddingsSubNode(sourceNode.type)) return 'unsupported_vendor_embeddings';
|
||||
return null;
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user