From c4fc0447c04cbdb12ed9b55160bf9de770544436 Mon Sep 17 00:00:00 2001 From: Jaakko Husso Date: Sat, 30 May 2026 09:22:20 +0300 Subject: [PATCH] feat(core): Include offending node JSON in workflow save errors (#31296) --- .../__tests__/submit-workflow.tool.test.ts | 291 +++++++++++++++++- .../tools/workflows/submit-workflow.tool.ts | 141 ++++++++- .../src/__tests__/workflow-helpers.test.ts | 54 ++++ packages/cli/src/workflow-helpers.ts | 43 ++- 4 files changed, 516 insertions(+), 13 deletions(-) diff --git a/packages/@n8n/instance-ai/src/tools/workflows/__tests__/submit-workflow.tool.test.ts b/packages/@n8n/instance-ai/src/tools/workflows/__tests__/submit-workflow.tool.test.ts index 615b2dc0779..5f1a29a7196 100644 --- a/packages/@n8n/instance-ai/src/tools/workflows/__tests__/submit-workflow.tool.test.ts +++ b/packages/@n8n/instance-ai/src/tools/workflows/__tests__/submit-workflow.tool.test.ts @@ -1,12 +1,15 @@ import { validateWorkflow, type WorkflowJSON } from '@n8n/workflow-sdk'; import { mock } from 'jest-mock-extended'; -import type { INodeTypes } from 'n8n-workflow'; +import type { INodeTypes, WorkflowStructureIssue } from 'n8n-workflow'; import { executeTool } from '../../../__tests__/tool-test-utils'; import type { InstanceAiContext } from '../../../types'; import type { SandboxWorkspace } from '../../../workspace/sandbox-fs'; import { + buildErrorDetails, + buildNodeIndex, classifySubmitFailure, + extractStructureIssues, normalizeWorkflowNodeParameters, type SubmitWorkflowAttempt, type SubmitWorkflowOutput, @@ -381,6 +384,292 @@ describe('classifySubmitFailure', () => { }); }); +describe('extractStructureIssues', () => { + it('returns the issues array when the error carries WorkflowStructureIssue[]', () => { + const caused = Object.assign(new Error('boom'), { + issues: [{ path: ['nodes', 0, 'parameters'], code: 'invalid_type', message: 'no good' }], + }); + const result = extractStructureIssues(caused); + expect(result).toHaveLength(1); + expect(result?.[0].path).toEqual(['nodes', 0, 'parameters']); + }); + + it('returns undefined for a plain Error without issues', () => { + expect(extractStructureIssues(new Error('plain'))).toBeUndefined(); + }); + + it('returns undefined when issues is an empty array', () => { + const caused = Object.assign(new Error('x'), { issues: [] }); + expect(extractStructureIssues(caused)).toBeUndefined(); + }); + + it('returns undefined when issue entries have wrong shape', () => { + const caused = Object.assign(new Error('x'), { + issues: [{ path: 'nope', message: 'wrong shape' }], + }); + expect(extractStructureIssues(caused)).toBeUndefined(); + }); + + it('returns undefined for non-objects', () => { + expect(extractStructureIssues(null)).toBeUndefined(); + expect(extractStructureIssues(undefined)).toBeUndefined(); + expect(extractStructureIssues('error')).toBeUndefined(); + }); +}); + +describe('buildNodeIndex', () => { + it('returns one entry per node with stable index ordering', () => { + const json = { + nodes: [ + { name: 'Gmail Trigger', type: 'x', position: [0, 0], parameters: {} }, + { name: 'Route', type: 'y', position: [0, 0], parameters: {} }, + { name: 'Manual Trigger (temp)', type: 'z', position: [0, 0], parameters: {} }, + ], + connections: {}, + } as unknown as WorkflowJSON; + + expect(buildNodeIndex(json)).toEqual([ + { index: 0, name: 'Gmail Trigger' }, + { index: 1, name: 'Route' }, + { index: 2, name: 'Manual Trigger (temp)' }, + ]); + }); + + it('returns an empty array when nodes is missing', () => { + expect(buildNodeIndex({ connections: {} } as unknown as WorkflowJSON)).toEqual([]); + }); +}); + +describe('buildErrorDetails', () => { + const json = { + name: 'Test', + nodes: [ + { + id: 'n-0', + name: 'Gmail Trigger', + type: 'n8n-nodes-base.gmailTrigger', + typeVersion: 1, + position: [0, 0], + parameters: { foo: 'bar' }, + }, + { + id: 'n-1', + name: 'Manual Trigger (temp)', + type: 'n8n-nodes-base.manualTrigger', + typeVersion: 1, + position: [0, 200], + parameters: null, + }, + ], + connections: { + 'Gmail Trigger': { main: [[{ node: 'GhostNode', type: 'main', index: 0 }]] }, + }, + } as unknown as WorkflowJSON; + + it('attaches nodeJson when the path starts with nodes[N]', () => { + const issues: WorkflowStructureIssue[] = [ + { + path: ['nodes', 1, 'parameters'], + code: 'invalid_type', + message: 'Expected object, received null', + } as WorkflowStructureIssue, + ]; + + const [detail] = buildErrorDetails(issues, json); + + expect(detail.path).toBe('nodes[1].parameters'); + expect(detail.code).toBe('invalid_type'); + expect(detail.offendingValue).toBeNull(); + expect(detail.nodeJson).toEqual(json.nodes[1]); + expect(detail.connectionSlice).toBeUndefined(); + }); + + it('attaches connectionSlice when the path starts with connections.', () => { + const issues: WorkflowStructureIssue[] = [ + { + path: ['connections', 'Gmail Trigger', 'main', 0, 0, 'node'], + code: 'unknown_connection_target', + message: 'Connection target "GhostNode" does not reference an existing node', + }, + ]; + + const [detail] = buildErrorDetails(issues, json); + + expect(detail.path).toBe('connections.Gmail Trigger.main[0][0].node'); + expect(detail.offendingValue).toBe('GhostNode'); + expect(detail.connectionSlice).toEqual(json.connections['Gmail Trigger']); + expect(detail.nodeJson).toBeUndefined(); + }); + + it('attaches neither slice for workflow-level paths and keeps offendingValue', () => { + const issues: WorkflowStructureIssue[] = [ + { + path: ['name'], + code: 'invalid_type', + message: 'Required', + } as WorkflowStructureIssue, + ]; + + const [detail] = buildErrorDetails(issues, json); + expect(detail.path).toBe('name'); + expect(detail.code).toBe('invalid_type'); + expect(detail.offendingValue).toBe('Test'); + expect(detail.nodeJson).toBeUndefined(); + expect(detail.connectionSlice).toBeUndefined(); + }); + + it('uses "workflow" as the path label for root-level issues', () => { + const issues: WorkflowStructureIssue[] = [ + { + path: [], + code: 'invalid_type', + message: 'root payload bad', + } as unknown as WorkflowStructureIssue, + ]; + + const [detail] = buildErrorDetails(issues, json); + expect(detail.path).toBe('workflow'); + }); +}); + +describe('createSubmitWorkflowTool — structured save-failure payload', () => { + beforeEach(() => { + mockedValidateWorkflow.mockReset(); + mockedValidateWorkflow.mockReturnValue({ errors: [], warnings: [] } as never); + }); + + it('returns errorDetails and nodeIndex when the save throws a structured error', async () => { + // Mimic WorkflowStructureBadRequestError from packages/cli/src/workflow-helpers.ts: + // a regular Error with an `issues` property. Use `position` (not `parameters`) + // because `normalizeWorkflowNodeParameters` runs before save and would coerce + // a `null` parameters value to `{}` — invalidating any assertion about it. + const saveError = Object.assign( + new Error( + 'Workflow structure is invalid. nodes[1].position (invalid_type): Expected tuple, received null', + ), + { + issues: [ + { + path: ['nodes', 1, 'position'], + code: 'invalid_type', + message: 'Expected tuple, received null', + }, + ], + }, + ); + + const workflowJson = { + name: 'Triage workflow', + nodes: [ + { + id: 'n-0', + name: 'Gmail Trigger', + type: 'n8n-nodes-base.gmailTrigger', + typeVersion: 1, + position: [0, 0], + parameters: {}, + }, + { + id: 'n-1', + name: 'Manual Trigger (temp)', + type: 'n8n-nodes-base.manualTrigger', + typeVersion: 1, + position: null, + parameters: {}, + }, + ], + connections: {}, + }; + + const workflowService = { + createFromWorkflowJSON: jest.fn(async () => { + await Promise.resolve(); + throw saveError; + }), + }; + + const attempts: SubmitWorkflowAttempt[] = []; + const tool = createSubmitWorkflowTool( + makeContext({} as InstanceAiContext['permissions'], { + workflowService: workflowService as unknown as InstanceAiContext['workflowService'], + }), + makeBuildSuccessWorkspace(workflowJson), + new Map(), + (attempt) => { + attempts.push(attempt); + }, + ); + + const output = await executeTool(tool, { + filePath: 'src/workflow.ts', + name: 'Triage workflow', + }); + + expect(output.success).toBe(false); + expect(output.errors?.[0]).toContain('nodes[1].position'); + expect(output.errorDetails).toHaveLength(1); + expect(output.errorDetails?.[0]).toMatchObject({ + path: 'nodes[1].position', + code: 'invalid_type', + message: 'Expected tuple, received null', + offendingValue: null, + }); + expect(output.errorDetails?.[0].nodeJson).toMatchObject({ + name: 'Manual Trigger (temp)', + type: 'n8n-nodes-base.manualTrigger', + }); + expect(output.nodeIndex).toEqual([ + { index: 0, name: 'Gmail Trigger' }, + { index: 1, name: 'Manual Trigger (temp)' }, + ]); + + expect(attempts).toHaveLength(1); + expect(attempts[0].errorDetails?.[0].path).toBe('nodes[1].position'); + expect(attempts[0].nodeIndex).toHaveLength(2); + }); + + it('still emits nodeIndex (but no errorDetails) when the save error is plain', async () => { + const workflowService = { + createFromWorkflowJSON: jest.fn(async () => { + await Promise.resolve(); + throw new Error('database unavailable'); + }), + }; + + const workflowJson = { + name: 'Plain failure', + nodes: [ + { + id: 'n-0', + name: 'A', + type: 'n8n-nodes-base.set', + typeVersion: 1, + position: [0, 0], + parameters: {}, + }, + ], + connections: {}, + }; + + const tool = createSubmitWorkflowTool( + makeContext({} as InstanceAiContext['permissions'], { + workflowService: workflowService as unknown as InstanceAiContext['workflowService'], + }), + makeBuildSuccessWorkspace(workflowJson), + new Map(), + ); + + const output = await executeTool(tool, { + filePath: 'src/workflow.ts', + name: 'Plain failure', + }); + + expect(output.success).toBe(false); + expect(output.errorDetails).toBeUndefined(); + expect(output.nodeIndex).toEqual([{ index: 0, name: 'A' }]); + }); +}); + describe('normalizeWorkflowNodeParameters', () => { it('normalizes missing and null node parameters to empty objects', () => { const workflow = { diff --git a/packages/@n8n/instance-ai/src/tools/workflows/submit-workflow.tool.ts b/packages/@n8n/instance-ai/src/tools/workflows/submit-workflow.tool.ts index d57d2d12425..f5df1af4cdc 100644 --- a/packages/@n8n/instance-ai/src/tools/workflows/submit-workflow.tool.ts +++ b/packages/@n8n/instance-ai/src/tools/workflows/submit-workflow.tool.ts @@ -10,6 +10,7 @@ import { Tool } from '@n8n/agents'; import { hasPlaceholderDeep } from '@n8n/utils'; import type { WorkflowJSON } from '@n8n/workflow-sdk'; import { validateWorkflow } from '@n8n/workflow-sdk'; +import type { WorkflowStructureIssue } from 'n8n-workflow'; import { createHash, randomUUID } from 'node:crypto'; import { z } from 'zod'; @@ -58,6 +59,8 @@ export interface SubmitWorkflowAttempt { hasUnresolvedPlaceholders?: boolean; remediation?: RemediationMetadata; errors?: string[]; + errorDetails?: SubmitWorkflowErrorDetail[]; + nodeIndex?: Array<{ index: number; name: string }>; } function hashContent(content: string | null): string { @@ -86,6 +89,100 @@ export function normalizeWorkflowNodeParameters(json: WorkflowJSON): void { } } +/** + * Recover a WorkflowStructureIssue[] from a caught save error by duck typing. + * + * The CLI adapter throws `WorkflowStructureBadRequestError` (defined in + * packages/cli/src/workflow-helpers.ts) which carries `issues: WorkflowStructureIssue[]`. + * We can't import the class here without taking a dep on `cli`, so we recognise + * the shape: `issues` is a non-empty array whose entries each have an array + * `path` and a string `message`. + */ +export function extractStructureIssues(error: unknown): WorkflowStructureIssue[] | undefined { + if (!error || typeof error !== 'object') return undefined; + const candidate = (error as { issues?: unknown }).issues; + if (!Array.isArray(candidate) || candidate.length === 0) return undefined; + for (const issue of candidate) { + if (!issue || typeof issue !== 'object') return undefined; + const { path, message } = issue as { path?: unknown; message?: unknown }; + if (!Array.isArray(path) || typeof message !== 'string') return undefined; + } + return candidate as WorkflowStructureIssue[]; +} + +/** + * Build the `nodeIndex` map (cheap — ~10-20 tokens per node) so the AI can + * resolve a `nodes[N]` Zod path back to its source-code node by name without + * counting entries in its own SDK-builder code. + */ +export function buildNodeIndex(json: WorkflowJSON): Array<{ index: number; name: string }> { + return (json.nodes ?? []).map((node, index) => ({ index, name: node.name ?? '' })); +} + +/** + * Traverse `root` following the Zod issue `path`. Returns whatever lives at + * the path, or `undefined` if any segment is missing / out of range. + */ +function valueAtPath(root: unknown, path: ReadonlyArray): unknown { + let cursor: unknown = root; + for (const segment of path) { + if (cursor === null || cursor === undefined) return undefined; + if (typeof segment === 'number') { + if (!Array.isArray(cursor)) return undefined; + cursor = cursor[segment]; + } else { + if (typeof cursor !== 'object') return undefined; + cursor = (cursor as Record)[segment]; + } + } + return cursor; +} + +/** + * Format a Zod issue path (mixed string|number segments) as the same bracketed + * string the server's error message uses — e.g. `nodes[9].parameters`. Mirrors + * `formatWorkflowStructureIssuePath` in `n8n-workflow` but kept inline so we + * don't take a runtime dep on it from the tool layer. + */ +function formatIssuePath(path: ReadonlyArray): string { + if (path.length === 0) return 'workflow'; + return path.reduce((acc, segment) => { + if (typeof segment === 'number') return `${acc}[${segment}]`; + return acc ? `${acc}.${segment}` : segment; + }, ''); +} + +/** + * Build the per-issue diagnostic array surfaced as `errorDetails` on save + * failure. For each issue, attach the smallest sufficient slice of `json`: + * - `nodes[N].*` paths → full `nodeJson` so the AI sees the offending node + * - `connections..*` paths → the source's `connectionSlice` + * - everything else → no slice; `offendingValue` and `path` carry the signal + */ +export function buildErrorDetails( + issues: WorkflowStructureIssue[], + json: WorkflowJSON, +): SubmitWorkflowErrorDetail[] { + return issues.map((issue) => { + const path = issue.path; + const detail: SubmitWorkflowErrorDetail = { + path: formatIssuePath(path), + code: issue.code, + message: issue.message, + offendingValue: valueAtPath(json, path), + }; + + const [head, second] = path; + if (head === 'nodes' && typeof second === 'number') { + detail.nodeJson = json.nodes?.[second]; + } else if (head === 'connections' && typeof second === 'string') { + detail.connectionSlice = json.connections?.[second]; + } + + return detail; + }); +} + /** * Ensure webhook nodes have a webhookId so n8n registers clean URL paths. * Without it, getNodeWebhookPath() falls back to encoding the node name @@ -182,6 +279,25 @@ export const submitWorkflowInputSchema = z.object({ name: z.string().optional().describe('Workflow name (required for new workflows)'), }); +/** + * Per-issue diagnostic returned alongside `errors[]` when a save fails with a + * structured workflow-structure validation error. Carries the smallest sufficient + * slice of the submitted JSON so the AI can map a Zod path back to its source code + * without counting nodes manually: + * - `path` / `code` / `message` come from the server's Zod issue + * - `offendingValue` is the value found at `path` inside the submitted JSON + * - `nodeJson` is populated when the path is `nodes[N].*` — the full node object + * - `connectionSlice` is populated when the path is `connections..*` + */ +export const submitWorkflowErrorDetailSchema = z.object({ + path: z.string(), + code: z.string(), + message: z.string(), + offendingValue: z.unknown().optional(), + nodeJson: z.unknown().optional(), + connectionSlice: z.unknown().optional(), +}); + export const submitWorkflowOutputSchema = z.object({ success: z.boolean(), workflowId: z.string().optional(), @@ -209,11 +325,23 @@ export const submitWorkflowOutputSchema = z.object({ }) .optional(), errors: z.array(z.string()).optional(), + /** + * Structured per-issue diagnostics emitted on save failure. One entry per Zod + * issue, ordered to match `errors[]`. + */ + errorDetails: z.array(submitWorkflowErrorDetailSchema).optional(), + /** + * Index-to-name map for the submitted workflow's nodes. Always emitted when + * `json` reached the save step. Lets the AI translate `nodes[N]` paths in + * error messages back to the named node in its source code in one step. + */ + nodeIndex: z.array(z.object({ index: z.number().int().min(0), name: z.string() })).optional(), warnings: z.array(z.string()).optional(), }); export type SubmitWorkflowInput = z.infer; export type SubmitWorkflowOutput = z.infer; +export type SubmitWorkflowErrorDetail = z.infer; export interface SubmitWorkflowToolOptions { root?: string; @@ -512,11 +640,22 @@ export function createSubmitWorkflowTool( `Workflow save failed: ${error instanceof Error ? error.message : 'Unknown error'}`, ]; const remediation = classifySubmitFailure(errors, 'workflow_save_failed'); - await reportAttempt({ success: false, errors, remediation }); + const issues = extractStructureIssues(error); + const errorDetails = issues ? buildErrorDetails(issues, json) : undefined; + const nodeIndex = buildNodeIndex(json); + await reportAttempt({ + success: false, + errors, + remediation, + errorDetails, + nodeIndex, + }); return { success: false, errors, remediation, + errorDetails, + nodeIndex, }; } diff --git a/packages/cli/src/__tests__/workflow-helpers.test.ts b/packages/cli/src/__tests__/workflow-helpers.test.ts index 25670ee97a6..7c3189c4d06 100644 --- a/packages/cli/src/__tests__/workflow-helpers.test.ts +++ b/packages/cli/src/__tests__/workflow-helpers.test.ts @@ -16,7 +16,10 @@ import { shouldRestartParentExecution, validatePinDataSize, validateWorkflowNodeGroups, + validateWorkflowStructure, + WorkflowStructureBadRequestError, } from '@/workflow-helpers'; +import { BadRequestError } from '@/errors/response-errors/bad-request.error'; import { mock } from 'jest-mock-extended'; describe('workflow-helpers', () => { @@ -384,6 +387,57 @@ describe('removeDefaultValues', () => { }); }); +describe('validateWorkflowStructure', () => { + it('returns silently when the workflow is structurally valid', () => { + expect(() => + validateWorkflowStructure({ + nodes: [ + { + id: 'n1', + name: 'Manual', + type: 'n8n-nodes-base.manualTrigger', + position: [0, 0], + parameters: {}, + } as never, + ], + connections: {}, + }), + ).not.toThrow(); + }); + + it('throws WorkflowStructureBadRequestError carrying the Zod issues', () => { + let caught: unknown; + try { + validateWorkflowStructure({ + nodes: [ + { + id: 'n1', + name: 'Bad', + type: 'n8n-nodes-base.manualTrigger', + position: [0, 0], + parameters: null, + } as never, + ], + connections: {}, + }); + } catch (error) { + caught = error; + } + + expect(caught).toBeInstanceOf(WorkflowStructureBadRequestError); + // Subclass of BadRequestError so existing HTTP handlers still see a 400. + expect(caught).toBeInstanceOf(BadRequestError); + const structured = caught as WorkflowStructureBadRequestError; + expect(structured.message).toContain('Workflow structure is invalid.'); + expect(structured.message).toContain('nodes[0].parameters'); + expect(structured.issues.length).toBeGreaterThan(0); + expect(structured.issues[0]).toMatchObject({ + path: ['nodes', 0, 'parameters'], + code: 'invalid_type', + }); + }); +}); + describe('validateWorkflowNodeGroups', () => { const makeNode = (id: string) => ({ id, name: `Node ${id}`, type: 'test', position: [0, 0], parameters: {} }) as never; diff --git a/packages/cli/src/workflow-helpers.ts b/packages/cli/src/workflow-helpers.ts index 8399e6e0f6f..d65b4cfafd4 100644 --- a/packages/cli/src/workflow-helpers.ts +++ b/packages/cli/src/workflow-helpers.ts @@ -2,20 +2,19 @@ import { MAX_PINNED_DATA_SIZE, MAX_WORKFLOW_SIZE, MAX_EXPECTED_REQUEST_SIZE } fr import { CredentialsRepository } from '@n8n/db'; import type { WorkflowEntity, WorkflowHistory, ExecutionRepository } from '@n8n/db'; import { Container } from '@n8n/di'; -import type { - IDataObject, - INodeCredentialsDetails, - INodeTypes, - IRun, - ITaskData, - IWorkflowBase, - IWorkflowSettings, - RelatedExecution, -} from 'n8n-workflow'; import { formatWorkflowStructureIssuePath, resolveNodeWebhookId, safeParseWorkflowStructure, + type IDataObject, + type INodeCredentialsDetails, + type INodeTypes, + type IRun, + type ITaskData, + type IWorkflowBase, + type IWorkflowSettings, + type RelatedExecution, + type WorkflowStructureIssue, } from 'n8n-workflow'; import { v4 as uuid } from 'uuid'; @@ -177,6 +176,25 @@ export function validateWorkflowNodeGroups(workflow: Pick` message + * are unchanged from before this class existed, so REST clients are unaffected. + */ +export class WorkflowStructureBadRequestError extends BadRequestError { + constructor( + message: string, + readonly issues: WorkflowStructureIssue[], + ) { + super(message); + } +} + export function validateWorkflowStructure(workflow: Pick) { const result = safeParseWorkflowStructure(workflow); @@ -192,7 +210,10 @@ export function validateWorkflowStructure(workflow: Pick