diff --git a/packages/@n8n/instance-ai/evaluations/system-prompts/mock-execution-verify.ts b/packages/@n8n/instance-ai/evaluations/system-prompts/mock-execution-verify.ts index 18de16ece02..84485f93d25 100644 --- a/packages/@n8n/instance-ai/evaluations/system-prompts/mock-execution-verify.ts +++ b/packages/@n8n/instance-ai/evaluations/system-prompts/mock-execution-verify.ts @@ -73,7 +73,7 @@ Each branch's items are capped at 10 for artifact size. The full untruncated tot When a checklist item fails, categorize the root cause: - **builder_issue**: The AI agent that built the workflow misconfigured a node (missing parameters, wrong settings, incomplete config, wrong routing logic, missing nodes). Evidence: configIssues flags, nodes crashing before making HTTP requests, Switch/IF nodes missing required options, workflow structure doesn't match what the prompt asked for. Also applies when the configured identifier could never resolve against any faithful API response (wrong-kind resource-locator value, e.g. "Sheet with ID Reservas not found" — see the lookup bullet in step 3). Also applies when a node's HTTP request was recorded WITHOUT a URL (an intercepted request showing "GET (no URL)" — "GET undefined" in older captures — or a 400 error stating "the node sent an HTTP request without a URL"): the node's routing produced an empty request because the selected resource/operation does not exist on that node type — the workflow could never work against the real API either. This is node misconfiguration, NOT a mock or framework problem. Also applies when a Code node receives correct input data but its connected downstream branch produces wrong output — that's wrong node logic. **For Filter / IF / Switch: an item appearing in the unmatched branch is NOT wrong output — it's correctly routed there by the predicate. Only flag a builder_issue when items that should have matched the predicate end up in the wrong branch.** When judging whether items matched, evaluate against the scenario's stated data values — if the mock's values contradict the scenario and that is why the predicate routed unexpectedly, it is a mock_issue, not a builder_issue. **Also applies when the builder produced an empty or trivial workflow (0 nodes, or only a trigger and no action nodes) — even if the build phase appears to have completed.** A "No trigger or start node found" execution error caused by zero nodes in the saved workflow is a builder failure, not a framework failure: the builder is responsible for committing at least a trigger. Also applies when an Execute Workflow node crashes BEFORE invoking its sub-workflow because its own input mapping is invalid — \`workflowInputs.mappingMode: "defineBelow"\` with \`workflowInputs.value: null\` produces "Cannot convert undefined or null to object" at iteration 0. That saved config could never work against real n8n either. Do NOT rationalize this crash as the sub-workflow "not existing in the test environment" — the referenced sub-workflow was created by the same build; the crash happens in the parent's mapping code before any lookup. -- **mock_issue**: The LLM mock handler returned incorrect or missing data. Evidence: _evalMockError in responses, mock response shape doesn't match what the node expects, mock data missing fields that downstream nodes reference. IMPORTANT: Trace the data flow carefully — if the mock returned correct data but a downstream filter or code node transformed it incorrectly, that is a builder_issue, not a mock_issue. Also applies when the mock returned values that contradict the scenario's stated Data setup (wrong magnitude, percentage, or item count), causing a correctly-built gate, filter, or aggregation to produce an unexpected result. +- **mock_issue**: The LLM mock handler returned incorrect or missing data. Evidence: _evalMockError in responses, mock response shape doesn't match what the node expects, mock data missing fields that downstream nodes reference. IMPORTANT: Trace the data flow carefully — if the mock returned correct data but a downstream filter or code node transformed it incorrectly, that is a builder_issue, not a mock_issue. Also applies when the mock returned values that contradict the scenario's stated Data setup (wrong magnitude, percentage, or item count), causing a correctly-built gate, filter, or aggregation to produce an unexpected result. Also applies to PINNED node fixtures — nodes tagged [pinned] in the execution trace received harness-GENERATED output (fixture generation is part of the mock harness): pinned data wrong for the scenario — most commonly a phantom single empty item \`{}\` where the scenario requires ZERO items, or values contradicting the Data setup — is a mock_issue, never a framework_issue and never a builder_issue. - **framework_issue**: The evaluation framework itself failed delivering input to an otherwise-built workflow. Evidence: a built workflow with at least a trigger node exists, but Phase 1 returned an error or the trigger output is empty (empty JSON object), causing cascading failures. Pre-analysis flags starting with "FRAMEWORK ISSUE", "Phase 1 error" warnings. Also applies when the test environment cannot express the scenario's premise: the scenario presumes state left behind by earlier production runs (e.g. \`$getWorkflowStaticData\` holding a "previous" value — eval executions always start with empty static data), or the execution suspended at a node that models the passage of real time or an external callback the harness cannot deliver. Categorize these framework_issue consistently — not builder_issue — regardless of which scenario branch the empty state routed to. DOES NOT apply when the workflow is empty (0 nodes) — that is a builder_issue, see above. A workflow that runs as built but doesn't meet the success criteria is a builder_issue — the builder owns satisfying the scenario as written; there is no separate "legitimate failure" category. - **verification_gap**: You don't have enough information in the artifact to make a determination. diff --git a/packages/@n8n/workflow-sdk/src/mock-data/mock-data.test.ts b/packages/@n8n/workflow-sdk/src/mock-data/mock-data.test.ts index 80bbc9cbf2f..497cc0a4a9d 100644 --- a/packages/@n8n/workflow-sdk/src/mock-data/mock-data.test.ts +++ b/packages/@n8n/workflow-sdk/src/mock-data/mock-data.test.ts @@ -172,6 +172,26 @@ describe('parsePinDataResponse', () => { expect(parsePinDataResponse('sorry', ['Get Rows'])).toEqual({}); expect(parsePinDataResponse(JSON.stringify({ Other: [{}] }), ['Get Rows'])).toEqual({}); }); + + it('drops phantom empty items so a zero-item premise pins as []', () => { + expect(parsePinDataResponse(JSON.stringify({ 'Get Rows': [{}] }), ['Get Rows'])).toEqual({ + 'Get Rows': [], + }); + expect( + parsePinDataResponse(JSON.stringify({ 'Get Rows': [{ json: {} }] }), ['Get Rows']), + ).toEqual({ 'Get Rows': [] }); + }); + + it('keeps real items when dropping empty ones, and items with non-json payloads', () => { + expect( + parsePinDataResponse(JSON.stringify({ 'Get Rows': [{}, { id: 1 }] }), ['Get Rows']), + ).toEqual({ 'Get Rows': [{ json: { id: 1 } }] }); + expect( + parsePinDataResponse(JSON.stringify({ 'Get Rows': [{ json: {}, binary: { data: {} } }] }), [ + 'Get Rows', + ]), + ).toEqual({ 'Get Rows': [{ json: {}, binary: { data: {} } }] }); + }); }); describe('repairStructuredOutput', () => { diff --git a/packages/@n8n/workflow-sdk/src/mock-data/parse.ts b/packages/@n8n/workflow-sdk/src/mock-data/parse.ts index 56ba8e20e71..3ec554e699d 100644 --- a/packages/@n8n/workflow-sdk/src/mock-data/parse.ts +++ b/packages/@n8n/workflow-sdk/src/mock-data/parse.ts @@ -27,20 +27,33 @@ export function parsePinDataResponse(responseText: string, expectedNodes: string // Keep empty arrays — a valid "no stored data" pin; dropping one falls back to real execution. if (!Array.isArray(nodeData)) continue; - pinData[nodeName] = nodeData.map((item: unknown) => { - // The execution engine expects { json: IDataObject } format. - // The LLM may return items with or without the json wrapper. - if (typeof item === 'object' && item !== null && 'json' in item) { - return item as Record; - } - // Wrap raw objects in { json: ... } for the execution engine - return { json: item ?? {} }; - }); + pinData[nodeName] = nodeData + .map((item: unknown) => { + // The execution engine expects { json: IDataObject } format. + // The LLM may return items with or without the json wrapper. + if (typeof item === 'object' && item !== null && 'json' in item) { + return item as Record; + } + // Wrap raw objects in { json: ... } for the execution engine + return { json: item ?? {} }; + }) + // A fieldless item is never a meaningful fixture — it's the model's way + // of saying "no data" while obeying "generate items" (a zero-item premise + // must pin as [], not [{}]: the phantom item corrupts downstream nodes). + .filter((item) => !isEmptyFixtureItem(item)); } return pinData; } +/** True for `{}` / `{ json: {} }` items with no other payload (e.g. binary). */ +function isEmptyFixtureItem(item: Record): boolean { + const json = item.json; + if (typeof json !== 'object' || json === null) return false; + if (Object.keys(json).length > 0) return false; + return !Object.keys(item).some((key) => key !== 'json' && key !== 'pairedItem'); +} + /** Parse a string as a JSON object/array; undefined when it isn't one. */ function tryParseJsonContainer(text: string): Record | unknown[] | undefined { const trimmed = text.trim(); diff --git a/packages/@n8n/workflow-sdk/src/mock-data/prompt.ts b/packages/@n8n/workflow-sdk/src/mock-data/prompt.ts index cc0031767bc..a8286a78713 100644 --- a/packages/@n8n/workflow-sdk/src/mock-data/prompt.ts +++ b/packages/@n8n/workflow-sdk/src/mock-data/prompt.ts @@ -8,7 +8,7 @@ export const PIN_DATA_SYSTEM_PROMPT = `You are a test data generator for n8n wor RULES: 1. Data must be consistent across nodes. If node A creates an entity with id "abc-123", downstream nodes referencing that entity must use "abc-123". When a node's "Direct downstream consumers" are listed, emit EXACTLY the field names their parameters/expressions/code read (e.g. a Code node reading item.json.last_details requires a field named "last_details") — never rename or synonymize them. -2. Generate 1-2 items per node. +2. Generate 1-2 items per node — UNLESS the Test Scenario calls for zero items for a node (no stored data, empty list, nothing new, all already seen): then that node's value MUST be an empty array []. NEVER represent "no data" as a single item with no fields ({}): a phantom empty item flows downstream and corrupts the run; the empty array IS the correct representation. 3. When a JSON Schema is provided, follow its structure exactly. 4. When no schema is provided, generate a realistic response based on the node type, resource, and operation. 5. Use realistic but clearly fake values (e.g., "jane@example.com", "proj_abc123"). @@ -16,7 +16,7 @@ RULES: 7. AI root nodes (Agent/Chain) have NODE-TYPE-SPECIFIC output shapes — follow each node's schema or "AI ROOT OUTPUT SHAPE" instruction exactly and never invent a different envelope key. Summary: agent wraps in { "output": ... } (a parsed object matching the parser schema when one is attached, otherwise a plain text string — never a JSON-encoded string); chainLlm uses { "text": "" } without a parser, or { "output": } like agent when a parser is attached; chainRetrievalQa uses { "response": "" }; chainSummarization uses { "output": { "output_text": "" } }; informationExtractor wraps the extracted fields in { "output": }; textClassifier passes the input item through unchanged; sentimentAnalysis is the input item plus a "sentimentAnalysis" object. 8. CRITICAL: If a "Test Scenario" section is provided, it is the authoritative test state and OVERRIDES everything else, including the general data context and your own sense of realism. When it describes stored/previous records (e.g. "the store already holds a record with status X"), exact values, counts, literal substrings, or that stored data matches current data, reproduce those constraints EXACTLY — never substitute more "interesting" or more typical data. A boring no-change/empty/matching case is usually the point of the test. 9. Return ONLY a valid JSON object, no explanation or markdown fencing. -10. CRITICAL: You MUST generate data for EVERY node listed in "Nodes Requiring Mock Data". Never skip a node, even if the test scenario describes an empty or error response. An empty response is still valid data.`; +10. CRITICAL: You MUST generate data for EVERY node listed in "Nodes Requiring Mock Data". Never skip a node, even if the test scenario describes an empty or error response. An empty response is still valid data — represent it as an empty array [], never as [{}].`; const MAX_EMBEDDED_SCHEMA_CHARS = 3000; diff --git a/packages/cli/src/modules/instance-ai/eval/__tests__/execution.service.test.ts b/packages/cli/src/modules/instance-ai/eval/__tests__/execution.service.test.ts index 23dce50273a..0ba7a3f2213 100644 --- a/packages/cli/src/modules/instance-ai/eval/__tests__/execution.service.test.ts +++ b/packages/cli/src/modules/instance-ai/eval/__tests__/execution.service.test.ts @@ -462,6 +462,29 @@ describe('EvalExecutionService', () => { expect(workflowStaticDataService.saveStaticDataById).toHaveBeenCalledWith('wf-1', {}); }); + it('preserves an intentional zero-item bypass pin instead of injecting a phantom item', async () => { + const bypassNode = { + id: 'node-3', + name: 'Only New Jobs', + type: 'n8n-nodes-base.dataTable', + typeVersion: 1, + position: [400, 0], + parameters: {}, + } as INode; + workflowFinderService.findWorkflowForUser.mockResolvedValue( + makeWorkflowEntity({ nodes: [makeStartNode(), bypassNode] }) as never, + ); + identifyNodesForPinDataMock.mockReturnValue([bypassNode]); + generatePinDataMock.mockResolvedValue({ 'Only New Jobs': [] }); + + await service.executeWithLlmMock('wf-1', makeUser()); + + const runArg = workflowRunner.run.mock.calls[0][0] as unknown as { + pinData?: Record; + }; + expect(runArg.pinData?.['Only New Jobs']).toEqual([]); + }); + it('returns a framework failure when bypass pin data generation fails', async () => { const bypassNode = { id: 'node-3', diff --git a/packages/cli/src/modules/instance-ai/eval/execution.service.ts b/packages/cli/src/modules/instance-ai/eval/execution.service.ts index a5dbf78f4ce..054b0c244f5 100644 --- a/packages/cli/src/modules/instance-ai/eval/execution.service.ts +++ b/packages/cli/src/modules/instance-ai/eval/execution.service.ts @@ -303,19 +303,18 @@ export class EvalExecutionService { const normalized = normalizePinData(result as unknown as IPinData); - // generatePinData swallows internal failures (LLM timeout, parse error) - // and returns {} or a partial map instead of throwing, so the catch - // fallback below never fires for those. An unpinned bypass node - // EXECUTES for real — for AI roots the vendor SDK then makes real - // network calls (observed in CI: un-mocked Anthropic request → - // "Authorization failed"). Guarantee every bypass node is pinned, - // even if only with an empty item. + // A MISSING bypass entry would let the node execute for real — for AI + // roots the vendor SDK then makes real network calls (observed in CI: + // un-mocked Anthropic request → "Authorization failed"). An EMPTY array + // is different: the execution engine honors it as "pinned, zero items" + // (presence check, not length), and zero-item scenario premises depend + // on it — never replace [] with a phantom item (TRUST-343). for (const nodeName of bypassNodeNames) { - if (!normalized[nodeName] || normalized[nodeName].length === 0) { + if (!normalized[nodeName]) { this.logger.warn( - `[EvalMock] Phase 1.5 produced no pin data for bypass node "${nodeName}" — pinning an empty item to prevent real execution`, + `[EvalMock] Phase 1.5 produced no pin data for bypass node "${nodeName}" — pinning empty to prevent real execution`, ); - normalized[nodeName] = [{ json: {} }]; + normalized[nodeName] = []; } }