feat(core): Include offending node JSON in workflow save errors (#31296)

This commit is contained in:
Jaakko Husso 2026-05-30 09:22:20 +03:00 committed by GitHub
parent e2e0394856
commit c4fc0447c0
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 516 additions and 13 deletions

View File

@ -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.<sourceName>', () => {
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<SubmitWorkflowOutput>(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<SubmitWorkflowOutput>(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 = {

View File

@ -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<string | number>): 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<string, unknown>)[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 | number>): string {
if (path.length === 0) return 'workflow';
return path.reduce<string>((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.<sourceName>.*` 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.<sourceName>.*`
*/
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<typeof submitWorkflowInputSchema>;
export type SubmitWorkflowOutput = z.infer<typeof submitWorkflowOutputSchema>;
export type SubmitWorkflowErrorDetail = z.infer<typeof submitWorkflowErrorDetailSchema>;
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,
};
}

View File

@ -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;

View File

@ -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<IWorkflowBase, 'nodes'
}
}
/**
* BadRequestError thrown by validateWorkflowStructure when a workflow fails
* structural Zod / graph validation. Carries the original WorkflowStructureIssue[]
* so downstream consumers (e.g. the Instance AI submit-workflow tool) can build
* rich diagnostics node JSON at the offending path, value at the path, and a
* full nodes[] name map without reparsing the flattened message string.
*
* The status code (400) and `Workflow structure is invalid. <details>` 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<IWorkflowBase, 'nodes' | 'connections'>) {
const result = safeParseWorkflowStructure(workflow);
@ -192,7 +210,10 @@ export function validateWorkflowStructure(workflow: Pick<IWorkflowBase, 'nodes'
})
.join('; ');
throw new BadRequestError(`Workflow structure is invalid. ${details}`);
throw new WorkflowStructureBadRequestError(
`Workflow structure is invalid. ${details}`,
result.issues,
);
}
/**