mirror of
https://github.com/n8n-io/n8n.git
synced 2026-10-17 05:29:44 +02:00
fix(core): Block publishing workflows with unconnected required node inputs (#37265)
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
b0803dc450
commit
5e28ff8c23
|
|
@ -7,8 +7,14 @@
|
|||
*/
|
||||
|
||||
import type { Logger } from '@n8n/backend-common';
|
||||
import { parseWorkflowCodeToBuilder, validateWorkflow, workflow } from '@n8n/workflow-sdk';
|
||||
import type { WorkflowJSON } from '@n8n/workflow-sdk';
|
||||
import {
|
||||
connectRequiredSubnodeInputs,
|
||||
describeAddedSubnodeConnection,
|
||||
parseWorkflowCodeToBuilder,
|
||||
validateWorkflow,
|
||||
workflow,
|
||||
} from '@n8n/workflow-sdk';
|
||||
import type { AddedSubnodeConnection, WorkflowJSON } from '@n8n/workflow-sdk';
|
||||
import type { INodeTypes } from 'n8n-workflow';
|
||||
|
||||
import type { ParseAndValidateResult, ValidationWarning } from '../types';
|
||||
|
|
@ -67,6 +73,16 @@ export class ParseValidateHandler {
|
|||
this.nodeTypesProvider = config.nodeTypesProvider;
|
||||
}
|
||||
|
||||
/**
|
||||
* Complete required AI subnode connections the generated code left unwired,
|
||||
* e.g. an output parser with `autoFix: true` and no model. Needs node types,
|
||||
* so a handler built without a provider leaves the workflow untouched.
|
||||
*/
|
||||
private connectRequiredInputs(json: WorkflowJSON): AddedSubnodeConnection[] {
|
||||
if (!this.nodeTypesProvider) return [];
|
||||
return connectRequiredSubnodeInputs(json, this.nodeTypesProvider);
|
||||
}
|
||||
|
||||
/**
|
||||
* Options for the graph validation pass.
|
||||
*
|
||||
|
|
@ -252,6 +268,7 @@ export class ParseValidateHandler {
|
|||
|
||||
// Convert to JSON for JSON-based validation
|
||||
const json = builder.toJSON();
|
||||
this.connectRequiredInputs(json);
|
||||
|
||||
// Run JSON-based validation for additional checks
|
||||
const validationResult = validateWorkflow(json, {
|
||||
|
|
@ -292,6 +309,10 @@ export class ParseValidateHandler {
|
|||
|
||||
// Convert to JSON with Dagre layout matching the FE's tidy-up
|
||||
const workflowJson: WorkflowJSON = builder.toJSON({ tidyUp: true, existingGroupIdsByName });
|
||||
// A fresh serialization, so the links added above have to be re-added.
|
||||
for (const link of this.connectRequiredInputs(workflowJson)) {
|
||||
allWarnings.push(describeAddedSubnodeConnection(link));
|
||||
}
|
||||
|
||||
this.logger?.debug('Parsed workflow', {
|
||||
id: workflowJson.id,
|
||||
|
|
|
|||
|
|
@ -3,7 +3,13 @@
|
|||
*/
|
||||
|
||||
import type { WorkflowJSON } from '@n8n/workflow-sdk';
|
||||
import { parseWorkflowCodeToBuilder, validateWorkflow, workflow } from '@n8n/workflow-sdk';
|
||||
import {
|
||||
connectRequiredSubnodeInputs,
|
||||
describeAddedSubnodeConnection,
|
||||
parseWorkflowCodeToBuilder,
|
||||
validateWorkflow,
|
||||
workflow,
|
||||
} from '@n8n/workflow-sdk';
|
||||
import type { INodeTypes } from 'n8n-workflow';
|
||||
import type { Mock } from 'vitest';
|
||||
|
||||
|
|
@ -15,12 +21,16 @@ vi.mock('@n8n/workflow-sdk', () => ({
|
|||
validateWorkflow: vi.fn(),
|
||||
workflow: { fromJSON: vi.fn() },
|
||||
stripImportStatements: vi.fn((code: string) => code),
|
||||
connectRequiredSubnodeInputs: vi.fn(() => []),
|
||||
describeAddedSubnodeConnection: vi.fn(),
|
||||
}));
|
||||
|
||||
// Typed mock references
|
||||
const mockParseWorkflowCodeToBuilder = parseWorkflowCodeToBuilder as Mock;
|
||||
const mockValidateWorkflow = validateWorkflow as Mock;
|
||||
const mockFromJSON = workflow.fromJSON as Mock;
|
||||
const mockConnectRequiredSubnodeInputs = connectRequiredSubnodeInputs as Mock;
|
||||
const mockDescribeAddedSubnodeConnection = describeAddedSubnodeConnection as Mock;
|
||||
|
||||
describe('ParseValidateHandler', () => {
|
||||
let handler: ParseValidateHandler;
|
||||
|
|
@ -58,6 +68,63 @@ describe('ParseValidateHandler', () => {
|
|||
expect(mockBuilder.validate).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('completes required subnode inputs on the returned workflow and reports them', async () => {
|
||||
// The helper itself is covered in required-subnode-connections.test.ts.
|
||||
// What is untested here is the handler around it: that it runs against
|
||||
// the serialized workflow with the provider, and surfaces what it added.
|
||||
const nodeTypesProvider = { getByNameAndVersion: vi.fn() } as unknown as INodeTypes;
|
||||
const mockWorkflow = {
|
||||
id: 'test',
|
||||
name: 'Test',
|
||||
nodes: [{ id: 'parser', name: 'Parser', type: 'parser' }],
|
||||
connections: {},
|
||||
};
|
||||
const mockBuilder = {
|
||||
regenerateNodeIds: vi.fn(),
|
||||
validate: vi.fn().mockReturnValue({ valid: true, errors: [], warnings: [] }),
|
||||
generatePinData: vi.fn(),
|
||||
toJSON: vi.fn().mockReturnValue(mockWorkflow),
|
||||
};
|
||||
mockParseWorkflowCodeToBuilder.mockReturnValue(mockBuilder);
|
||||
mockValidateWorkflow.mockReturnValue({ valid: true, errors: [], warnings: [] });
|
||||
mockConnectRequiredSubnodeInputs.mockReturnValue([
|
||||
{ nodeName: 'Parser', connectionType: 'ai_languageModel', sourceNode: 'Model' },
|
||||
]);
|
||||
mockDescribeAddedSubnodeConnection.mockReturnValue({
|
||||
code: 'REQUIRED_SUBNODE_CONNECTED',
|
||||
message: 'Connected Model to Parser',
|
||||
severity: 'info',
|
||||
});
|
||||
|
||||
const result = await new ParseValidateHandler({ nodeTypesProvider }).parseAndValidate(
|
||||
'const workflow = {}',
|
||||
);
|
||||
|
||||
expect(mockConnectRequiredSubnodeInputs).toHaveBeenCalledWith(
|
||||
mockWorkflow,
|
||||
nodeTypesProvider,
|
||||
);
|
||||
expect(result.workflow).toBe(mockWorkflow);
|
||||
expect(result.warnings.map((warning) => warning.code)).toContain(
|
||||
'REQUIRED_SUBNODE_CONNECTED',
|
||||
);
|
||||
});
|
||||
|
||||
it('leaves the workflow alone when no node-type provider is configured', async () => {
|
||||
const mockBuilder = {
|
||||
regenerateNodeIds: vi.fn(),
|
||||
validate: vi.fn().mockReturnValue({ valid: true, errors: [], warnings: [] }),
|
||||
generatePinData: vi.fn(),
|
||||
toJSON: vi.fn().mockReturnValue({ id: 'test', name: 'Test', nodes: [], connections: {} }),
|
||||
};
|
||||
mockParseWorkflowCodeToBuilder.mockReturnValue(mockBuilder);
|
||||
mockValidateWorkflow.mockReturnValue({ valid: true, errors: [], warnings: [] });
|
||||
|
||||
await handler.parseAndValidate('const workflow = {}');
|
||||
|
||||
expect(mockConnectRequiredSubnodeInputs).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('skips structural checks in JSON validation that the graph pass already covers', async () => {
|
||||
const mockBuilder = {
|
||||
regenerateNodeIds: vi.fn(),
|
||||
|
|
|
|||
|
|
@ -252,6 +252,8 @@ describe('emit-instance-ai', () => {
|
|||
'hasProperty',
|
||||
'isPlainObject',
|
||||
// Validation
|
||||
'connectRequiredSubnodeInputs',
|
||||
'describeAddedSubnodeConnection',
|
||||
'containsExpression',
|
||||
'isSensitiveHeader',
|
||||
'isCredentialFieldName',
|
||||
|
|
|
|||
|
|
@ -176,6 +176,11 @@ export {
|
|||
type ValidateWorkflowBuilderOptions,
|
||||
type ValidateWorkflowBuilderResult,
|
||||
type CollectedValidationIssue,
|
||||
connectRequiredSubnodeInputs,
|
||||
describeAddedSubnodeConnection,
|
||||
type AddedSubnodeConnection,
|
||||
type ClearedSubnodeInput,
|
||||
type WorkflowForSubnodeWiring,
|
||||
} from './validation';
|
||||
|
||||
// Code-node source lint — the host re-runs the Python rules with the executing
|
||||
|
|
|
|||
|
|
@ -16,6 +16,11 @@ export {
|
|||
type ValidateWorkflowBuilderOptions,
|
||||
type ValidateWorkflowBuilderResult,
|
||||
type CollectedValidationIssue,
|
||||
connectRequiredSubnodeInputs,
|
||||
describeAddedSubnodeConnection,
|
||||
type AddedSubnodeConnection,
|
||||
type ClearedSubnodeInput,
|
||||
type WorkflowForSubnodeWiring,
|
||||
} from './validation/index';
|
||||
|
||||
export {
|
||||
|
|
|
|||
|
|
@ -43,5 +43,13 @@ export {
|
|||
type DisplayOptionsContext,
|
||||
} from './display-options';
|
||||
|
||||
export {
|
||||
connectRequiredSubnodeInputs,
|
||||
describeAddedSubnodeConnection,
|
||||
type AddedSubnodeConnection,
|
||||
type ClearedSubnodeInput,
|
||||
type WorkflowForSubnodeWiring,
|
||||
} from './required-subnode-connections';
|
||||
|
||||
export { resolveMainInputCount } from './node-port-resolvers/resolve-main-input-count';
|
||||
export { resolveMainOutputCount } from './node-port-resolvers/resolve-main-output-count';
|
||||
|
|
|
|||
|
|
@ -0,0 +1,251 @@
|
|||
import type { INodeTypes } from 'n8n-workflow';
|
||||
import { describe, expect, it } from 'vitest';
|
||||
|
||||
import {
|
||||
connectRequiredSubnodeInputs,
|
||||
type WorkflowForSubnodeWiring,
|
||||
} from './required-subnode-connections';
|
||||
|
||||
const AGENT = '@n8n/n8n-nodes-langchain.agent';
|
||||
const PARSER = '@n8n/n8n-nodes-langchain.outputParserStructured';
|
||||
const LOADER = '@n8n/n8n-nodes-langchain.documentDefaultDataLoader';
|
||||
const MODEL = '@n8n/n8n-nodes-langchain.lmChatOpenAi';
|
||||
const AUTOFIX = '@n8n/n8n-nodes-langchain.outputParserAutofixing';
|
||||
|
||||
const BUILDER_HINTS: Record<string, unknown> = {
|
||||
[AGENT]: {
|
||||
ai_languageModel: { required: true },
|
||||
ai_outputParser: { required: false, displayOptions: { show: { hasOutputParser: [true] } } },
|
||||
},
|
||||
[PARSER]: {
|
||||
ai_languageModel: { required: true, displayOptions: { show: { autoFix: [true] } } },
|
||||
},
|
||||
[AUTOFIX]: {
|
||||
ai_languageModel: { required: true },
|
||||
},
|
||||
[LOADER]: {
|
||||
ai_textSplitter: {
|
||||
required: true,
|
||||
displayOptions: { show: { textSplittingMode: ['custom'] } },
|
||||
},
|
||||
},
|
||||
};
|
||||
|
||||
const nodeTypes = {
|
||||
getByNameAndVersion: (type: string) => {
|
||||
const inputs = BUILDER_HINTS[type];
|
||||
if (!inputs) return undefined;
|
||||
return { description: { builderHint: { inputs } } };
|
||||
},
|
||||
} as unknown as INodeTypes;
|
||||
|
||||
/** Agent with a model and a parser whose capability needs a model of its own. */
|
||||
function agentWithParser(
|
||||
parserParameters: Record<string, unknown>,
|
||||
options: { models?: string[] } = {},
|
||||
): WorkflowForSubnodeWiring {
|
||||
const models = options.models ?? ['OpenAI Chat Model'];
|
||||
const connections: WorkflowForSubnodeWiring['connections'] = {
|
||||
'Output Parser': {
|
||||
ai_outputParser: [[{ node: 'Feature Release Agent', type: 'ai_outputParser', index: 0 }]],
|
||||
},
|
||||
};
|
||||
for (const model of models) {
|
||||
connections[model] = {
|
||||
ai_languageModel: [[{ node: 'Feature Release Agent', type: 'ai_languageModel', index: 0 }]],
|
||||
};
|
||||
}
|
||||
|
||||
return {
|
||||
nodes: [
|
||||
{
|
||||
name: 'Feature Release Agent',
|
||||
type: AGENT,
|
||||
typeVersion: 3.1,
|
||||
parameters: { hasOutputParser: true },
|
||||
},
|
||||
{ name: 'Output Parser', type: PARSER, typeVersion: 1.3, parameters: parserParameters },
|
||||
...models.map((name) => ({ name, type: MODEL, typeVersion: 1.3, parameters: {} })),
|
||||
],
|
||||
connections,
|
||||
};
|
||||
}
|
||||
|
||||
describe('connectRequiredSubnodeInputs', () => {
|
||||
it("wires the agent's model into an autoFix parser that has none", () => {
|
||||
const workflow = agentWithParser({ schemaType: 'fromJson', autoFix: true });
|
||||
|
||||
const result = connectRequiredSubnodeInputs(workflow, nodeTypes);
|
||||
|
||||
expect(result).toEqual([
|
||||
{
|
||||
sourceNode: 'OpenAI Chat Model',
|
||||
targetNode: 'Output Parser',
|
||||
connectionType: 'ai_languageModel',
|
||||
viaParent: 'Feature Release Agent',
|
||||
},
|
||||
]);
|
||||
expect(workflow.connections['OpenAI Chat Model']?.ai_languageModel?.[0]).toEqual([
|
||||
{ node: 'Feature Release Agent', type: 'ai_languageModel', index: 0 },
|
||||
{ node: 'Output Parser', type: 'ai_languageModel', index: 0 },
|
||||
]);
|
||||
});
|
||||
|
||||
it('leaves a parser alone when autoFix is off, since it needs no model', () => {
|
||||
const workflow = agentWithParser({ schemaType: 'fromJson' });
|
||||
|
||||
const result = connectRequiredSubnodeInputs(workflow, nodeTypes);
|
||||
|
||||
expect(result).toEqual([]);
|
||||
expect(workflow.connections['OpenAI Chat Model']?.ai_languageModel?.[0]).toHaveLength(1);
|
||||
});
|
||||
|
||||
it('leaves an already-wired required input alone', () => {
|
||||
const workflow = agentWithParser({ autoFix: true });
|
||||
workflow.connections['OpenAI Chat Model']!.ai_languageModel![0]!.push({
|
||||
node: 'Output Parser',
|
||||
type: 'ai_languageModel',
|
||||
index: 0,
|
||||
});
|
||||
|
||||
const result = connectRequiredSubnodeInputs(workflow, nodeTypes);
|
||||
|
||||
expect(result).toEqual([]);
|
||||
});
|
||||
|
||||
it('does not repair an input the caller just cleared', () => {
|
||||
const workflow = agentWithParser({ autoFix: true });
|
||||
|
||||
const result = connectRequiredSubnodeInputs(workflow, nodeTypes, {
|
||||
clearedInputs: [{ nodeName: 'Output Parser', connectionType: 'ai_languageModel' }],
|
||||
});
|
||||
|
||||
expect(result).toEqual([]);
|
||||
expect(workflow.connections['OpenAI Chat Model']?.ai_languageModel?.[0]).toHaveLength(1);
|
||||
});
|
||||
|
||||
it('still repairs inputs the caller did not clear', () => {
|
||||
const workflow = agentWithParser({ autoFix: true });
|
||||
|
||||
const result = connectRequiredSubnodeInputs(workflow, nodeTypes, {
|
||||
clearedInputs: [{ nodeName: 'Some Other Node', connectionType: 'ai_languageModel' }],
|
||||
});
|
||||
|
||||
expect(result).toHaveLength(1);
|
||||
});
|
||||
|
||||
it('wires nothing when the parent has no model to take', () => {
|
||||
const workflow = agentWithParser({ autoFix: true }, { models: [] });
|
||||
|
||||
expect(connectRequiredSubnodeInputs(workflow, nodeTypes)).toEqual([]);
|
||||
});
|
||||
|
||||
it('wires nothing when the parent has two models, rather than picking one', () => {
|
||||
const workflow = agentWithParser({ autoFix: true }, { models: ['Primary', 'Fallback'] });
|
||||
|
||||
expect(connectRequiredSubnodeInputs(workflow, nodeTypes)).toEqual([]);
|
||||
expect(workflow.connections.Primary?.ai_languageModel?.[0]).toHaveLength(1);
|
||||
expect(workflow.connections.Fallback?.ai_languageModel?.[0]).toHaveLength(1);
|
||||
});
|
||||
|
||||
it('does not treat an ungated required input as missing', () => {
|
||||
// The agent's own ai_languageModel is ungated and already connected.
|
||||
const workflow = agentWithParser({ autoFix: true });
|
||||
|
||||
const result = connectRequiredSubnodeInputs(workflow, nodeTypes);
|
||||
|
||||
expect(result.map((a) => a.targetNode)).toEqual(['Output Parser']);
|
||||
});
|
||||
|
||||
it('handles a non-boolean gate on a different connection type', () => {
|
||||
const workflow: WorkflowForSubnodeWiring = {
|
||||
nodes: [
|
||||
{ name: 'Agent', type: AGENT, typeVersion: 3.1, parameters: {} },
|
||||
{
|
||||
name: 'Loader',
|
||||
type: LOADER,
|
||||
typeVersion: 1,
|
||||
parameters: { textSplittingMode: 'custom' },
|
||||
},
|
||||
{ name: 'Splitter', type: 'ts', typeVersion: 1, parameters: {} },
|
||||
],
|
||||
connections: {
|
||||
Loader: { ai_document: [[{ node: 'Agent', type: 'ai_document', index: 0 }]] },
|
||||
Splitter: { ai_textSplitter: [[{ node: 'Agent', type: 'ai_textSplitter', index: 0 }]] },
|
||||
},
|
||||
};
|
||||
|
||||
const result = connectRequiredSubnodeInputs(workflow, nodeTypes);
|
||||
|
||||
expect(result).toEqual([
|
||||
{
|
||||
sourceNode: 'Splitter',
|
||||
targetNode: 'Loader',
|
||||
connectionType: 'ai_textSplitter',
|
||||
viaParent: 'Agent',
|
||||
},
|
||||
]);
|
||||
});
|
||||
|
||||
it('ignores node types it cannot resolve', () => {
|
||||
const workflow: WorkflowForSubnodeWiring = {
|
||||
nodes: [{ name: 'Mystery', type: 'community.unknown', typeVersion: 1, parameters: {} }],
|
||||
connections: {},
|
||||
};
|
||||
|
||||
expect(connectRequiredSubnodeInputs(workflow, nodeTypes)).toEqual([]);
|
||||
});
|
||||
|
||||
it('survives a provider that throws on an unknown version', () => {
|
||||
const throwingNodeTypes = {
|
||||
getByNameAndVersion: () => {
|
||||
throw new Error('Node version not found');
|
||||
},
|
||||
} as unknown as INodeTypes;
|
||||
|
||||
const workflow = agentWithParser({ autoFix: true });
|
||||
|
||||
expect(connectRequiredSubnodeInputs(workflow, throwingNodeTypes)).toEqual([]);
|
||||
});
|
||||
|
||||
// An autofixing parser both consumes and produces `ai_outputParser`, so two
|
||||
// of them chain. Repairing the outer one is what makes the inner one's source
|
||||
// findable, and `workflow.nodes` carries no ordering guarantee.
|
||||
describe.each([
|
||||
['outer parser first', ['Autofix Outer', 'Autofix Inner']],
|
||||
['inner parser first', ['Autofix Inner', 'Autofix Outer']],
|
||||
])('with a repair that depends on another repair (%s)', (_label, order) => {
|
||||
it('wires both, whatever order the nodes arrive in', () => {
|
||||
const workflow: WorkflowForSubnodeWiring = {
|
||||
nodes: [
|
||||
{ name: 'Agent', type: AGENT, typeVersion: 3.1, parameters: {} },
|
||||
{ name: 'OpenAI Chat Model', type: MODEL, typeVersion: 1.3, parameters: {} },
|
||||
...order.map((name) => ({ name, type: AUTOFIX, typeVersion: 1, parameters: {} })),
|
||||
],
|
||||
connections: {
|
||||
'OpenAI Chat Model': {
|
||||
ai_languageModel: [[{ node: 'Agent', type: 'ai_languageModel', index: 0 }]],
|
||||
},
|
||||
'Autofix Outer': {
|
||||
ai_outputParser: [[{ node: 'Agent', type: 'ai_outputParser', index: 0 }]],
|
||||
},
|
||||
'Autofix Inner': {
|
||||
ai_outputParser: [[{ node: 'Autofix Outer', type: 'ai_outputParser', index: 0 }]],
|
||||
},
|
||||
},
|
||||
};
|
||||
|
||||
const added = connectRequiredSubnodeInputs(workflow, nodeTypes);
|
||||
|
||||
expect(added.map((link) => link.targetNode).sort()).toEqual([
|
||||
'Autofix Inner',
|
||||
'Autofix Outer',
|
||||
]);
|
||||
// The inner one takes its model from the parent that was just repaired.
|
||||
expect(added.find((link) => link.targetNode === 'Autofix Inner')).toMatchObject({
|
||||
sourceNode: 'OpenAI Chat Model',
|
||||
viaParent: 'Autofix Outer',
|
||||
});
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
@ -0,0 +1,228 @@
|
|||
/**
|
||||
* Completes AI subnode connections that a node's own parameters made required.
|
||||
*
|
||||
* Turning on a capability can add a required input one level below the node the
|
||||
* author was thinking about (`autoFix` on an output parser needs a model), and
|
||||
* generated code routinely sets the parameter without wiring it.
|
||||
*/
|
||||
|
||||
import type { INodeTypes } from 'n8n-workflow';
|
||||
|
||||
import { matchesDisplayOptions } from './display-options';
|
||||
import type { DisplayOptions, DisplayOptionsContext } from './display-options';
|
||||
|
||||
/**
|
||||
* Structural rather than `WorkflowJSON` so both callers fit: the SDK and
|
||||
* `n8n-workflow` declare separate node and connection types.
|
||||
*/
|
||||
export interface WorkflowForSubnodeWiring {
|
||||
nodes: Array<{
|
||||
name?: string;
|
||||
type: string;
|
||||
typeVersion?: number | string;
|
||||
parameters?: Record<string, unknown>;
|
||||
}>;
|
||||
connections: {
|
||||
[sourceNode: string]:
|
||||
| {
|
||||
[connectionType: string]:
|
||||
| Array<Array<{ node: string; type?: string; index?: number }> | null>
|
||||
| undefined;
|
||||
}
|
||||
| undefined;
|
||||
};
|
||||
}
|
||||
|
||||
/** A connection this pass added. */
|
||||
export interface AddedSubnodeConnection {
|
||||
sourceNode: string;
|
||||
targetNode: string;
|
||||
connectionType: string;
|
||||
/** Parent both nodes hang off, which is where the source came from. */
|
||||
viaParent: string;
|
||||
}
|
||||
|
||||
/** Kept next to the producer so both callers share one wording. */
|
||||
export function describeAddedSubnodeConnection(link: AddedSubnodeConnection): {
|
||||
code: string;
|
||||
message: string;
|
||||
nodeName: string;
|
||||
} {
|
||||
return {
|
||||
code: 'REQUIRED_SUBNODE_CONNECTED',
|
||||
message: `Connected '${link.sourceNode}' to the ${link.connectionType} input of '${link.targetNode}', which its own parameters made required. The source was taken from '${link.viaParent}'. Wire this explicitly in future edits.`,
|
||||
nodeName: link.targetNode,
|
||||
};
|
||||
}
|
||||
|
||||
/** An input the caller cleared on purpose, which must not be repaired. */
|
||||
export interface ClearedSubnodeInput {
|
||||
nodeName: string;
|
||||
connectionType: string;
|
||||
}
|
||||
|
||||
/** node -> connection type -> source nodes feeding it. */
|
||||
type IncomingIndex = Map<string, Map<string, Set<string>>>;
|
||||
|
||||
function buildIncomingIndex(workflow: WorkflowForSubnodeWiring): IncomingIndex {
|
||||
const incoming: IncomingIndex = new Map();
|
||||
|
||||
for (const [sourceNode, byType] of Object.entries(workflow.connections ?? {})) {
|
||||
for (const [connectionType, outputs] of Object.entries(byType ?? {})) {
|
||||
if (!Array.isArray(outputs)) continue;
|
||||
for (const targets of outputs) {
|
||||
for (const target of targets ?? []) {
|
||||
if (!target?.node) continue;
|
||||
const type = target.type ?? connectionType;
|
||||
const byTargetType = incoming.get(target.node) ?? new Map<string, Set<string>>();
|
||||
const sources = byTargetType.get(type) ?? new Set<string>();
|
||||
sources.add(sourceNode);
|
||||
byTargetType.set(type, sources);
|
||||
incoming.set(target.node, byTargetType);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return incoming;
|
||||
}
|
||||
|
||||
/** Every node this subnode feeds over an `ai_*` connection. */
|
||||
function findParents(workflow: WorkflowForSubnodeWiring, nodeName: string): string[] {
|
||||
const parents = new Set<string>();
|
||||
const byType = workflow.connections?.[nodeName];
|
||||
|
||||
for (const [connectionType, outputs] of Object.entries(byType ?? {})) {
|
||||
if (!connectionType.startsWith('ai_') || !Array.isArray(outputs)) continue;
|
||||
for (const targets of outputs) {
|
||||
for (const target of targets ?? []) {
|
||||
if (target?.node) parents.add(target.node);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return [...parents];
|
||||
}
|
||||
|
||||
function addConnection(
|
||||
workflow: WorkflowForSubnodeWiring,
|
||||
incoming: IncomingIndex,
|
||||
sourceNode: string,
|
||||
targetNode: string,
|
||||
connectionType: string,
|
||||
): void {
|
||||
const byType = (workflow.connections[sourceNode] ??= {});
|
||||
const outputs = (byType[connectionType] ??= []);
|
||||
outputs[0] = [...(outputs[0] ?? []), { node: targetNode, type: connectionType, index: 0 }];
|
||||
|
||||
// Keep the index in step with the graph. A repair one level down reads the
|
||||
// sources of its own parent, and that parent may have been repaired earlier
|
||||
// in this same pass, so a stale index would lose the chain.
|
||||
const byTargetType = incoming.get(targetNode) ?? new Map<string, Set<string>>();
|
||||
const sources = byTargetType.get(connectionType) ?? new Set<string>();
|
||||
sources.add(sourceNode);
|
||||
byTargetType.set(connectionType, sources);
|
||||
incoming.set(targetNode, byTargetType);
|
||||
}
|
||||
|
||||
/**
|
||||
* Mutates `workflow.connections` and returns the links added. Anything it cannot
|
||||
* satisfy is left for `validateWorkflow` to report as `MISSING_REQUIRED_INPUT`.
|
||||
*
|
||||
* `clearedInputs` are inputs the caller just disconnected; repairing one would
|
||||
* undo their own removal.
|
||||
*/
|
||||
export function connectRequiredSubnodeInputs(
|
||||
workflow: WorkflowForSubnodeWiring,
|
||||
nodeTypesProvider: INodeTypes,
|
||||
options: { clearedInputs?: readonly ClearedSubnodeInput[] } = {},
|
||||
): AddedSubnodeConnection[] {
|
||||
const cleared = new Set(
|
||||
(options.clearedInputs ?? []).map((input) => `${input.nodeName}\u0000${input.connectionType}`),
|
||||
);
|
||||
const added: AddedSubnodeConnection[] = [];
|
||||
const incoming = buildIncomingIndex(workflow);
|
||||
|
||||
function onePass(): number {
|
||||
let count = 0;
|
||||
|
||||
for (const node of workflow.nodes) {
|
||||
if (!node.name) continue;
|
||||
|
||||
// validate-workflow.ts does the same: the wire can carry a string.
|
||||
const version =
|
||||
typeof node.typeVersion === 'string'
|
||||
? parseFloat(node.typeVersion)
|
||||
: (node.typeVersion ?? 1);
|
||||
|
||||
let builderHintInputs;
|
||||
try {
|
||||
builderHintInputs = nodeTypesProvider.getByNameAndVersion(node.type, version)?.description
|
||||
?.builderHint?.inputs;
|
||||
} catch {
|
||||
continue; // unknown type or version
|
||||
}
|
||||
if (!builderHintInputs) continue;
|
||||
|
||||
const parameters = node.parameters ?? {};
|
||||
|
||||
for (const [connectionType, inputConfig] of Object.entries(builderHintInputs)) {
|
||||
if (!connectionType.startsWith('ai_')) continue;
|
||||
if (!inputConfig?.required) continue;
|
||||
if (cleared.has(`${node.name}\u0000${connectionType}`)) continue;
|
||||
|
||||
// A gated input does not exist until its condition holds.
|
||||
const displayOptions = inputConfig.displayOptions as DisplayOptions | undefined;
|
||||
if (displayOptions) {
|
||||
const context: DisplayOptionsContext = {
|
||||
parameters,
|
||||
nodeVersion: version,
|
||||
rootParameters: parameters,
|
||||
};
|
||||
if (!matchesDisplayOptions(context, displayOptions)) continue;
|
||||
}
|
||||
|
||||
// buildIncomingIndex only stores non-empty sets.
|
||||
if (incoming.get(node.name)?.get(connectionType)) continue;
|
||||
|
||||
// Take the source from the parent the subnode hangs off, not the whole graph.
|
||||
const parentBySource = new Map<string, string>();
|
||||
for (const parent of findParents(workflow, node.name)) {
|
||||
for (const source of incoming.get(parent)?.get(connectionType) ?? []) {
|
||||
if (source !== node.name) parentBySource.set(source, parent);
|
||||
}
|
||||
}
|
||||
|
||||
// Wire only when the source is unambiguous.
|
||||
const candidates = [...parentBySource.keys()];
|
||||
if (candidates.length !== 1) continue;
|
||||
|
||||
const [sourceNode] = candidates;
|
||||
addConnection(workflow, incoming, sourceNode, node.name, connectionType);
|
||||
added.push({
|
||||
sourceNode,
|
||||
targetNode: node.name,
|
||||
connectionType,
|
||||
viaParent: parentBySource.get(sourceNode) as string,
|
||||
});
|
||||
count++;
|
||||
}
|
||||
}
|
||||
|
||||
return count;
|
||||
}
|
||||
|
||||
// One repair can supply the parent another repair reads from, and
|
||||
// `workflow.nodes` is in no particular order, so a single pass would make the
|
||||
// result depend on that order. Sweep until a pass changes nothing.
|
||||
//
|
||||
// A pass only fills an input that had no source, and `addConnection` records
|
||||
// it, so each input is filled at most once and each chain needs at most one
|
||||
// pass per link. The cap is belt and braces: it holds even if a later edit
|
||||
// stops the index from being updated, which would otherwise spin here.
|
||||
for (let pass = 0; pass <= workflow.nodes.length; pass++) {
|
||||
if (onePass() === 0) break;
|
||||
}
|
||||
|
||||
return added;
|
||||
}
|
||||
|
|
@ -187,7 +187,7 @@ describe('BreakingChangeMigrationService', () => {
|
|||
const workflow = buildWorkflow([aiNode], { versionId: 'v1', activeVersionId: 'v1' });
|
||||
workflowFinderService.findWorkflowForUser.mockResolvedValue(workflow);
|
||||
workflowService.update.mockResolvedValue(mock<WorkflowEntity>({ versionId: 'new-version' }));
|
||||
workflowValidationService.validateForActivation.mockReturnValue({ isValid: true });
|
||||
workflowValidationService.validateForActivation.mockResolvedValue({ isValid: true });
|
||||
|
||||
const result = await service.migrateWorkflow(RULE_ID, 'wf-1', user);
|
||||
|
||||
|
|
@ -230,7 +230,7 @@ describe('BreakingChangeMigrationService', () => {
|
|||
const workflow = buildWorkflow([aiNode], { versionId: 'v2', activeVersionId: 'v1' });
|
||||
workflowFinderService.findWorkflowForUser.mockResolvedValue(workflow);
|
||||
workflowService.update.mockResolvedValue(mock<WorkflowEntity>({ versionId: 'new-version' }));
|
||||
workflowValidationService.validateForActivation.mockReturnValue({ isValid: true });
|
||||
workflowValidationService.validateForActivation.mockResolvedValue({ isValid: true });
|
||||
|
||||
const result = await service.migrateWorkflow(RULE_ID, 'wf-1', user);
|
||||
|
||||
|
|
@ -245,7 +245,7 @@ describe('BreakingChangeMigrationService', () => {
|
|||
workflowFinderService.findWorkflowForUser.mockResolvedValue(workflow);
|
||||
workflowService.update.mockResolvedValue(mock<WorkflowEntity>({ versionId: 'new-version' }));
|
||||
// e.g. only a manual trigger, so activation validation fails.
|
||||
workflowValidationService.validateForActivation.mockReturnValue({
|
||||
workflowValidationService.validateForActivation.mockResolvedValue({
|
||||
isValid: false,
|
||||
error: 'no trigger',
|
||||
});
|
||||
|
|
@ -285,7 +285,7 @@ describe('BreakingChangeMigrationService', () => {
|
|||
workflowService.update.mockResolvedValue(
|
||||
mock<WorkflowEntity>({ versionId: 'new-version' }),
|
||||
);
|
||||
workflowValidationService.validateForActivation.mockReturnValue({ isValid: true });
|
||||
workflowValidationService.validateForActivation.mockResolvedValue({ isValid: true });
|
||||
|
||||
const result = await service.migrateWorkflow(EACH_RULE_ID, 'wf-1', user);
|
||||
|
||||
|
|
|
|||
|
|
@ -94,10 +94,12 @@ export class BreakingChangeMigrationService {
|
|||
unmapped.length === 0 &&
|
||||
notes.length === 0 &&
|
||||
wasPublishedVersion &&
|
||||
this.workflowValidationService.validateForActivation(
|
||||
Object.fromEntries(nodes.map((node) => [node.name, node])),
|
||||
connections,
|
||||
this.nodeTypes,
|
||||
(
|
||||
await this.workflowValidationService.validateForActivation(
|
||||
Object.fromEntries(nodes.map((node) => [node.name, node])),
|
||||
connections,
|
||||
this.nodeTypes,
|
||||
)
|
||||
).isValid;
|
||||
|
||||
// Checksum of the workflow as fetched, so a concurrent edit landing between
|
||||
|
|
|
|||
|
|
@ -3114,6 +3114,296 @@ describe('update-workflow MCP tool', () => {
|
|||
expect(response.validationWarnings).toEqual([]);
|
||||
});
|
||||
|
||||
test('does not re-add a required subnode link that this batch removed', async () => {
|
||||
nodeTypes.getByNameAndVersion.mockImplementation(((type: string) => {
|
||||
if (type === '@n8n/n8n-nodes-langchain.outputParserStructured') {
|
||||
return {
|
||||
description: {
|
||||
group: ['transform'],
|
||||
builderHint: {
|
||||
inputs: {
|
||||
ai_languageModel: {
|
||||
required: true,
|
||||
displayOptions: { show: { autoFix: [true] } },
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
};
|
||||
}
|
||||
return { description: { group: ['transform'] } };
|
||||
}) as unknown as typeof nodeTypes.getByNameAndVersion);
|
||||
|
||||
findWorkflowMock.mockResolvedValue(
|
||||
Object.assign(new WorkflowEntity(), {
|
||||
id: 'wf-1',
|
||||
name: 'Existing',
|
||||
settings: { availableInMCP: true },
|
||||
nodes: [
|
||||
makeNode({
|
||||
id: 'agent',
|
||||
name: 'AI Agent',
|
||||
type: '@n8n/n8n-nodes-langchain.agent',
|
||||
typeVersion: 3.1,
|
||||
}),
|
||||
makeNode({
|
||||
id: 'parser',
|
||||
name: 'Parser',
|
||||
type: '@n8n/n8n-nodes-langchain.outputParserStructured',
|
||||
typeVersion: 1.3,
|
||||
parameters: { autoFix: true },
|
||||
}),
|
||||
makeNode({
|
||||
id: 'model',
|
||||
name: 'Model',
|
||||
type: '@n8n/n8n-nodes-langchain.lmChatOpenAi',
|
||||
typeVersion: 1.3,
|
||||
}),
|
||||
],
|
||||
connections: {
|
||||
Parser: {
|
||||
ai_outputParser: [[{ node: 'AI Agent', type: 'ai_outputParser', index: 0 }]],
|
||||
},
|
||||
Model: {
|
||||
ai_languageModel: [
|
||||
[
|
||||
{ node: 'AI Agent', type: 'ai_languageModel', index: 0 },
|
||||
{ node: 'Parser', type: 'ai_languageModel', index: 0 },
|
||||
],
|
||||
],
|
||||
},
|
||||
} as IConnections,
|
||||
}),
|
||||
);
|
||||
|
||||
const result = await callHandler({
|
||||
workflowId: 'wf-1',
|
||||
operations: [
|
||||
{
|
||||
type: 'removeConnection',
|
||||
source: 'Model',
|
||||
target: 'Parser',
|
||||
connectionType: 'ai_languageModel',
|
||||
},
|
||||
],
|
||||
});
|
||||
|
||||
expect(result.isError).toBeUndefined();
|
||||
|
||||
const saved = updateMock.mock.calls[0][1] as WorkflowEntity;
|
||||
const targets = saved.connections.Model.ai_languageModel?.[0]?.map((c) => c.node);
|
||||
expect(targets).toEqual(['AI Agent']);
|
||||
});
|
||||
|
||||
test('keeps the removal when the batch also renames the target node', async () => {
|
||||
nodeTypes.getByNameAndVersion.mockImplementation(((type: string) => {
|
||||
if (type === '@n8n/n8n-nodes-langchain.outputParserStructured') {
|
||||
return {
|
||||
description: {
|
||||
group: ['transform'],
|
||||
builderHint: {
|
||||
inputs: {
|
||||
ai_languageModel: {
|
||||
required: true,
|
||||
displayOptions: { show: { autoFix: [true] } },
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
};
|
||||
}
|
||||
return { description: { group: ['transform'] } };
|
||||
}) as unknown as typeof nodeTypes.getByNameAndVersion);
|
||||
|
||||
findWorkflowMock.mockResolvedValue(
|
||||
Object.assign(new WorkflowEntity(), {
|
||||
id: 'wf-1',
|
||||
name: 'Existing',
|
||||
settings: { availableInMCP: true },
|
||||
nodes: [
|
||||
makeNode({
|
||||
id: 'agent',
|
||||
name: 'AI Agent',
|
||||
type: '@n8n/n8n-nodes-langchain.agent',
|
||||
typeVersion: 3.1,
|
||||
}),
|
||||
makeNode({
|
||||
id: 'parser',
|
||||
name: 'Parser',
|
||||
type: '@n8n/n8n-nodes-langchain.outputParserStructured',
|
||||
typeVersion: 1.3,
|
||||
parameters: { autoFix: true },
|
||||
}),
|
||||
makeNode({
|
||||
id: 'model',
|
||||
name: 'Model',
|
||||
type: '@n8n/n8n-nodes-langchain.lmChatOpenAi',
|
||||
typeVersion: 1.3,
|
||||
}),
|
||||
],
|
||||
connections: {
|
||||
Parser: {
|
||||
ai_outputParser: [[{ node: 'AI Agent', type: 'ai_outputParser', index: 0 }]],
|
||||
},
|
||||
Model: {
|
||||
ai_languageModel: [
|
||||
[
|
||||
{ node: 'AI Agent', type: 'ai_languageModel', index: 0 },
|
||||
{ node: 'Parser', type: 'ai_languageModel', index: 0 },
|
||||
],
|
||||
],
|
||||
},
|
||||
} as IConnections,
|
||||
}),
|
||||
);
|
||||
|
||||
const result = await callHandler({
|
||||
workflowId: 'wf-1',
|
||||
operations: [
|
||||
{
|
||||
type: 'removeConnection',
|
||||
source: 'Model',
|
||||
target: 'Parser',
|
||||
connectionType: 'ai_languageModel',
|
||||
},
|
||||
{ type: 'renameNode', oldName: 'Parser', newName: 'Schema' },
|
||||
],
|
||||
});
|
||||
|
||||
expect(result.isError).toBeUndefined();
|
||||
|
||||
const saved = updateMock.mock.calls[0][1] as WorkflowEntity;
|
||||
const targets = saved.connections.Model.ai_languageModel?.[0]?.map((c) => c.node);
|
||||
expect(targets).toEqual(['AI Agent']);
|
||||
});
|
||||
|
||||
/**
|
||||
* A parser whose required model input is unsatisfied, with the only model
|
||||
* wired to the agent instead. A repair pass therefore has an unambiguous
|
||||
* source to borrow, so any auto-wiring shows up in the saved graph.
|
||||
*/
|
||||
const seedParserMissingItsModel = () => {
|
||||
nodeTypes.getByNameAndVersion.mockImplementation(((type: string) => {
|
||||
if (type === '@n8n/n8n-nodes-langchain.outputParserStructured') {
|
||||
return {
|
||||
description: {
|
||||
group: ['transform'],
|
||||
builderHint: {
|
||||
inputs: {
|
||||
ai_languageModel: {
|
||||
required: true,
|
||||
displayOptions: { show: { autoFix: [true] } },
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
};
|
||||
}
|
||||
return { description: { group: ['transform'] } };
|
||||
}) as unknown as typeof nodeTypes.getByNameAndVersion);
|
||||
|
||||
findWorkflowMock.mockResolvedValue(
|
||||
Object.assign(new WorkflowEntity(), {
|
||||
id: 'wf-1',
|
||||
name: 'Existing',
|
||||
settings: { availableInMCP: true },
|
||||
nodes: [
|
||||
makeNode({
|
||||
id: 'agent',
|
||||
name: 'AI Agent',
|
||||
type: '@n8n/n8n-nodes-langchain.agent',
|
||||
typeVersion: 3.1,
|
||||
}),
|
||||
makeNode({
|
||||
id: 'parser',
|
||||
name: 'Parser',
|
||||
type: '@n8n/n8n-nodes-langchain.outputParserStructured',
|
||||
typeVersion: 1.3,
|
||||
parameters: { autoFix: true },
|
||||
}),
|
||||
makeNode({
|
||||
id: 'model',
|
||||
name: 'Model',
|
||||
type: '@n8n/n8n-nodes-langchain.lmChatOpenAi',
|
||||
typeVersion: 1.3,
|
||||
}),
|
||||
],
|
||||
connections: {
|
||||
Parser: {
|
||||
ai_outputParser: [[{ node: 'AI Agent', type: 'ai_outputParser', index: 0 }]],
|
||||
},
|
||||
Model: {
|
||||
ai_languageModel: [[{ node: 'AI Agent', type: 'ai_languageModel', index: 0 }]],
|
||||
},
|
||||
} as IConnections,
|
||||
}),
|
||||
);
|
||||
};
|
||||
|
||||
test('does not auto-wire anything for a layout-only update', async () => {
|
||||
seedParserMissingItsModel();
|
||||
const result = await callHandler({
|
||||
workflowId: 'wf-1',
|
||||
operations: [{ type: 'setNodePosition', nodeName: 'Parser', position: [10, 20] }],
|
||||
});
|
||||
|
||||
expect(result.isError).toBeUndefined();
|
||||
|
||||
const response = parseResult(result);
|
||||
const warnings = (response.validationWarnings ?? []) as Array<{ code: string }>;
|
||||
expect(warnings.some((w) => w.code === 'REQUIRED_SUBNODE_CONNECTED')).toBe(false);
|
||||
|
||||
// The warning is derived from the links the repair added, so assert the
|
||||
// graph too: dropping the warning must not be enough to pass this.
|
||||
const saved = updateMock.mock.calls[0][1] as WorkflowEntity;
|
||||
const targets = saved.connections.Model.ai_languageModel?.[0]?.map((c) => c.node);
|
||||
expect(targets).toEqual(['AI Agent']);
|
||||
});
|
||||
|
||||
test('does not auto-wire anything for a settings-only update', async () => {
|
||||
seedParserMissingItsModel();
|
||||
const result = await callHandler({
|
||||
workflowId: 'wf-1',
|
||||
operations: [
|
||||
{ type: 'setNodeSettings', nodeName: 'Parser', settings: { retryOnFail: true } },
|
||||
],
|
||||
});
|
||||
|
||||
expect(result.isError).toBeUndefined();
|
||||
|
||||
const response = parseResult(result);
|
||||
const warnings = (response.validationWarnings ?? []) as Array<{ code: string }>;
|
||||
expect(warnings.some((w) => w.code === 'REQUIRED_SUBNODE_CONNECTED')).toBe(false);
|
||||
|
||||
// The model stays wired to the agent alone; the repair pass must not
|
||||
// have borrowed it for the parser's required input.
|
||||
const saved = updateMock.mock.calls[0][1] as WorkflowEntity;
|
||||
const targets = saved.connections.Model.ai_languageModel?.[0]?.map((c) => c.node);
|
||||
expect(targets).toEqual(['AI Agent']);
|
||||
});
|
||||
|
||||
test('does not auto-wire anything for a workflow-level-only update', async () => {
|
||||
seedParserMissingItsModel();
|
||||
|
||||
const result = await callHandler({
|
||||
workflowId: 'wf-1',
|
||||
operations: [
|
||||
{ type: 'setWorkflowMetadata', description: 'Renamed by an agent' },
|
||||
{ type: 'setWorkflowSettings', settings: { executionOrder: 'v1' } },
|
||||
],
|
||||
});
|
||||
|
||||
expect(result.isError).toBeUndefined();
|
||||
|
||||
const response = parseResult(result);
|
||||
const warnings = (response.validationWarnings ?? []) as Array<{ code: string }>;
|
||||
expect(warnings.some((w) => w.code === 'REQUIRED_SUBNODE_CONNECTED')).toBe(false);
|
||||
|
||||
const saved = updateMock.mock.calls[0][1] as WorkflowEntity;
|
||||
const targets = saved.connections.Model.ai_languageModel?.[0]?.map((c) => c.node);
|
||||
expect(targets).toEqual(['AI Agent']);
|
||||
});
|
||||
|
||||
test('refuses to save when an addConnection wires an agent as a tool to another agent', async () => {
|
||||
findWorkflowMock.mockResolvedValue(
|
||||
Object.assign(new WorkflowEntity(), {
|
||||
|
|
|
|||
|
|
@ -3,8 +3,13 @@ import type { Logger } from '@n8n/backend-common';
|
|||
import type { GlobalConfig } from '@n8n/config';
|
||||
import { type User, type SharedWorkflowRepository, WorkflowEntity } from '@n8n/db';
|
||||
import { hasGlobalScope } from '@n8n/permissions';
|
||||
import {
|
||||
connectRequiredSubnodeInputs,
|
||||
describeAddedSubnodeConnection,
|
||||
type ClearedSubnodeInput,
|
||||
} from '@n8n/workflow-sdk';
|
||||
import isEqual from 'lodash/isEqual';
|
||||
import type { IWorkflowSettings } from 'n8n-workflow';
|
||||
import { NodeConnectionTypes, type IWorkflowSettings } from 'n8n-workflow';
|
||||
import { z } from 'zod';
|
||||
|
||||
import type { CollaborationService } from '@/collaboration/collaboration.service';
|
||||
|
|
@ -729,6 +734,26 @@ const isTagOperation = (op: PartialUpdateOperation) =>
|
|||
|
||||
const isSettingsOperation = (op: PartialUpdateOperation) => op.type === 'setWorkflowSettings';
|
||||
|
||||
/**
|
||||
* Operations that cannot leave a required subnode input unsatisfied: they touch
|
||||
* neither connections nor the parameters a conditional `inputs` expression reads.
|
||||
* Node-level execution settings qualify, since `inputs` only sees `$parameter`.
|
||||
*/
|
||||
const GRAPH_NEUTRAL_OPERATIONS = new Set<PartialUpdateOperation['type']>([
|
||||
'addTags',
|
||||
'removeTags',
|
||||
'setNodeSettings',
|
||||
'setWorkflowSettings',
|
||||
'setWorkflowMetadata',
|
||||
'addNodeGroup',
|
||||
'removeNodeGroup',
|
||||
'updateNodeGroup',
|
||||
'setNodeGroups',
|
||||
'setNodePosition',
|
||||
]);
|
||||
|
||||
const touchesGraph = (op: PartialUpdateOperation) => !GRAPH_NEUTRAL_OPERATIONS.has(op.type);
|
||||
|
||||
/**
|
||||
* Rejects operations this instance cannot serve, before anything is loaded or
|
||||
* applied.
|
||||
|
|
@ -1160,6 +1185,29 @@ export const createUpdateWorkflowTool = (
|
|||
throw new Error(result.error);
|
||||
}
|
||||
|
||||
// Setting a parameter can make a subnode input required without wiring it.
|
||||
// Skip inputs this batch disconnected, or removeConnection is a no-op.
|
||||
// Names are carried through later renames, since the pass runs against
|
||||
// the final graph.
|
||||
const clearedInputs: ClearedSubnodeInput[] = [];
|
||||
for (const op of strictOperations) {
|
||||
if (op.type === 'removeConnection') {
|
||||
clearedInputs.push({
|
||||
nodeName: op.target,
|
||||
connectionType: op.connectionType ?? NodeConnectionTypes.Main,
|
||||
});
|
||||
} else if (op.type === 'renameNode') {
|
||||
for (const cleared of clearedInputs) {
|
||||
if (cleared.nodeName === op.oldName) cleared.nodeName = op.newName;
|
||||
}
|
||||
}
|
||||
}
|
||||
// A tag, settings or layout edit cannot create an unsatisfied input, and
|
||||
// wiring one off the back of such an update would be a surprise.
|
||||
const addedSubnodeLinks = strictOperations.some(touchesGraph)
|
||||
? connectRequiredSubnodeInputs(result.workflow, nodeTypes, { clearedInputs })
|
||||
: [];
|
||||
|
||||
const { skippedOperations, removedGroups, nodeGroupsNeedPersisting } =
|
||||
resolveNodeGroupViolations(result, nodeTypes);
|
||||
|
||||
|
|
@ -1248,6 +1296,10 @@ export const createUpdateWorkflowTool = (
|
|||
nodeTypes,
|
||||
);
|
||||
|
||||
for (const link of addedSubnodeLinks) {
|
||||
validationWarnings.push(describeAddedSubnodeConnection(link));
|
||||
}
|
||||
|
||||
validationWarnings.push(
|
||||
...(await buildUninstalledNodeWarnings(
|
||||
workflowUpdateData.nodes.filter((node) => result.addedNodeNames.includes(node.name)),
|
||||
|
|
|
|||
|
|
@ -54,7 +54,7 @@ export const REVIEW_TABLES = [
|
|||
export function stubWorkflowValidation(
|
||||
workflowValidationService: MockProxy<WorkflowValidationService>,
|
||||
): void {
|
||||
workflowValidationService.validateForActivation.mockReturnValue({ isValid: true });
|
||||
workflowValidationService.validateForActivation.mockResolvedValue({ isValid: true });
|
||||
workflowValidationService.validateDynamicCredentials.mockResolvedValue({ isValid: true });
|
||||
workflowValidationService.validatePublisherCredentialAccess.mockResolvedValue({ isValid: true });
|
||||
workflowValidationService.validateSubWorkflowReferences.mockResolvedValue({ isValid: true });
|
||||
|
|
|
|||
|
|
@ -138,7 +138,7 @@ describe('publishing a workflow under review', () => {
|
|||
const request = await createOpenReview(workflow.id, versionId);
|
||||
|
||||
// Activation fails once — after the approval has already committed.
|
||||
workflowValidationService.validateForActivation.mockReturnValueOnce({
|
||||
workflowValidationService.validateForActivation.mockResolvedValueOnce({
|
||||
isValid: false,
|
||||
error: 'The workflow has issues',
|
||||
});
|
||||
|
|
|
|||
|
|
@ -6,6 +6,7 @@ import type {
|
|||
INodeTypeDescription,
|
||||
ICredentialType,
|
||||
} from 'n8n-workflow';
|
||||
import { deepCopy, NodeConnectionTypes } from 'n8n-workflow';
|
||||
import { mock } from 'vitest-mock-extended';
|
||||
|
||||
import type { CredentialTypes } from '@/credential-types';
|
||||
|
|
@ -327,7 +328,7 @@ describe('WorkflowValidationService', () => {
|
|||
return nodeType;
|
||||
};
|
||||
|
||||
it('should return valid for workflow with no connected nodes', () => {
|
||||
it('should return valid for workflow with no connected nodes', async () => {
|
||||
const nodes = {
|
||||
Webhook: createNode('Webhook', 'n8n-nodes-base.webhook'),
|
||||
};
|
||||
|
|
@ -335,12 +336,12 @@ describe('WorkflowValidationService', () => {
|
|||
|
||||
mockNodeTypes.getByNameAndVersion.mockReturnValue(createMockNodeType([], [], true));
|
||||
|
||||
const result = service.validateForActivation(nodes, connections, mockNodeTypes);
|
||||
const result = await service.validateForActivation(nodes, connections, mockNodeTypes);
|
||||
|
||||
expect(result.isValid).toBe(true);
|
||||
});
|
||||
|
||||
it('should return valid for workflow with all valid connected nodes', () => {
|
||||
it('should return valid for workflow with all valid connected nodes', async () => {
|
||||
const nodes = {
|
||||
Webhook: createNode('Webhook', 'n8n-nodes-base.webhook'),
|
||||
'HTTP Request': createNode('HTTP Request', 'n8n-nodes-base.httpRequest', {
|
||||
|
|
@ -365,12 +366,12 @@ describe('WorkflowValidationService', () => {
|
|||
return undefined;
|
||||
}) as any);
|
||||
|
||||
const result = service.validateForActivation(nodes, connections, mockNodeTypes);
|
||||
const result = await service.validateForActivation(nodes, connections, mockNodeTypes);
|
||||
|
||||
expect(result.isValid).toBe(true);
|
||||
});
|
||||
|
||||
it('should return invalid when connected node is missing required credential', () => {
|
||||
it('should return invalid when connected node is missing required credential', async () => {
|
||||
const nodes = {
|
||||
Webhook: createNode('Webhook', 'n8n-nodes-base.webhook'),
|
||||
Agent: createNode('Agent', 'n8n-nodes-base.agent', {
|
||||
|
|
@ -394,7 +395,7 @@ describe('WorkflowValidationService', () => {
|
|||
return undefined;
|
||||
}) as any);
|
||||
|
||||
const result = service.validateForActivation(nodes, connections, mockNodeTypes);
|
||||
const result = await service.validateForActivation(nodes, connections, mockNodeTypes);
|
||||
|
||||
expect(result.isValid).toBe(false);
|
||||
expect(result.error).toContain('Cannot publish workflow');
|
||||
|
|
@ -403,7 +404,7 @@ describe('WorkflowValidationService', () => {
|
|||
expect(result.error).toContain('Missing required credential: OpenAI API');
|
||||
});
|
||||
|
||||
it('should return invalid when connected node has credential without ID', () => {
|
||||
it('should return invalid when connected node has credential without ID', async () => {
|
||||
const nodes = {
|
||||
Webhook: createNode('Webhook', 'n8n-nodes-base.webhook'),
|
||||
Agent: createNode('Agent', 'n8n-nodes-base.agent', {
|
||||
|
|
@ -428,13 +429,13 @@ describe('WorkflowValidationService', () => {
|
|||
return undefined;
|
||||
}) as any);
|
||||
|
||||
const result = service.validateForActivation(nodes, connections, mockNodeTypes);
|
||||
const result = await service.validateForActivation(nodes, connections, mockNodeTypes);
|
||||
|
||||
expect(result.isValid).toBe(false);
|
||||
expect(result.error).toContain('Credential not configured: OpenAI API');
|
||||
});
|
||||
|
||||
it('should skip validation for disabled nodes', () => {
|
||||
it('should skip validation for disabled nodes', async () => {
|
||||
const nodes = {
|
||||
Webhook: createNode('Webhook', 'n8n-nodes-base.webhook'),
|
||||
Agent: createNode('Agent', 'n8n-nodes-base.agent', {
|
||||
|
|
@ -459,12 +460,12 @@ describe('WorkflowValidationService', () => {
|
|||
return undefined;
|
||||
}) as any);
|
||||
|
||||
const result = service.validateForActivation(nodes, connections, mockNodeTypes);
|
||||
const result = await service.validateForActivation(nodes, connections, mockNodeTypes);
|
||||
|
||||
expect(result.isValid).toBe(true);
|
||||
});
|
||||
|
||||
it('should skip validation for disconnected nodes', () => {
|
||||
it('should skip validation for disconnected nodes', async () => {
|
||||
const nodes = {
|
||||
Webhook: createNode('Webhook', 'n8n-nodes-base.webhook'),
|
||||
Agent: createNode('Agent', 'n8n-nodes-base.agent', {
|
||||
|
|
@ -489,12 +490,43 @@ describe('WorkflowValidationService', () => {
|
|||
return undefined;
|
||||
}) as any);
|
||||
|
||||
const result = service.validateForActivation(nodes, connections, mockNodeTypes);
|
||||
const result = await service.validateForActivation(nodes, connections, mockNodeTypes);
|
||||
|
||||
expect(result.isValid).toBe(true);
|
||||
});
|
||||
|
||||
it('should validate multiple nodes with issues', () => {
|
||||
it('should skip validation for an island with no path from a trigger', async () => {
|
||||
const nodes = {
|
||||
Webhook: createNode('Webhook', 'n8n-nodes-base.webhook'),
|
||||
Agent: createNode('Agent', 'n8n-nodes-base.agent', { parameters: {} }),
|
||||
Helper: createNode('Helper', 'n8n-nodes-base.agent', { parameters: {} }),
|
||||
};
|
||||
// Wired to each other but never reached by the webhook, so neither runs.
|
||||
const connections: IConnections = {
|
||||
Helper: { main: [[{ node: 'Agent', type: NodeConnectionTypes.Main, index: 0 }]] },
|
||||
};
|
||||
|
||||
mockNodeTypes.getByNameAndVersion.mockImplementation(((
|
||||
type: string,
|
||||
): INodeType | undefined => {
|
||||
if (type === 'n8n-nodes-base.webhook') {
|
||||
return createMockNodeType([], [], true);
|
||||
}
|
||||
if (type === 'n8n-nodes-base.agent') {
|
||||
return createMockNodeType(
|
||||
[{ name: 'openAiApi', displayName: 'OpenAI API', required: true }],
|
||||
[],
|
||||
);
|
||||
}
|
||||
return undefined;
|
||||
}) as any);
|
||||
|
||||
const result = await service.validateForActivation(nodes, connections, mockNodeTypes);
|
||||
|
||||
expect(result.isValid).toBe(true);
|
||||
});
|
||||
|
||||
it('should validate multiple nodes with issues', async () => {
|
||||
const nodes = {
|
||||
Webhook: createNode('Webhook', 'n8n-nodes-base.webhook'),
|
||||
Agent1: createNode('Agent1', 'n8n-nodes-base.agent', {
|
||||
|
|
@ -524,7 +556,7 @@ describe('WorkflowValidationService', () => {
|
|||
return undefined;
|
||||
}) as any);
|
||||
|
||||
const result = service.validateForActivation(nodes, connections, mockNodeTypes);
|
||||
const result = await service.validateForActivation(nodes, connections, mockNodeTypes);
|
||||
|
||||
expect(result.isValid).toBe(false);
|
||||
expect(result.error).toContain('2 nodes have configuration issues');
|
||||
|
|
@ -532,7 +564,7 @@ describe('WorkflowValidationService', () => {
|
|||
expect(result.error).toContain('Node "Agent2"');
|
||||
});
|
||||
|
||||
it('should return invalid when node type is not found', () => {
|
||||
it('should return invalid when node type is not found', async () => {
|
||||
const nodes = {
|
||||
Webhook: createNode('Webhook', 'n8n-nodes-base.webhook'),
|
||||
Unknown: createNode('Unknown', 'n8n-nodes-base.unknownNode'),
|
||||
|
|
@ -548,14 +580,14 @@ describe('WorkflowValidationService', () => {
|
|||
return undefined;
|
||||
}) as any);
|
||||
|
||||
const result = service.validateForActivation(nodes, connections, mockNodeTypes);
|
||||
const result = await service.validateForActivation(nodes, connections, mockNodeTypes);
|
||||
|
||||
expect(result.isValid).toBe(false);
|
||||
expect(result.error).toContain('Node "Unknown"');
|
||||
expect(result.error).toContain('Node type not found');
|
||||
});
|
||||
|
||||
it('should return invalid when workflow has no trigger node', () => {
|
||||
it('should return invalid when workflow has no trigger node', async () => {
|
||||
const nodes = {
|
||||
Set: createNode('Set', 'n8n-nodes-base.set'),
|
||||
};
|
||||
|
|
@ -563,13 +595,13 @@ describe('WorkflowValidationService', () => {
|
|||
|
||||
mockNodeTypes.getByNameAndVersion.mockReturnValue(createMockNodeType([], []));
|
||||
|
||||
const result = service.validateForActivation(nodes, connections, mockNodeTypes);
|
||||
const result = await service.validateForActivation(nodes, connections, mockNodeTypes);
|
||||
|
||||
expect(result.isValid).toBe(false);
|
||||
expect(result.error).toContain('no trigger node');
|
||||
});
|
||||
|
||||
it('should respect displayOptions when validating credentials', () => {
|
||||
it('should respect displayOptions when validating credentials', async () => {
|
||||
// Simulates a Webhook node with authentication parameter set to 'none'
|
||||
const nodes = {
|
||||
Webhook: createNode('Webhook', 'n8n-nodes-base.webhook', {
|
||||
|
|
@ -605,13 +637,13 @@ describe('WorkflowValidationService', () => {
|
|||
|
||||
mockNodeTypes.getByNameAndVersion.mockReturnValue(nodeType);
|
||||
|
||||
const result = service.validateForActivation(nodes, connections, mockNodeTypes);
|
||||
const result = await service.validateForActivation(nodes, connections, mockNodeTypes);
|
||||
|
||||
// Should be valid because authentication='none', so no credentials are required
|
||||
expect(result.isValid).toBe(true);
|
||||
});
|
||||
|
||||
it('should validate credentials when displayOptions match', () => {
|
||||
it('should validate credentials when displayOptions match', async () => {
|
||||
// Simulates a Webhook node with authentication='basicAuth' but missing credential
|
||||
const nodes = {
|
||||
Webhook: createNode('Webhook', 'n8n-nodes-base.webhook', {
|
||||
|
|
@ -637,7 +669,7 @@ describe('WorkflowValidationService', () => {
|
|||
|
||||
mockNodeTypes.getByNameAndVersion.mockReturnValue(nodeType);
|
||||
|
||||
const result = service.validateForActivation(nodes, connections, mockNodeTypes);
|
||||
const result = await service.validateForActivation(nodes, connections, mockNodeTypes);
|
||||
|
||||
// Should be invalid because authentication='basicAuth' but no credential is set
|
||||
expect(result.isValid).toBe(false);
|
||||
|
|
@ -1742,4 +1774,704 @@ describe('WorkflowValidationService', () => {
|
|||
expect(result.error).toContain('"Webhook"');
|
||||
});
|
||||
});
|
||||
describe('validateRequiredInputsConnected', () => {
|
||||
const nodeTypes = mock<NodeTypes>();
|
||||
|
||||
const node = (name: string, type: string, disabled = false): INode => ({
|
||||
name,
|
||||
type,
|
||||
id: `node-${name}`,
|
||||
typeVersion: 1,
|
||||
position: [0, 0],
|
||||
parameters: {},
|
||||
...(disabled ? { disabled } : {}),
|
||||
});
|
||||
|
||||
/** Parser-like node whose `Model` input is declared required. */
|
||||
const parserType = {
|
||||
description: {
|
||||
displayName: 'Parser',
|
||||
name: 'parser',
|
||||
group: ['transform'],
|
||||
version: 1,
|
||||
description: '',
|
||||
defaults: { name: 'Parser' },
|
||||
inputs: [{ displayName: 'Model', type: 'ai_languageModel', required: true }],
|
||||
outputs: ['ai_outputParser'],
|
||||
properties: [],
|
||||
} as unknown as INodeTypeDescription,
|
||||
} as INodeType;
|
||||
|
||||
/** A parent with no required gated inputs of its own. */
|
||||
const agentType = {
|
||||
description: {
|
||||
displayName: 'Agent',
|
||||
name: 'agent',
|
||||
group: ['transform'],
|
||||
version: 1,
|
||||
description: '',
|
||||
defaults: { name: 'Agent' },
|
||||
inputs: ['main'],
|
||||
outputs: ['main'],
|
||||
properties: [],
|
||||
} as unknown as INodeTypeDescription,
|
||||
} as INodeType;
|
||||
|
||||
const modelType = {
|
||||
description: {
|
||||
displayName: 'Model',
|
||||
name: 'model',
|
||||
group: ['transform'],
|
||||
version: 1,
|
||||
description: '',
|
||||
defaults: { name: 'Model' },
|
||||
inputs: [],
|
||||
outputs: ['ai_languageModel'],
|
||||
properties: [],
|
||||
} as unknown as INodeTypeDescription,
|
||||
} as INodeType;
|
||||
|
||||
/** An entry point, so nodes wired behind it are reachable by a run. */
|
||||
const triggerType = {
|
||||
description: {
|
||||
displayName: 'Trigger',
|
||||
name: 'trigger',
|
||||
group: ['trigger'],
|
||||
version: 1,
|
||||
description: '',
|
||||
defaults: { name: 'Trigger' },
|
||||
inputs: [],
|
||||
outputs: ['main'],
|
||||
properties: [],
|
||||
} as unknown as INodeTypeDescription,
|
||||
trigger: async () => undefined,
|
||||
} as unknown as INodeType;
|
||||
|
||||
/** Wires the trigger into `target`, the way an activatable workflow would. */
|
||||
const startedAt = (target: string) => ({
|
||||
Trigger: { main: [[{ node: target, type: 'main', index: 0 }]] },
|
||||
});
|
||||
|
||||
beforeEach(() => {
|
||||
nodeTypes.getByNameAndVersion.mockImplementation((type: string) => {
|
||||
if (type === 'trigger') return triggerType;
|
||||
if (type === 'parser') return parserType;
|
||||
if (type === 'agent') return agentType;
|
||||
return modelType;
|
||||
});
|
||||
});
|
||||
|
||||
it('rejects activation when a required input has nothing connected', async () => {
|
||||
const result = await service.validateRequiredInputsConnected(
|
||||
[node('Trigger', 'trigger'), node('Parser', 'parser'), node('Agent', 'agent')],
|
||||
{
|
||||
...startedAt('Agent'),
|
||||
Parser: {
|
||||
ai_outputParser: [[{ node: 'Agent', type: 'ai_outputParser', index: 0 }]],
|
||||
},
|
||||
} as unknown as IConnections,
|
||||
nodeTypes,
|
||||
);
|
||||
|
||||
expect(result.isValid).toBe(false);
|
||||
expect(result.error).toContain(
|
||||
"'Parser' has no node connected to its required 'Model' input",
|
||||
);
|
||||
});
|
||||
|
||||
it('ignores an island of wired nodes with no path from a trigger', async () => {
|
||||
// The parser is connected, so it is not a floating node, but nothing
|
||||
// reaches its agent. The island never runs, so it must not block
|
||||
// publishing.
|
||||
const result = await service.validateRequiredInputsConnected(
|
||||
[
|
||||
node('Trigger', 'trigger'),
|
||||
node('Tail', 'agent'),
|
||||
node('Parser', 'parser'),
|
||||
node('Island', 'agent'),
|
||||
],
|
||||
{
|
||||
...startedAt('Tail'),
|
||||
Parser: {
|
||||
ai_outputParser: [[{ node: 'Island', type: 'ai_outputParser', index: 0 }]],
|
||||
},
|
||||
} as unknown as IConnections,
|
||||
nodeTypes,
|
||||
);
|
||||
|
||||
expect(result).toEqual({ isValid: true });
|
||||
});
|
||||
|
||||
it('ignores a chain behind a disabled trigger', async () => {
|
||||
// A disabled trigger starts nothing, so nothing downstream of it runs.
|
||||
nodeTypes.getByNameAndVersion.mockImplementation((type: string) => {
|
||||
if (type === 'trigger') return triggerType;
|
||||
if (type === 'mainParser') {
|
||||
return {
|
||||
description: {
|
||||
...parserType.description,
|
||||
outputs: ['main'],
|
||||
} as unknown as INodeTypeDescription,
|
||||
} as INodeType;
|
||||
}
|
||||
return modelType;
|
||||
});
|
||||
|
||||
const result = await service.validateRequiredInputsConnected(
|
||||
[node('Trigger', 'trigger', true), node('Parser', 'mainParser')],
|
||||
startedAt('Parser') as unknown as IConnections,
|
||||
nodeTypes,
|
||||
);
|
||||
|
||||
expect(result).toEqual({ isValid: true });
|
||||
});
|
||||
|
||||
it('still checks a node behind a disabled node on the main path', async () => {
|
||||
// A disabled node passes its input through, so what follows it still runs.
|
||||
nodeTypes.getByNameAndVersion.mockImplementation((type: string) => {
|
||||
if (type === 'trigger') return triggerType;
|
||||
if (type === 'mainParser') {
|
||||
return {
|
||||
description: {
|
||||
...parserType.description,
|
||||
outputs: ['main'],
|
||||
} as unknown as INodeTypeDescription,
|
||||
} as INodeType;
|
||||
}
|
||||
if (type === 'agent') return agentType;
|
||||
return modelType;
|
||||
});
|
||||
|
||||
const result = await service.validateRequiredInputsConnected(
|
||||
[node('Trigger', 'trigger'), node('Skipped', 'agent', true), node('Parser', 'mainParser')],
|
||||
{
|
||||
...startedAt('Skipped'),
|
||||
Skipped: { main: [[{ node: 'Parser', type: 'main', index: 0 }]] },
|
||||
} as unknown as IConnections,
|
||||
nodeTypes,
|
||||
);
|
||||
|
||||
expect(result.isValid).toBe(false);
|
||||
expect(result.error).toContain("'Parser'");
|
||||
});
|
||||
|
||||
it('ignores a node that is wired to nothing', async () => {
|
||||
// A floating node cannot break a run, so it must not block publishing.
|
||||
const result = await service.validateRequiredInputsConnected(
|
||||
[node('Parser', 'parser')],
|
||||
{},
|
||||
nodeTypes,
|
||||
);
|
||||
|
||||
expect(result).toEqual({ isValid: true });
|
||||
});
|
||||
|
||||
it('ignores a subnode whose only consumer is disabled', async () => {
|
||||
// The trigger reaches the disabled agent, so the parser counts as
|
||||
// executable. A disabled agent never resolves it though, so only the
|
||||
// disabled-consumer exemption keeps this workflow publishable.
|
||||
const result = await service.validateRequiredInputsConnected(
|
||||
[node('Trigger', 'trigger'), node('Parser', 'parser'), node('Agent', 'agent', true)],
|
||||
{
|
||||
...startedAt('Agent'),
|
||||
Parser: {
|
||||
ai_outputParser: [[{ node: 'Agent', type: 'ai_outputParser', index: 0 }]],
|
||||
},
|
||||
} as unknown as IConnections,
|
||||
nodeTypes,
|
||||
);
|
||||
|
||||
expect(result).toEqual({ isValid: true });
|
||||
});
|
||||
|
||||
it('ignores a subnode nested behind a disabled consumer', async () => {
|
||||
// 'Inner' feeds 'Outer', which is enabled, so looking only one hop out
|
||||
// reads it as live. 'Outer' only feeds the disabled agent though, so
|
||||
// nothing ever resolves either of them.
|
||||
const result = await service.validateRequiredInputsConnected(
|
||||
[
|
||||
node('Trigger', 'trigger'),
|
||||
node('Inner', 'parser'),
|
||||
node('Outer', 'parser'),
|
||||
node('Agent', 'agent', true),
|
||||
],
|
||||
{
|
||||
...startedAt('Agent'),
|
||||
Outer: {
|
||||
ai_outputParser: [[{ node: 'Agent', type: 'ai_outputParser', index: 0 }]],
|
||||
},
|
||||
Inner: {
|
||||
ai_outputParser: [[{ node: 'Outer', type: 'ai_outputParser', index: 0 }]],
|
||||
},
|
||||
} as unknown as IConnections,
|
||||
nodeTypes,
|
||||
);
|
||||
|
||||
expect(result).toEqual({ isValid: true });
|
||||
});
|
||||
|
||||
it('still checks a subnode nested behind an enabled consumer', async () => {
|
||||
// Same shape with the agent enabled, so the whole chain resolves.
|
||||
const result = await service.validateRequiredInputsConnected(
|
||||
[
|
||||
node('Trigger', 'trigger'),
|
||||
node('Inner', 'parser'),
|
||||
node('Outer', 'parser'),
|
||||
node('Agent', 'agent'),
|
||||
],
|
||||
{
|
||||
...startedAt('Agent'),
|
||||
Outer: {
|
||||
ai_outputParser: [[{ node: 'Agent', type: 'ai_outputParser', index: 0 }]],
|
||||
},
|
||||
Inner: {
|
||||
ai_outputParser: [[{ node: 'Outer', type: 'ai_outputParser', index: 0 }]],
|
||||
},
|
||||
} as unknown as IConnections,
|
||||
nodeTypes,
|
||||
);
|
||||
|
||||
expect(result.isValid).toBe(false);
|
||||
expect(result.error).toContain("'Inner'");
|
||||
expect(result.error).toContain("'Outer'");
|
||||
});
|
||||
|
||||
it('ignores a subnode whose consumer chain loops back on itself', async () => {
|
||||
// A cycle between two supply nodes must not hang the consumer walk.
|
||||
const result = await service.validateRequiredInputsConnected(
|
||||
[
|
||||
node('Trigger', 'trigger'),
|
||||
node('Left', 'parser'),
|
||||
node('Right', 'parser'),
|
||||
node('Agent', 'agent', true),
|
||||
],
|
||||
{
|
||||
...startedAt('Agent'),
|
||||
Left: {
|
||||
ai_outputParser: [
|
||||
[
|
||||
{ node: 'Right', type: 'ai_outputParser', index: 0 },
|
||||
{ node: 'Agent', type: 'ai_outputParser', index: 0 },
|
||||
],
|
||||
],
|
||||
},
|
||||
Right: {
|
||||
ai_outputParser: [[{ node: 'Left', type: 'ai_outputParser', index: 0 }]],
|
||||
},
|
||||
} as unknown as IConnections,
|
||||
nodeTypes,
|
||||
);
|
||||
|
||||
expect(result).toEqual({ isValid: true });
|
||||
});
|
||||
|
||||
it('still checks a subnode that also supplies an enabled consumer', async () => {
|
||||
const result = await service.validateRequiredInputsConnected(
|
||||
[
|
||||
node('Trigger', 'trigger'),
|
||||
node('Parser', 'parser'),
|
||||
node('Off', 'agent', true),
|
||||
node('On', 'agent'),
|
||||
],
|
||||
{
|
||||
...startedAt('On'),
|
||||
Parser: {
|
||||
ai_outputParser: [
|
||||
[
|
||||
{ node: 'Off', type: 'ai_outputParser', index: 0 },
|
||||
{ node: 'On', type: 'ai_outputParser', index: 0 },
|
||||
],
|
||||
],
|
||||
},
|
||||
} as unknown as IConnections,
|
||||
nodeTypes,
|
||||
);
|
||||
|
||||
expect(result.isValid).toBe(false);
|
||||
});
|
||||
|
||||
it('still checks a node that can output main but has only a disabled subnode consumer', async () => {
|
||||
// Its main output happens to be unwired, so the connection record shows
|
||||
// only the ai_ edge. Declared outputs are what decide, not the wiring.
|
||||
nodeTypes.getByNameAndVersion.mockImplementation((type: string) => {
|
||||
if (type === 'trigger') return triggerType;
|
||||
if (type === 'dualOutput') {
|
||||
return {
|
||||
description: {
|
||||
...parserType.description,
|
||||
outputs: ['main', 'ai_tool'],
|
||||
} as unknown as INodeTypeDescription,
|
||||
} as INodeType;
|
||||
}
|
||||
return agentType;
|
||||
});
|
||||
|
||||
const result = await service.validateRequiredInputsConnected(
|
||||
[node('Trigger', 'trigger'), node('Store', 'dualOutput'), node('Agent', 'agent', true)],
|
||||
{
|
||||
...startedAt('Store'),
|
||||
Store: { ai_tool: [[{ node: 'Agent', type: 'ai_tool', index: 0 }]] },
|
||||
} as unknown as IConnections,
|
||||
nodeTypes,
|
||||
);
|
||||
|
||||
expect(result.isValid).toBe(false);
|
||||
expect(result.error).toContain("'Store'");
|
||||
});
|
||||
|
||||
it('still checks a node on the main path when its consumer is disabled', async () => {
|
||||
// Unlike a subnode, a node with a main output runs whatever happens
|
||||
// downstream, so its own unmet input still blocks publishing.
|
||||
nodeTypes.getByNameAndVersion.mockImplementation((type: string) => {
|
||||
if (type === 'trigger') return triggerType;
|
||||
if (type === 'mainParser') {
|
||||
return {
|
||||
description: {
|
||||
...parserType.description,
|
||||
outputs: ['main'],
|
||||
} as unknown as INodeTypeDescription,
|
||||
} as INodeType;
|
||||
}
|
||||
return agentType;
|
||||
});
|
||||
|
||||
const result = await service.validateRequiredInputsConnected(
|
||||
[node('Trigger', 'trigger'), node('Parser', 'mainParser'), node('Next', 'agent', true)],
|
||||
{
|
||||
...startedAt('Parser'),
|
||||
Parser: { main: [[{ node: 'Next', type: 'main', index: 0 }]] },
|
||||
} as unknown as IConnections,
|
||||
nodeTypes,
|
||||
);
|
||||
|
||||
expect(result.isValid).toBe(false);
|
||||
});
|
||||
|
||||
describe('with a fallback model declared on a second input of the same type', () => {
|
||||
const agentWithFallbackType = {
|
||||
description: {
|
||||
displayName: 'Agent',
|
||||
name: 'agentWithFallback',
|
||||
group: ['transform'],
|
||||
version: 1,
|
||||
description: '',
|
||||
defaults: { name: 'Agent' },
|
||||
inputs: [
|
||||
'main',
|
||||
{ displayName: 'Chat Model', type: 'ai_languageModel', required: true },
|
||||
{ displayName: 'Fallback Model', type: 'ai_languageModel', required: true },
|
||||
],
|
||||
outputs: ['main'],
|
||||
properties: [],
|
||||
} as unknown as INodeTypeDescription,
|
||||
} as INodeType;
|
||||
|
||||
beforeEach(() => {
|
||||
nodeTypes.getByNameAndVersion.mockImplementation((type: string) => {
|
||||
if (type === 'trigger') return triggerType;
|
||||
return type === 'agentWithFallback' ? agentWithFallbackType : modelType;
|
||||
});
|
||||
});
|
||||
|
||||
it('rejects activation when only the primary model is connected', async () => {
|
||||
const result = await service.validateRequiredInputsConnected(
|
||||
[node('Trigger', 'trigger'), node('Agent', 'agentWithFallback'), node('Model', 'model')],
|
||||
{
|
||||
...startedAt('Agent'),
|
||||
Model: {
|
||||
ai_languageModel: [[{ node: 'Agent', type: 'ai_languageModel', index: 0 }]],
|
||||
},
|
||||
} as unknown as IConnections,
|
||||
nodeTypes,
|
||||
);
|
||||
|
||||
expect(result.isValid).toBe(false);
|
||||
expect(result.error).toContain("'Fallback Model'");
|
||||
});
|
||||
|
||||
it('allows activation once each model sits on its own input', async () => {
|
||||
const result = await service.validateRequiredInputsConnected(
|
||||
[
|
||||
node('Trigger', 'trigger'),
|
||||
node('Agent', 'agentWithFallback'),
|
||||
node('Primary', 'model'),
|
||||
node('Fallback', 'model'),
|
||||
],
|
||||
{
|
||||
...startedAt('Agent'),
|
||||
Primary: {
|
||||
ai_languageModel: [[{ node: 'Agent', type: 'ai_languageModel', index: 0 }]],
|
||||
},
|
||||
Fallback: {
|
||||
ai_languageModel: [[{ node: 'Agent', type: 'ai_languageModel', index: 1 }]],
|
||||
},
|
||||
} as unknown as IConnections,
|
||||
nodeTypes,
|
||||
);
|
||||
|
||||
expect(result).toEqual({ isValid: true });
|
||||
});
|
||||
|
||||
it('treats a disabled fallback as not connected', async () => {
|
||||
const result = await service.validateRequiredInputsConnected(
|
||||
[
|
||||
node('Trigger', 'trigger'),
|
||||
node('Agent', 'agentWithFallback'),
|
||||
node('Primary', 'model'),
|
||||
node('Fallback', 'model', true),
|
||||
],
|
||||
{
|
||||
...startedAt('Agent'),
|
||||
Primary: {
|
||||
ai_languageModel: [[{ node: 'Agent', type: 'ai_languageModel', index: 0 }]],
|
||||
},
|
||||
Fallback: {
|
||||
ai_languageModel: [[{ node: 'Agent', type: 'ai_languageModel', index: 1 }]],
|
||||
},
|
||||
} as unknown as IConnections,
|
||||
nodeTypes,
|
||||
);
|
||||
|
||||
expect(result.isValid).toBe(false);
|
||||
expect(result.error).toContain("'Fallback Model'");
|
||||
});
|
||||
});
|
||||
|
||||
it('still checks a trigger that is wired to nothing', async () => {
|
||||
// A trigger starts the run whether or not anything feeds it, so the
|
||||
// floating-node exemption must not cover it.
|
||||
nodeTypes.getByNameAndVersion.mockImplementation((type: string) => {
|
||||
if (type === 'trigger') return triggerType;
|
||||
if (type === 'triggerParser') {
|
||||
return { ...parserType, trigger: async () => undefined } as unknown as INodeType;
|
||||
}
|
||||
return modelType;
|
||||
});
|
||||
|
||||
const result = await service.validateRequiredInputsConnected(
|
||||
[node('Trigger', 'triggerParser')],
|
||||
{},
|
||||
nodeTypes,
|
||||
);
|
||||
|
||||
expect(result.isValid).toBe(false);
|
||||
expect(result.error).toContain("'Trigger'");
|
||||
});
|
||||
|
||||
it('does not rewrite the parameters of the nodes it was given', async () => {
|
||||
// These are the nodes about to be persisted as the active version.
|
||||
const parser = node('Parser', 'parser');
|
||||
const before = parser.parameters;
|
||||
|
||||
await service.validateRequiredInputsConnected(
|
||||
[parser, node('Model', 'model')],
|
||||
{
|
||||
Model: {
|
||||
ai_languageModel: [[{ node: 'Parser', type: 'ai_languageModel', index: 0 }]],
|
||||
},
|
||||
} as unknown as IConnections,
|
||||
nodeTypes,
|
||||
);
|
||||
|
||||
expect(parser.parameters).toBe(before);
|
||||
});
|
||||
|
||||
it('allows activation once the required input is connected', async () => {
|
||||
const result = await service.validateRequiredInputsConnected(
|
||||
[node('Parser', 'parser'), node('Model', 'model')],
|
||||
{
|
||||
Model: {
|
||||
ai_languageModel: [[{ node: 'Parser', type: 'ai_languageModel', index: 0 }]],
|
||||
},
|
||||
} as unknown as IConnections,
|
||||
nodeTypes,
|
||||
);
|
||||
|
||||
expect(result).toEqual({ isValid: true });
|
||||
});
|
||||
|
||||
it('treats a disabled source as not connected', async () => {
|
||||
const result = await service.validateRequiredInputsConnected(
|
||||
[node('Trigger', 'trigger'), node('Parser', 'parser'), node('Model', 'model', true)],
|
||||
{
|
||||
...startedAt('Parser'),
|
||||
Model: {
|
||||
ai_languageModel: [[{ node: 'Parser', type: 'ai_languageModel', index: 0 }]],
|
||||
},
|
||||
} as unknown as IConnections,
|
||||
nodeTypes,
|
||||
);
|
||||
|
||||
expect(result.isValid).toBe(false);
|
||||
});
|
||||
|
||||
it('ignores disabled nodes with unmet requirements', async () => {
|
||||
const result = await service.validateRequiredInputsConnected(
|
||||
[node('Parser', 'parser', true), node('Agent', 'agent')],
|
||||
{
|
||||
Parser: {
|
||||
ai_outputParser: [[{ node: 'Agent', type: 'ai_outputParser', index: 0 }]],
|
||||
},
|
||||
} as unknown as IConnections,
|
||||
nodeTypes,
|
||||
);
|
||||
|
||||
expect(result).toEqual({ isValid: true });
|
||||
});
|
||||
|
||||
it('reports every unmet required input at once', async () => {
|
||||
const result = await service.validateRequiredInputsConnected(
|
||||
[
|
||||
node('Trigger', 'trigger'),
|
||||
node('Parser A', 'parser'),
|
||||
node('Parser B', 'parser'),
|
||||
node('Agent', 'agent'),
|
||||
],
|
||||
{
|
||||
...startedAt('Agent'),
|
||||
'Parser A': {
|
||||
ai_outputParser: [[{ node: 'Agent', type: 'ai_outputParser', index: 0 }]],
|
||||
},
|
||||
'Parser B': {
|
||||
ai_outputParser: [[{ node: 'Agent', type: 'ai_outputParser', index: 0 }]],
|
||||
},
|
||||
} as unknown as IConnections,
|
||||
nodeTypes,
|
||||
);
|
||||
|
||||
expect(result.error).toContain("'Parser A'");
|
||||
expect(result.error).toContain("'Parser B'");
|
||||
});
|
||||
|
||||
describe('with a dynamic inputs expression', () => {
|
||||
/** `inputs` is an expression, so it resolves against the node's parameters. */
|
||||
const gatedParserType = {
|
||||
description: {
|
||||
displayName: 'Parser',
|
||||
name: 'gatedParser',
|
||||
group: ['transform'],
|
||||
version: 1,
|
||||
description: '',
|
||||
defaults: { name: 'Parser' },
|
||||
inputs:
|
||||
'={{ $parameter.autoFix ? [{ displayName: "Model", type: "ai_languageModel", required: true }] : [] }}',
|
||||
outputs: ['ai_outputParser'],
|
||||
properties: [
|
||||
{ name: 'autoFix', displayName: 'Auto-fix', type: 'boolean', default: true },
|
||||
],
|
||||
} as unknown as INodeTypeDescription,
|
||||
} as INodeType;
|
||||
|
||||
/**
|
||||
* Same shape, but the expression hits a runtime error. Mirrors the
|
||||
* LangChain Code node, which maps over its `Inputs` collection — empty
|
||||
* by default, so the map throws on `undefined`.
|
||||
*/
|
||||
const brokenParserType = {
|
||||
description: {
|
||||
...gatedParserType.description,
|
||||
name: 'brokenParser',
|
||||
inputs: '={{ $parameter.inputs.input.map((i) => ({ type: i.type })) }}',
|
||||
} as unknown as INodeTypeDescription,
|
||||
} as INodeType;
|
||||
|
||||
/** Same shape, but the expression cannot be parsed at all. */
|
||||
const malformedParserType = {
|
||||
description: {
|
||||
...gatedParserType.description,
|
||||
name: 'malformedParser',
|
||||
inputs: '={{ ( }}',
|
||||
} as unknown as INodeTypeDescription,
|
||||
} as INodeType;
|
||||
|
||||
beforeEach(() => {
|
||||
nodeTypes.getByNameAndVersion.mockImplementation((type: string) => {
|
||||
if (type === 'trigger') return triggerType;
|
||||
if (type === 'gatedParser') return gatedParserType;
|
||||
if (type === 'brokenParser') return brokenParserType;
|
||||
if (type === 'malformedParser') return malformedParserType;
|
||||
if (type === 'agent') return agentType;
|
||||
return modelType;
|
||||
});
|
||||
});
|
||||
|
||||
it('resolves the expression against parameter defaults the caller never set', async () => {
|
||||
// The node omits `autoFix`; only the type default makes the input
|
||||
// required. The transient workflow fills defaults in, so the
|
||||
// expression must see the filled-in value, not the caller's bare node.
|
||||
const parser = node('Parser', 'gatedParser');
|
||||
expect(parser.parameters).toEqual({});
|
||||
|
||||
const result = await service.validateRequiredInputsConnected(
|
||||
[node('Trigger', 'trigger'), parser, node('Agent', 'agent')],
|
||||
{
|
||||
...startedAt('Agent'),
|
||||
Parser: {
|
||||
ai_outputParser: [[{ node: 'Agent', type: 'ai_outputParser', index: 0 }]],
|
||||
},
|
||||
} as unknown as IConnections,
|
||||
nodeTypes,
|
||||
);
|
||||
|
||||
expect(result.isValid).toBe(false);
|
||||
expect(result.error).toContain("'Parser'");
|
||||
// Filling defaults must not leak back onto the version being saved.
|
||||
expect(parser.parameters).toEqual({});
|
||||
});
|
||||
|
||||
it('reads a runtime error in the expression the way the engine does', async () => {
|
||||
// The engine swallows it and runs the node with no inputs, so refusing
|
||||
// to publish here would block a workflow the runtime is happy with.
|
||||
// The real case is the LangChain Code node, which maps over its
|
||||
// `Inputs` collection — empty by default.
|
||||
const result = await service.validateRequiredInputsConnected(
|
||||
[node('Trigger', 'trigger'), node('Parser', 'brokenParser'), node('Agent', 'agent')],
|
||||
{
|
||||
...startedAt('Agent'),
|
||||
Parser: {
|
||||
ai_outputParser: [[{ node: 'Agent', type: 'ai_outputParser', index: 0 }]],
|
||||
},
|
||||
} as unknown as IConnections,
|
||||
nodeTypes,
|
||||
);
|
||||
|
||||
expect(result.isValid).toBe(true);
|
||||
});
|
||||
|
||||
it('reports an input it could not determine instead of treating it as absent', async () => {
|
||||
// An unparseable expression is a bug in the node type, not a state a
|
||||
// user can configure, so it is worth surfacing.
|
||||
const result = await service.validateRequiredInputsConnected(
|
||||
[node('Trigger', 'trigger'), node('Parser', 'malformedParser'), node('Agent', 'agent')],
|
||||
{
|
||||
...startedAt('Agent'),
|
||||
Parser: {
|
||||
ai_outputParser: [[{ node: 'Agent', type: 'ai_outputParser', index: 0 }]],
|
||||
},
|
||||
} as unknown as IConnections,
|
||||
nodeTypes,
|
||||
);
|
||||
|
||||
expect(result.isValid).toBe(false);
|
||||
expect(result.error).toContain('could not be determined');
|
||||
});
|
||||
});
|
||||
|
||||
it('does not mutate the connections it was given', async () => {
|
||||
const connections = {
|
||||
Model: {
|
||||
ai_languageModel: [[{ node: 'Parser', type: 'ai_languageModel', index: 0 }]],
|
||||
},
|
||||
} as unknown as IConnections;
|
||||
const snapshot = deepCopy(connections);
|
||||
|
||||
await service.validateRequiredInputsConnected(
|
||||
[node('Parser', 'parser'), node('Model', 'model')],
|
||||
connections,
|
||||
nodeTypes,
|
||||
);
|
||||
|
||||
expect(connections).toEqual(snapshot);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -1363,14 +1363,14 @@ describe('WorkflowService', () => {
|
|||
// Bypass validation internals
|
||||
const internals = workflowService as unknown as {
|
||||
_detectWebhookConflicts: () => Promise<void>;
|
||||
_validateNodes: () => void;
|
||||
_validateNodes: () => Promise<void>;
|
||||
_validateDynamicCredentials: () => Promise<void>;
|
||||
_validatePublisherCredentialAccess: () => Promise<void>;
|
||||
_validateSubWorkflowReferences: () => Promise<void>;
|
||||
_validateTriggerNodeIds: () => void;
|
||||
};
|
||||
vi.spyOn(internals, '_detectWebhookConflicts').mockResolvedValue(undefined);
|
||||
vi.spyOn(internals, '_validateNodes').mockReturnValue(undefined);
|
||||
vi.spyOn(internals, '_validateNodes').mockResolvedValue(undefined);
|
||||
vi.spyOn(internals, '_validateDynamicCredentials').mockResolvedValue(undefined);
|
||||
vi.spyOn(internals, '_validatePublisherCredentialAccess').mockResolvedValue(undefined);
|
||||
vi.spyOn(internals, '_validateSubWorkflowReferences').mockResolvedValue(undefined);
|
||||
|
|
|
|||
|
|
@ -7,12 +7,16 @@ import { FULL_ACCESS_NODE_TYPES } from 'n8n-core';
|
|||
import {
|
||||
validateWorkflowHasTriggerLikeNode,
|
||||
NodeHelpers,
|
||||
Workflow,
|
||||
mapConnectionsByDestination,
|
||||
validateNodeCredentials,
|
||||
isNodeConnected,
|
||||
getUnconnectedRequiredInputs,
|
||||
getReachableNodeNames,
|
||||
onlySuppliesDisabledNodes,
|
||||
isTriggerLikeNode,
|
||||
isTriggerNode,
|
||||
classifyTriggerIdentity,
|
||||
NodeConnectionTypes,
|
||||
} from 'n8n-workflow';
|
||||
import type {
|
||||
INode,
|
||||
|
|
@ -28,6 +32,7 @@ import { CredentialTypes } from '@/credential-types';
|
|||
import { DynamicCredentialsProxy } from '@/credentials/dynamic-credentials-proxy';
|
||||
import { CredentialsPermissionChecker } from '@/executions/pre-execution-checks/credentials-permission-checker';
|
||||
import type { NodeTypes } from '@/node-types';
|
||||
import { withExpressionIsolate } from '@/utils';
|
||||
|
||||
export interface WorkflowValidationResult {
|
||||
isValid: boolean;
|
||||
|
|
@ -64,8 +69,32 @@ export class WorkflowValidationService {
|
|||
) {}
|
||||
|
||||
/**
|
||||
* Validates node configuration (credentials, parameters) for connected and enabled nodes.
|
||||
* Trigger-like nodes are always validated even without connections.
|
||||
* Names of every node a run can reach. Publishing must not fail over a node
|
||||
* that never executes, and an edge alone does not make a node reachable: an
|
||||
* island of wired-up nodes with no path from a trigger never runs.
|
||||
*
|
||||
* Every enabled trigger is a start node, a disconnected one included, since
|
||||
* it still starts the workflow.
|
||||
*/
|
||||
private executableNodeNames(
|
||||
nodes: INode[],
|
||||
connections: IConnections,
|
||||
nodeTypes: NodeTypes,
|
||||
): Set<string> {
|
||||
return getReachableNodeNames(
|
||||
nodes,
|
||||
connections,
|
||||
mapConnectionsByDestination(connections),
|
||||
(node) => {
|
||||
const nodeType = nodeTypes.getByNameAndVersion(node.type, node.typeVersion);
|
||||
return !!nodeType && isTriggerLikeNode(nodeType);
|
||||
},
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Validates node configuration (credentials, parameters) for every node a run
|
||||
* can reach. Enabled nodes only.
|
||||
*/
|
||||
private validateNodeConfiguration(
|
||||
nodes: INode[],
|
||||
|
|
@ -73,7 +102,7 @@ export class WorkflowValidationService {
|
|||
nodeTypes: NodeTypes,
|
||||
): WorkflowValidationResult {
|
||||
try {
|
||||
const connectionsByDestination = mapConnectionsByDestination(connections);
|
||||
const executable = this.executableNodeNames(nodes, connections, nodeTypes);
|
||||
const issuesFound: Array<{ nodeName: string; issues: string[] }> = [];
|
||||
|
||||
for (const node of nodes) {
|
||||
|
|
@ -90,11 +119,10 @@ export class WorkflowValidationService {
|
|||
continue;
|
||||
}
|
||||
|
||||
const isNodeTriggerLike = isTriggerLikeNode(nodeType);
|
||||
|
||||
const isConnected = isNodeConnected(node.name, connections, connectionsByDestination);
|
||||
|
||||
if (!isConnected && !isNodeTriggerLike) continue;
|
||||
// A node no run can reach cannot fail one, so it must not block
|
||||
// publishing. Being wired to something is not enough: an island of
|
||||
// connected nodes with no path from a trigger never executes.
|
||||
if (!executable.has(node.name)) continue;
|
||||
|
||||
const nodeIssues: string[] = [];
|
||||
const credentialIssues = validateNodeCredentials(node, nodeType);
|
||||
|
|
@ -314,11 +342,11 @@ export class WorkflowValidationService {
|
|||
return [];
|
||||
}
|
||||
|
||||
validateForActivation(
|
||||
async validateForActivation(
|
||||
nodes: INodes,
|
||||
connections: IConnections,
|
||||
nodeTypes: NodeTypes,
|
||||
): WorkflowValidationResult {
|
||||
): Promise<WorkflowValidationResult> {
|
||||
// Validate workflow entry points: active, poll, webhook, or schedule triggers.
|
||||
const triggerValidation = validateWorkflowHasTriggerLikeNode(nodes, nodeTypes, STARTING_NODES);
|
||||
|
||||
|
|
@ -339,6 +367,103 @@ export class WorkflowValidationService {
|
|||
return configValidation;
|
||||
}
|
||||
|
||||
return await this.validateRequiredInputsConnected(nodesArray, connections, nodeTypes);
|
||||
}
|
||||
|
||||
/**
|
||||
* Refuses activation when a required input has nothing connected. The editor
|
||||
* draws this warning already but nothing enforced it, so such a workflow could
|
||||
* publish and then throw on every execution.
|
||||
*
|
||||
* Called by `validateForActivation`; public so it can be exercised directly.
|
||||
*/
|
||||
async validateRequiredInputsConnected(
|
||||
nodes: INode[],
|
||||
connections: IConnections,
|
||||
nodeTypes: NodeTypes,
|
||||
): Promise<WorkflowValidationResult> {
|
||||
// Transient, so dynamic `inputs` expressions can be evaluated. Built over
|
||||
// shallow node copies because the constructor reassigns `node.parameters`
|
||||
// with defaults filled in, and these nodes are the version about to be saved.
|
||||
const workflow = new Workflow({
|
||||
nodes: nodes.map((node) => ({ ...node })),
|
||||
connections,
|
||||
active: false,
|
||||
nodeTypes,
|
||||
});
|
||||
const executable = this.executableNodeNames(nodes, connections, nodeTypes);
|
||||
const issues: string[] = [];
|
||||
|
||||
// Memoized because the consumer walk revisits nodes, and resolving outputs
|
||||
// can mean evaluating an expression.
|
||||
const canOutputMainByName = new Map<string, boolean>();
|
||||
const canOutputMain = (candidate: INode): boolean => {
|
||||
const cached = canOutputMainByName.get(candidate.name);
|
||||
if (cached !== undefined) return cached;
|
||||
|
||||
const candidateType = nodeTypes.getByNameAndVersion(candidate.type, candidate.typeVersion);
|
||||
// Unknown type: assume it belongs to the flow, so nothing opts out quietly.
|
||||
const result = candidateType?.description
|
||||
? NodeHelpers.getNodeOutputs(workflow, candidate, candidateType.description).some(
|
||||
(output) =>
|
||||
(typeof output === 'string' ? output : output.type) === NodeConnectionTypes.Main,
|
||||
)
|
||||
: true;
|
||||
|
||||
canOutputMainByName.set(candidate.name, result);
|
||||
return result;
|
||||
};
|
||||
|
||||
// Those expressions need an isolate under the VM engine, or they throw.
|
||||
await withExpressionIsolate(workflow, async () => {
|
||||
for (const node of nodes) {
|
||||
if (node.disabled) continue;
|
||||
|
||||
const nodeType = nodeTypes.getByNameAndVersion(node.type, node.typeVersion);
|
||||
if (!nodeType?.description) continue;
|
||||
|
||||
// Same rule as validateNodeConfiguration: only nodes a run can reach.
|
||||
if (!executable.has(node.name)) continue;
|
||||
|
||||
// Same reasoning further out: a subnode is only ever resolved by the
|
||||
// node it supplies, so if every chain of consumers out of it is
|
||||
// disabled it cannot break a run either. Scoped to nodes with no main
|
||||
// output, since a node on the main path runs whatever its consumers do.
|
||||
if (
|
||||
!isTriggerLikeNode(nodeType) &&
|
||||
onlySuppliesDisabledNodes(node, connections, nodes, canOutputMain)
|
||||
) {
|
||||
continue;
|
||||
}
|
||||
|
||||
// Strictly: a swallowed expression error would read as "requires nothing".
|
||||
let required: ReturnType<typeof getUnconnectedRequiredInputs>;
|
||||
try {
|
||||
required = getUnconnectedRequiredInputs(workflow, node, nodeType.description, {
|
||||
throwOnExpressionError: true,
|
||||
});
|
||||
} catch (error) {
|
||||
issues.push(
|
||||
`the inputs of '${node.name}' could not be determined (${ensureError(error).message})`,
|
||||
);
|
||||
continue;
|
||||
}
|
||||
|
||||
for (const input of required) {
|
||||
issues.push(
|
||||
`'${node.name}' has no node connected to its required '${input.displayName ?? input.type}' input`,
|
||||
);
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
if (issues.length > 0) {
|
||||
return {
|
||||
isValid: false,
|
||||
error: `Workflow cannot be activated because required inputs are not connected: ${issues.join('; ')}.`,
|
||||
};
|
||||
}
|
||||
|
||||
return { isValid: true };
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -1075,7 +1075,7 @@ export class WorkflowService {
|
|||
|
||||
await this._detectWebhookConflicts(workflow, versionToActivate);
|
||||
|
||||
this._validateNodes(workflowId, versionToActivate.nodes, versionToActivate.connections);
|
||||
await this._validateNodes(workflowId, versionToActivate.nodes, versionToActivate.connections);
|
||||
await this._validateDynamicCredentials(workflowId, versionToActivate.nodes, workflow.settings);
|
||||
if (versionIdToActivate !== previousActiveVersionId) {
|
||||
await this._validatePublisherCredentialAccess(workflowId, user, versionToActivate.nodes);
|
||||
|
|
@ -1858,13 +1858,13 @@ export class WorkflowService {
|
|||
}
|
||||
}
|
||||
|
||||
_validateNodes(workflowId: string, nodes: INode[], connections: IConnections) {
|
||||
async _validateNodes(workflowId: string, nodes: INode[], connections: IConnections) {
|
||||
const nodesToValidate = nodes.reduce<INodes>((acc, node) => {
|
||||
acc[node.name] = node;
|
||||
return acc;
|
||||
}, {});
|
||||
|
||||
const validation = this.workflowValidationService.validateForActivation(
|
||||
const validation = await this.workflowValidationService.validateForActivation(
|
||||
nodesToValidate,
|
||||
connections,
|
||||
this.nodeTypes,
|
||||
|
|
|
|||
|
|
@ -144,7 +144,7 @@ beforeEach(() => {
|
|||
nodeTypes.getByNameAndVersion.mockReset();
|
||||
workflowValidationService.validateTriggerNodeIds.mockReset();
|
||||
workflowValidationService.validateTriggerNodeIds.mockReturnValue({ isValid: true });
|
||||
workflowValidationService.validateForActivation.mockReturnValue({ isValid: true });
|
||||
workflowValidationService.validateForActivation.mockResolvedValue({ isValid: true });
|
||||
workflowValidationService.validateDynamicCredentials.mockResolvedValue({ isValid: true });
|
||||
workflowValidationService.validatePublisherCredentialAccess.mockResolvedValue({ isValid: true });
|
||||
workflowValidationService.validateSubWorkflowReferences.mockResolvedValue({ isValid: true });
|
||||
|
|
@ -617,7 +617,7 @@ describe('activateWorkflow()', () => {
|
|||
await createWorkflowHistoryItem(workflow.id, { versionId: newVersionId });
|
||||
|
||||
// Mock validation to fail
|
||||
workflowValidationService.validateForActivation.mockReturnValue({
|
||||
workflowValidationService.validateForActivation.mockResolvedValue({
|
||||
isValid: false,
|
||||
error: 'Workflow cannot be activated because it has no trigger node.',
|
||||
});
|
||||
|
|
|
|||
|
|
@ -142,7 +142,7 @@ beforeEach(async () => {
|
|||
authMemberAgent = testServer.authAgentFor(member);
|
||||
anotherMember = await createMember();
|
||||
|
||||
workflowValidationService.validateForActivation.mockReturnValue({ isValid: true });
|
||||
workflowValidationService.validateForActivation.mockResolvedValue({ isValid: true });
|
||||
workflowValidationService.validateDynamicCredentials.mockResolvedValue({ isValid: true });
|
||||
workflowValidationService.validatePublisherCredentialAccess.mockResolvedValue({ isValid: true });
|
||||
workflowValidationService.validateSubWorkflowReferences.mockResolvedValue({ isValid: true });
|
||||
|
|
|
|||
|
|
@ -172,9 +172,9 @@ const containsTrigger = computed((): boolean => {
|
|||
return foundTriggers.value.length > 0;
|
||||
});
|
||||
|
||||
const nodesWithValidationIssues = computed(
|
||||
() => workflowDocumentStore.value.nodesWithValidationIssues,
|
||||
);
|
||||
// The nodes that actually block publishing, so the count in the message matches
|
||||
// why the button is disabled.
|
||||
const nodesWithValidationIssues = computed(() => workflowDocumentStore.value.publishBlockingNodes);
|
||||
|
||||
const hasNodeIssues = computed(() => workflowDocumentStore.value.hasPublishBlockingIssues);
|
||||
|
||||
|
|
|
|||
|
|
@ -276,6 +276,7 @@ describe('useCanvasOperations', () => {
|
|||
vi.mocked(workflowDocumentStoreInstance.getWorkflowObjectAccessorSnapshot).mockReturnValue({
|
||||
id: workflowDocumentStoreInstance.workflowId,
|
||||
connectionsBySourceNode: workflowDocumentStoreInstance.connectionsBySourceNode,
|
||||
connectionsByDestinationNode: workflowDocumentStoreInstance.connectionsByDestinationNode,
|
||||
pinData: workflowDocumentStoreInstance.pinnedDataByNodeName as IPinData,
|
||||
expression: workflowDocumentStoreInstance.getExpressionHandler(),
|
||||
getNode: workflowDocumentStoreInstance.getNodeByName,
|
||||
|
|
|
|||
|
|
@ -17,6 +17,7 @@ import { CUSTOM_API_CALL_KEY, EnterpriseEditionFeature } from '@/app/constants';
|
|||
import { mockedStore } from '@/__tests__/utils';
|
||||
import { mock } from 'vitest-mock-extended';
|
||||
import { faker } from '@faker-js/faker';
|
||||
import type { WorkflowObjectAccessors } from '@/app/types/workflow';
|
||||
import type { INodeUi } from '@/Interface';
|
||||
import type {
|
||||
IUsedCredential,
|
||||
|
|
@ -49,6 +50,7 @@ const mockDocumentStore = {
|
|||
updateNodeProperties: vi.fn(),
|
||||
getExpressionHandler: vi.fn(() => ({})),
|
||||
getPinDataSnapshot: vi.fn().mockReturnValue({}),
|
||||
getWorkflowObjectAccessorSnapshot: vi.fn(() => ({}) as unknown as WorkflowObjectAccessors),
|
||||
};
|
||||
|
||||
vi.mock('@/app/stores/workflowDocument.store', async () => {
|
||||
|
|
@ -712,6 +714,154 @@ describe('useNodeHelpers()', () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe('input issues in getNodeIssues', () => {
|
||||
const nodeTypeWithRequiredInput: INodeTypeDescription = {
|
||||
displayName: 'Agent',
|
||||
name: 'agent',
|
||||
group: ['transform'],
|
||||
version: 1,
|
||||
description: 'Agent node',
|
||||
defaults: { name: 'Agent' },
|
||||
inputs: [
|
||||
NodeConnectionTypes.Main,
|
||||
{ type: NodeConnectionTypes.AiLanguageModel, displayName: 'Model', required: true },
|
||||
],
|
||||
outputs: [NodeConnectionTypes.Main],
|
||||
properties: [],
|
||||
};
|
||||
|
||||
it('reports a required input with nothing connected', () => {
|
||||
const node = createTestNode({ name: 'Agent', type: 'agent' });
|
||||
const workflow = {
|
||||
getNode: () => node,
|
||||
connectionsByDestinationNode: {},
|
||||
} as unknown as Workflow;
|
||||
|
||||
const { getNodeIssues } = useNodeHelpers();
|
||||
const result = getNodeIssues(nodeTypeWithRequiredInput, node, workflow, [
|
||||
'typeUnknown',
|
||||
'parameters',
|
||||
'credentials',
|
||||
'execution',
|
||||
]);
|
||||
|
||||
expect(result?.input?.[NodeConnectionTypes.AiLanguageModel]).toBeDefined();
|
||||
});
|
||||
|
||||
it('reports both unconnected inputs when they share a connection type', () => {
|
||||
// An agent with a fallback declares Chat Model and Fallback Model, both
|
||||
// `ai_languageModel`. Keyed by type, so one must not hide the other.
|
||||
const node = createTestNode({ name: 'Agent', type: 'agent' });
|
||||
const workflow = {
|
||||
getNode: () => node,
|
||||
connectionsByDestinationNode: {},
|
||||
} as unknown as Workflow;
|
||||
|
||||
vi.spyOn(NodeHelpers, 'getNodeInputs').mockReturnValue([
|
||||
{ type: NodeConnectionTypes.AiLanguageModel, displayName: 'Chat Model', required: true },
|
||||
{
|
||||
type: NodeConnectionTypes.AiLanguageModel,
|
||||
displayName: 'Fallback Model',
|
||||
required: true,
|
||||
},
|
||||
]);
|
||||
|
||||
const { getNodeIssues } = useNodeHelpers();
|
||||
const result = getNodeIssues(nodeTypeWithRequiredInput, node, workflow, [
|
||||
'typeUnknown',
|
||||
'parameters',
|
||||
'credentials',
|
||||
'execution',
|
||||
]);
|
||||
|
||||
expect(result?.input?.[NodeConnectionTypes.AiLanguageModel]).toHaveLength(2);
|
||||
});
|
||||
|
||||
it('records the input issue when a parameter change makes an input required', () => {
|
||||
// The NDV calls this after a parameter change, so a newly required input
|
||||
// shows its warning without a workflow reload.
|
||||
const node = createTestNode({ name: 'Agent', type: 'agent' });
|
||||
const gatedType: INodeTypeDescription = {
|
||||
...nodeTypeWithRequiredInput,
|
||||
inputs: `={{ $parameter.hasOutputParser ? [{ type: "${NodeConnectionTypes.AiLanguageModel}", displayName: "Model", required: true }] : [] }}`,
|
||||
};
|
||||
|
||||
mockDocumentStore.getNodeByName = vi.fn().mockReturnValue(node);
|
||||
mockDocumentStore.setNodeIssue = vi.fn();
|
||||
mockDocumentStore.getWorkflowObjectAccessorSnapshot = vi.fn().mockReturnValue({
|
||||
getNode: () => node,
|
||||
connectionsByDestinationNode: {},
|
||||
});
|
||||
mockedStore(useNodeTypesStore).getNodeType = vi.fn().mockReturnValue(gatedType);
|
||||
vi.spyOn(NodeHelpers, 'getNodeInputs').mockReturnValue([
|
||||
{ type: NodeConnectionTypes.AiLanguageModel, displayName: 'Model', required: true },
|
||||
]);
|
||||
|
||||
const { updateNodeInputIssuesByName } = useNodeHelpers();
|
||||
updateNodeInputIssuesByName('Agent');
|
||||
|
||||
expect(mockDocumentStore.setNodeIssue).toHaveBeenCalledWith({
|
||||
node: 'Agent',
|
||||
type: 'input',
|
||||
value: expect.objectContaining({
|
||||
[NodeConnectionTypes.AiLanguageModel]: expect.anything(),
|
||||
}),
|
||||
});
|
||||
});
|
||||
|
||||
it('clears the input issue once the required input is satisfied', () => {
|
||||
const node = createTestNode({ name: 'Agent', type: 'agent' });
|
||||
|
||||
mockDocumentStore.getNodeByName = vi.fn().mockReturnValue(node);
|
||||
mockDocumentStore.setNodeIssue = vi.fn();
|
||||
mockDocumentStore.getWorkflowObjectAccessorSnapshot = vi.fn().mockReturnValue({
|
||||
getNode: (name: string) =>
|
||||
name === 'Model' ? createTestNode({ name: 'Model', type: 'model' }) : node,
|
||||
connectionsByDestinationNode: {
|
||||
Agent: {
|
||||
[NodeConnectionTypes.AiLanguageModel]: [
|
||||
[{ node: 'Model', type: NodeConnectionTypes.AiLanguageModel, index: 0 }],
|
||||
],
|
||||
},
|
||||
},
|
||||
});
|
||||
mockedStore(useNodeTypesStore).getNodeType = vi
|
||||
.fn()
|
||||
.mockReturnValue(nodeTypeWithRequiredInput);
|
||||
vi.spyOn(NodeHelpers, 'getNodeInputs').mockReturnValue([
|
||||
{ type: NodeConnectionTypes.AiLanguageModel, displayName: 'Model', required: true },
|
||||
]);
|
||||
|
||||
const { updateNodeInputIssuesByName } = useNodeHelpers();
|
||||
updateNodeInputIssuesByName('Agent');
|
||||
|
||||
expect(mockDocumentStore.setNodeIssue).toHaveBeenCalledWith({
|
||||
node: 'Agent',
|
||||
type: 'input',
|
||||
value: null,
|
||||
});
|
||||
});
|
||||
|
||||
it('skips the input check when input issues are ignored', () => {
|
||||
// The tool-config panel passes an accessor built from getNode alone and
|
||||
// opts out of input issues. The shared check reads the connection map,
|
||||
// so it must not run against such a partial accessor.
|
||||
const node = createTestNode({ name: 'Agent', type: 'agent' });
|
||||
const partialAccessor = { getNode: () => node } as unknown as Workflow;
|
||||
|
||||
const { getNodeIssues } = useNodeHelpers();
|
||||
const result = getNodeIssues(nodeTypeWithRequiredInput, node, partialAccessor, [
|
||||
'typeUnknown',
|
||||
'parameters',
|
||||
'credentials',
|
||||
'execution',
|
||||
'input',
|
||||
]);
|
||||
|
||||
expect(result?.input).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
describe('credential issues with AI Gateway', () => {
|
||||
const nodeTypeWithCreds: INodeTypeDescription = {
|
||||
displayName: 'Google AI',
|
||||
|
|
|
|||
|
|
@ -10,6 +10,7 @@ import {
|
|||
getChildNodes,
|
||||
getParentNodes,
|
||||
nodeIssuesToString,
|
||||
getUnconnectedRequiredInputs,
|
||||
} from 'n8n-workflow';
|
||||
import type {
|
||||
INodeProperties,
|
||||
|
|
@ -18,7 +19,6 @@ import type {
|
|||
INodeIssues,
|
||||
ICredentialType,
|
||||
INodeIssueObjectProperty,
|
||||
INodeInputConfiguration,
|
||||
INodeExecutionData,
|
||||
ITaskDataConnections,
|
||||
IBinaryKeyData,
|
||||
|
|
@ -227,11 +227,16 @@ export function useNodeHelpers() {
|
|||
}
|
||||
}
|
||||
|
||||
const nodeInputIssues = getNodeInputIssues(workflow, node, nodeType);
|
||||
if (nodeIssues === null) {
|
||||
nodeIssues = nodeInputIssues;
|
||||
} else {
|
||||
NodeHelpers.mergeIssues(nodeIssues, nodeInputIssues);
|
||||
// Honoured like the other kinds: the tool-config panel passes a partial
|
||||
// accessor (getNode only) and opts out of input issues, so the shared
|
||||
// check must not run against it.
|
||||
if (!ignoreIssues.includes('input')) {
|
||||
const nodeInputIssues = getNodeInputIssues(workflow, node, nodeType);
|
||||
if (nodeIssues === null) {
|
||||
nodeIssues = nodeInputIssues;
|
||||
} else {
|
||||
NodeHelpers.mergeIssues(nodeIssues, nodeInputIssues);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -297,6 +302,14 @@ export function useNodeHelpers() {
|
|||
});
|
||||
}
|
||||
|
||||
function updateNodeInputIssuesByName(name: string): void {
|
||||
const node = workflowDocumentStore.value.getNodeByName(name) ?? null;
|
||||
|
||||
if (node) {
|
||||
updateNodeInputIssues(node);
|
||||
}
|
||||
}
|
||||
|
||||
function updateNodesInputIssues() {
|
||||
const nodes = workflowDocumentStore.value.allNodes;
|
||||
|
||||
|
|
@ -391,25 +404,22 @@ export function useNodeHelpers() {
|
|||
const foundIssues: INodeIssueObjectProperty = {};
|
||||
|
||||
const workflowNode = workflow.getNode(node.name);
|
||||
let inputs: Array<NodeConnectionType | INodeInputConfiguration> = [];
|
||||
if (nodeType && workflowNode) {
|
||||
inputs = NodeHelpers.getNodeInputs(workflow, workflowNode, nodeType);
|
||||
}
|
||||
// Detection is shared with the backend publish check; only wording is local.
|
||||
const unconnected =
|
||||
nodeType && workflowNode
|
||||
? getUnconnectedRequiredInputs(workflow, workflowNode, nodeType)
|
||||
: [];
|
||||
|
||||
inputs.forEach((input) => {
|
||||
if (typeof input === 'string' || input.required !== true) {
|
||||
return;
|
||||
}
|
||||
|
||||
const parentNodes = workflow.getParentNodes(node.name, input.type, 1);
|
||||
|
||||
if (parentNodes.length === 0) {
|
||||
foundIssues[input.type] = [
|
||||
i18n.baseText('nodeIssues.input.missing', {
|
||||
interpolate: { inputName: input.displayName || input.type },
|
||||
}),
|
||||
];
|
||||
}
|
||||
unconnected.forEach((input) => {
|
||||
// Accumulate: a node can leave two inputs of one type unconnected, e.g.
|
||||
// an agent's Chat Model and Fallback Model are both `ai_languageModel`.
|
||||
// Assigning would drop all but the last.
|
||||
foundIssues[input.type] = [
|
||||
...(foundIssues[input.type] ?? []),
|
||||
i18n.baseText('nodeIssues.input.missing', {
|
||||
interpolate: { inputName: input.displayName || input.type },
|
||||
}),
|
||||
];
|
||||
});
|
||||
|
||||
if (Object.keys(foundIssues).length) {
|
||||
|
|
@ -1096,6 +1106,7 @@ export function useNodeHelpers() {
|
|||
updateNodesExecutionIssues,
|
||||
updateNodesParameterIssues,
|
||||
updateNodeInputIssues,
|
||||
updateNodeInputIssuesByName,
|
||||
updateNodeCredentialIssuesByName,
|
||||
updateNodeCredentialIssues,
|
||||
updateNodeParameterIssuesByName,
|
||||
|
|
|
|||
|
|
@ -72,7 +72,6 @@ const { mockDocumentStore } = vi.hoisted(() => {
|
|||
getPinnedDataLastUpdate: vi.fn(),
|
||||
getPinnedDataLastRemovedAt: vi.fn(),
|
||||
getWorkflowObjectAccessorSnapshot: vi.fn(),
|
||||
hasNodeValidationIssues: false,
|
||||
nodeValidationIssues: [],
|
||||
serialize: vi.fn(),
|
||||
} as Partial<Mocked<Writable<ReturnType<typeof useWorkflowDocumentStore>>>> as Mocked<
|
||||
|
|
@ -363,15 +362,14 @@ describe('useRunWorkflow({ router })', () => {
|
|||
|
||||
it('should not prevent running a webhook-based workflow that has issues', async () => {
|
||||
const { runWorkflowApi } = useRunWorkflow({ router });
|
||||
mockDocumentStore.hasNodeValidationIssues = true;
|
||||
// Nothing here gates a run on the document store's issue flags, so there is
|
||||
// no flag to set. The point stands: a webhook-based run still resolves.
|
||||
vi.mocked(workflowsStore).runWorkflow.mockResolvedValue({
|
||||
executionId: '123',
|
||||
waitingForWebhook: true,
|
||||
});
|
||||
|
||||
await expect(runWorkflowApi({} as IStartRunData)).resolves.not.toThrow();
|
||||
|
||||
mockDocumentStore.hasNodeValidationIssues = false;
|
||||
});
|
||||
|
||||
it('should handle workflow run failure', async () => {
|
||||
|
|
@ -415,7 +413,6 @@ describe('useRunWorkflow({ router })', () => {
|
|||
|
||||
vi.mocked(uiStore).activeActions = [''];
|
||||
vi.mocked(workflowsStore).runWorkflow.mockResolvedValue(mockExecutionResponse);
|
||||
mockDocumentStore.hasNodeValidationIssues = true;
|
||||
mockDocumentStore.serialize.mockReturnValue({
|
||||
id: 'workflowId',
|
||||
nodes: [],
|
||||
|
|
@ -432,7 +429,6 @@ describe('useRunWorkflow({ router })', () => {
|
|||
|
||||
vi.mocked(pushConnectionStore).isConnected = true;
|
||||
vi.mocked(workflowsStore).runWorkflow.mockResolvedValue(mockExecutionResponse);
|
||||
mockDocumentStore.hasNodeValidationIssues = false;
|
||||
mockDocumentStore.serialize.mockReturnValue({
|
||||
id: 'workflowId',
|
||||
nodes: [],
|
||||
|
|
@ -679,7 +675,6 @@ describe('useRunWorkflow({ router })', () => {
|
|||
|
||||
vi.mocked(pushConnectionStore).isConnected = true;
|
||||
vi.mocked(workflowsStore).runWorkflow.mockResolvedValue(mockExecutionResponse);
|
||||
mockDocumentStore.hasNodeValidationIssues = false;
|
||||
mockDocumentStore.serialize.mockReturnValue({
|
||||
id: 'workflowId',
|
||||
nodes: [],
|
||||
|
|
@ -919,7 +914,6 @@ describe('useRunWorkflow({ router })', () => {
|
|||
|
||||
vi.mocked(pushConnectionStore).isConnected = true;
|
||||
vi.mocked(workflowsStore).runWorkflow.mockResolvedValue(mockExecutionResponse);
|
||||
mockDocumentStore.hasNodeValidationIssues = false;
|
||||
mockDocumentStore.serialize.mockReturnValue(workflowData);
|
||||
seedActiveRunData(mockRunData);
|
||||
vi.mocked(agentRequestStore).getAgentRequest.mockReturnValue(agentRequest);
|
||||
|
|
@ -980,7 +974,6 @@ describe('useRunWorkflow({ router })', () => {
|
|||
vi.mocked(pushConnectionStore).isConnected = true;
|
||||
vi.mocked(workflowsStore).runWorkflow.mockResolvedValue({ executionId: '123' });
|
||||
|
||||
mockDocumentStore.hasNodeValidationIssues = false;
|
||||
mockDocumentStore.serialize.mockReturnValue({
|
||||
id: 'workflowId',
|
||||
nodes: [createTestNode({ id: currentNodeId, name: 'Test node' })],
|
||||
|
|
@ -1024,7 +1017,6 @@ describe('useRunWorkflow({ router })', () => {
|
|||
|
||||
vi.mocked(pushConnectionStore).isConnected = true;
|
||||
vi.mocked(workflowsStore).runWorkflow.mockResolvedValue(mockExecutionResponse);
|
||||
mockDocumentStore.hasNodeValidationIssues = false;
|
||||
mockDocumentStore.serialize.mockReturnValue(
|
||||
mock<WorkflowData>({ id: 'workflowId', nodes: [] }),
|
||||
);
|
||||
|
|
@ -1058,7 +1050,6 @@ describe('useRunWorkflow({ router })', () => {
|
|||
|
||||
vi.mocked(pushConnectionStore).isConnected = true;
|
||||
vi.mocked(workflowsStore).runWorkflow.mockResolvedValue(mockExecutionResponse);
|
||||
mockDocumentStore.hasNodeValidationIssues = false;
|
||||
mockDocumentStore.serialize.mockReturnValue(
|
||||
mock<WorkflowData>({ id: 'workflowId', nodes: [] }),
|
||||
);
|
||||
|
|
@ -1113,7 +1104,6 @@ describe('useRunWorkflow({ router })', () => {
|
|||
beforeEach(() => {
|
||||
vi.mocked(pushConnectionStore).isConnected = true;
|
||||
vi.mocked(workflowsStore).runWorkflow.mockResolvedValue({ executionId: 'exec-123' });
|
||||
mockDocumentStore.hasNodeValidationIssues = false;
|
||||
mockDocumentStore.checkIfNodeHasChatParent.mockReturnValue(false);
|
||||
mockDocumentStore.checkIfToolNodeHasChatParent.mockReturnValue(false);
|
||||
});
|
||||
|
|
@ -1173,7 +1163,6 @@ describe('useRunWorkflow({ router })', () => {
|
|||
beforeEach(() => {
|
||||
vi.mocked(pushConnectionStore).isConnected = true;
|
||||
vi.mocked(workflowsStore).runWorkflow.mockResolvedValue(mockExecutionResponse);
|
||||
mockDocumentStore.hasNodeValidationIssues = false;
|
||||
seedActiveRunData({ NodeName: [] });
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -28,13 +28,17 @@ import type { INodeUi, IWorkflowDb } from '@/Interface';
|
|||
import type { ProjectSharingData } from '@/features/collaboration/projects/projects.types';
|
||||
import type { IUsedCredential } from '@/features/credentials/credentials.types';
|
||||
|
||||
const { getNodeTypeMock } = vi.hoisted(() => ({
|
||||
const { getNodeTypeMock, isTriggerNodeMock } = vi.hoisted(() => ({
|
||||
getNodeTypeMock: vi.fn().mockReturnValue(null),
|
||||
// The publish gate asks which nodes start a run. Anything named like a trigger
|
||||
// counts, which keeps these tests readable without a node-type registry.
|
||||
isTriggerNodeMock: vi.fn((type: string) => type.toLowerCase().includes('trigger')),
|
||||
}));
|
||||
|
||||
vi.mock('@/app/stores/nodeTypes.store', () => ({
|
||||
useNodeTypesStore: vi.fn(() => ({
|
||||
getNodeType: getNodeTypeMock,
|
||||
isTriggerNode: isTriggerNodeMock,
|
||||
communityNodeType: vi.fn().mockReturnValue(null),
|
||||
getAllNodeTypes: vi.fn().mockReturnValue({
|
||||
nodeTypes: {},
|
||||
|
|
@ -205,8 +209,8 @@ describe('workflowDocument.store orchestration', () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe('hasNodeValidationIssues', () => {
|
||||
it('should return true when a node has issues and connected', () => {
|
||||
describe('nodeValidationIssues', () => {
|
||||
it('should report a node that has issues and is connected', () => {
|
||||
const workflowDocumentStore = useWorkflowDocumentStore(createWorkflowDocumentId('test-wf'));
|
||||
|
||||
workflowDocumentStore.setNodes([
|
||||
|
|
@ -218,11 +222,10 @@ describe('workflowDocument.store orchestration', () => {
|
|||
Node1: { main: [[{ node: 'Node2', type: NodeConnectionTypes.Main, index: 0 }]] },
|
||||
});
|
||||
|
||||
const hasIssues = workflowDocumentStore.hasNodeValidationIssues;
|
||||
expect(hasIssues).toBe(true);
|
||||
expect(workflowDocumentStore.nodeValidationIssues.length).toBeGreaterThan(0);
|
||||
});
|
||||
|
||||
it('should return false when node has issues but it is not connected', () => {
|
||||
it('should report nothing when the node with issues is not connected', () => {
|
||||
const workflowDocumentStore = useWorkflowDocumentStore(createWorkflowDocumentId('test-wf'));
|
||||
|
||||
workflowDocumentStore.setNodes([
|
||||
|
|
@ -230,11 +233,10 @@ describe('workflowDocument.store orchestration', () => {
|
|||
createNode({ name: 'Node2' }),
|
||||
]);
|
||||
|
||||
const hasIssues = workflowDocumentStore.hasNodeValidationIssues;
|
||||
expect(hasIssues).toBe(false);
|
||||
expect(workflowDocumentStore.nodeValidationIssues).toEqual([]);
|
||||
});
|
||||
|
||||
it('should return false when no nodes have issues', () => {
|
||||
it('should report nothing when no nodes have issues', () => {
|
||||
const workflowDocumentStore = useWorkflowDocumentStore(createWorkflowDocumentId('test-wf'));
|
||||
|
||||
workflowDocumentStore.setNodes([
|
||||
|
|
@ -245,17 +247,50 @@ describe('workflowDocument.store orchestration', () => {
|
|||
Node1: { main: [[{ node: 'Node2', type: NodeConnectionTypes.Main, index: 0 }]] },
|
||||
});
|
||||
|
||||
const hasIssues = workflowDocumentStore.hasNodeValidationIssues;
|
||||
expect(hasIssues).toBe(false);
|
||||
expect(workflowDocumentStore.nodeValidationIssues).toEqual([]);
|
||||
});
|
||||
|
||||
it('should return false when there are no nodes', () => {
|
||||
it('should report nothing when there are no nodes', () => {
|
||||
const workflowDocumentStore = useWorkflowDocumentStore(createWorkflowDocumentId('test-wf'));
|
||||
|
||||
workflowDocumentStore.setNodes([]);
|
||||
|
||||
const hasIssues = workflowDocumentStore.hasNodeValidationIssues;
|
||||
expect(hasIssues).toBe(false);
|
||||
expect(workflowDocumentStore.nodeValidationIssues).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('hasPublishBlockingIssues', () => {
|
||||
it('should block when a trigger reaches the node with issues', () => {
|
||||
const workflowDocumentStore = useWorkflowDocumentStore(createWorkflowDocumentId('test-wf'));
|
||||
|
||||
workflowDocumentStore.setNodes([
|
||||
createNode({ name: 'My trigger', type: 'n8n-nodes-base.scheduleTrigger' }),
|
||||
createNode({ name: 'Node1', issues: { parameters: { field: ['Error message'] } } }),
|
||||
]);
|
||||
workflowDocumentStore.setConnections({
|
||||
'My trigger': { main: [[{ node: 'Node1', type: NodeConnectionTypes.Main, index: 0 }]] },
|
||||
});
|
||||
|
||||
expect(workflowDocumentStore.hasPublishBlockingIssues).toBe(true);
|
||||
});
|
||||
|
||||
it('should not block when the node with issues sits on an island', () => {
|
||||
const workflowDocumentStore = useWorkflowDocumentStore(createWorkflowDocumentId('test-wf'));
|
||||
|
||||
// Node1 and Node2 are wired to each other, so connectedness would call
|
||||
// Node1 live. No trigger reaches them, so no run can involve them.
|
||||
workflowDocumentStore.setNodes([
|
||||
createNode({ name: 'My trigger', type: 'n8n-nodes-base.scheduleTrigger' }),
|
||||
createNode({ name: 'Reached' }),
|
||||
createNode({ name: 'Node1', issues: { parameters: { field: ['Error message'] } } }),
|
||||
createNode({ name: 'Node2' }),
|
||||
]);
|
||||
workflowDocumentStore.setConnections({
|
||||
'My trigger': { main: [[{ node: 'Reached', type: NodeConnectionTypes.Main, index: 0 }]] },
|
||||
Node1: { main: [[{ node: 'Node2', type: NodeConnectionTypes.Main, index: 0 }]] },
|
||||
});
|
||||
|
||||
expect(workflowDocumentStore.hasPublishBlockingIssues).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -39,7 +39,7 @@ import { assignNodeId, serializeNode } from '@/app/utils/nodes/nodeTransforms';
|
|||
import type { WorkflowObjectAccessors } from '../types';
|
||||
import type { IWorkflowDb } from '@/Interface';
|
||||
import type { INode, ProjectSharingData } from 'n8n-workflow';
|
||||
import { deepCopy, nodeIssuesToString } from 'n8n-workflow';
|
||||
import { deepCopy, nodeIssuesToString, NodeConnectionTypes, NodeHelpers } from 'n8n-workflow';
|
||||
import type { WorkflowData } from '@n8n/rest-api-client/api/workflows';
|
||||
import type { Scope } from '@n8n/permissions';
|
||||
import type { IUsedCredential } from '@/features/credentials/credentials.types';
|
||||
|
|
@ -251,6 +251,32 @@ export function useWorkflowDocumentStore(id: WorkflowDocumentId) {
|
|||
allNodes: workflowDocumentNodes.allNodes,
|
||||
outgoingConnectionsByNodeName: workflowDocumentConnections.outgoingConnectionsByNodeName,
|
||||
incomingConnectionsByNodeName: workflowDocumentConnections.incomingConnectionsByNodeName,
|
||||
connectionsBySourceNode: workflowDocumentConnections.connectionsBySourceNode,
|
||||
connectionsByDestinationNode: workflowDocumentConnections.connectionsByDestinationNode,
|
||||
// The editor only has the node description, so it reads the `trigger` group
|
||||
// where the server reads the type's trigger/webhook/poll implementation.
|
||||
isTriggerLike: (node) => nodeTypesStore.isTriggerNode(node.type),
|
||||
canOutputMain: (node) => {
|
||||
const nodeType = nodeTypesStore.getNodeType(node.type, node.typeVersion);
|
||||
// Unknown type answers "yes", so a half-built graph is never written off
|
||||
// as unable to affect a run.
|
||||
if (!nodeType) return true;
|
||||
try {
|
||||
// Evaluated, not read off the description: several vector stores build
|
||||
// `outputs` from an expression, and reading the raw expression would
|
||||
// call them main-path nodes. The server evaluates it the same way.
|
||||
return NodeHelpers.getNodeOutputs(
|
||||
{ expression: workflowDocumentExpression.getExpressionHandler() },
|
||||
node,
|
||||
nodeType,
|
||||
).some(
|
||||
(output) =>
|
||||
(typeof output === 'string' ? output : output.type) === NodeConnectionTypes.Main,
|
||||
);
|
||||
} catch {
|
||||
return true;
|
||||
}
|
||||
},
|
||||
nodesById: workflowDocumentNodes.nodesById,
|
||||
onNodesChange: workflowDocumentNodes.onNodesChange,
|
||||
nodeIssuesToString,
|
||||
|
|
@ -418,6 +444,8 @@ export function useWorkflowDocumentStore(id: WorkflowDocumentId) {
|
|||
return {
|
||||
id: workflowId,
|
||||
connectionsBySourceNode: workflowDocumentConnections.connectionsBySourceNode.value,
|
||||
connectionsByDestinationNode:
|
||||
workflowDocumentConnections.connectionsByDestinationNode.value,
|
||||
pinData: workflowDocumentPinData.getPinDataSnapshot(),
|
||||
expression: workflowDocumentExpression.getExpressionHandler(),
|
||||
getNode: workflowDocumentNodes.getNodeByName,
|
||||
|
|
|
|||
|
|
@ -2,7 +2,7 @@ import { describe, it, expect } from 'vitest';
|
|||
import { computed, shallowRef } from 'vue';
|
||||
import { createTestNode } from '@/__tests__/mocks';
|
||||
import type { INodeUi } from '@/Interface';
|
||||
import type { INodeConnections } from 'n8n-workflow';
|
||||
import type { IConnections, INodeConnections } from 'n8n-workflow';
|
||||
import {
|
||||
useWorkflowDocumentNodesIssues,
|
||||
type WorkflowDocumentNodesIssuesDeps,
|
||||
|
|
@ -11,38 +11,100 @@ import {
|
|||
const connectedNode = (overrides: Partial<INodeUi> = {}): INodeUi =>
|
||||
createTestNode({ name: 'Node', ...overrides }) as INodeUi;
|
||||
|
||||
/** `createTestNode` takes a `Partial<INode>`, which has no `issues`. */
|
||||
const nodeWithIssues = (name: string, issues: INodeUi['issues']): INodeUi =>
|
||||
createTestNode({ name, ...{ issues } }) as INodeUi;
|
||||
|
||||
const hasConnections: () => INodeConnections = () => ({
|
||||
main: [[{ node: 'Other', type: 'main' as const, index: 0 }]],
|
||||
});
|
||||
const noConnections: () => INodeConnections = () => ({});
|
||||
/** What the canvas leaves behind after the last link of a type is deleted. */
|
||||
const emptyConnectionEntry: () => INodeConnections = () => ({ ai_outputParser: [[]] });
|
||||
/** And the same with the output slot nulled out rather than emptied. */
|
||||
const nulledConnectionEntry: () => INodeConnections = () => ({ ai_outputParser: [null] });
|
||||
|
||||
function createDeps(nodes: INodeUi[], connected = true): WorkflowDocumentNodesIssuesDeps {
|
||||
type DepOverrides = Partial<WorkflowDocumentNodesIssuesDeps>;
|
||||
|
||||
function createDeps(
|
||||
nodes: INodeUi[],
|
||||
connected = true,
|
||||
overrides: DepOverrides = {},
|
||||
): WorkflowDocumentNodesIssuesDeps {
|
||||
return {
|
||||
allNodes: computed(() => nodes),
|
||||
outgoingConnectionsByNodeName: connected ? hasConnections : noConnections,
|
||||
incomingConnectionsByNodeName: noConnections,
|
||||
connectionsBySourceNode: computed<IConnections>(() => ({})),
|
||||
connectionsByDestinationNode: computed<IConnections>(() => ({})),
|
||||
// Default: everything is a trigger, so every node is reachable and the
|
||||
// per-issue-kind rules are what the assertions actually exercise.
|
||||
isTriggerLike: () => true,
|
||||
canOutputMain: () => true,
|
||||
nodesById: shallowRef(new Map(nodes.map((n) => [n.id, n]))),
|
||||
onNodesChange: () => {},
|
||||
nodeIssuesToString: () => [],
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
/** Trigger -> Agent over `main`, with `subNode` supplying the agent over `ai_outputParser`. */
|
||||
function subNodeGraph({ agentDisabled = false } = {}) {
|
||||
const trigger = createTestNode({ name: 'Trigger', type: 'n8n-nodes-base.scheduleTrigger' });
|
||||
const agent = createTestNode({
|
||||
name: 'Agent',
|
||||
type: '@n8n/n8n-nodes-langchain.agent',
|
||||
disabled: agentDisabled,
|
||||
});
|
||||
const subNode = nodeWithIssues('Parser', {
|
||||
input: { ai_languageModel: ['Model input is not connected'] },
|
||||
});
|
||||
|
||||
const connectionsBySourceNode: IConnections = {
|
||||
Trigger: { main: [[{ node: 'Agent', type: 'main', index: 0 }]] },
|
||||
Parser: { ai_outputParser: [[{ node: 'Agent', type: 'ai_outputParser', index: 0 }]] },
|
||||
};
|
||||
const connectionsByDestinationNode: IConnections = {
|
||||
Agent: {
|
||||
main: [[{ node: 'Trigger', type: 'main', index: 0 }]],
|
||||
ai_outputParser: [[{ node: 'Parser', type: 'ai_outputParser', index: 0 }]],
|
||||
},
|
||||
};
|
||||
|
||||
return {
|
||||
nodes: [trigger, agent, subNode] as INodeUi[],
|
||||
overrides: {
|
||||
connectionsBySourceNode: computed(() => connectionsBySourceNode),
|
||||
connectionsByDestinationNode: computed(() => connectionsByDestinationNode),
|
||||
isTriggerLike: (node: INodeUi) => node.name === 'Trigger',
|
||||
// Only the agent sits on the main path; the parser just supplies it.
|
||||
canOutputMain: (node: INodeUi) => node.name !== 'Parser',
|
||||
} satisfies DepOverrides,
|
||||
};
|
||||
}
|
||||
|
||||
describe('useWorkflowDocumentNodesIssues', () => {
|
||||
describe('hasNodeValidationIssues', () => {
|
||||
it('includes execution issues', () => {
|
||||
const node = connectedNode({ issues: { execution: true } });
|
||||
const { hasNodeValidationIssues } = useWorkflowDocumentNodesIssues(createDeps([node]));
|
||||
describe('nodeValidationIssues', () => {
|
||||
// Feeds the assistant's build todos, which still ask whether a node is wired
|
||||
// to anything rather than whether a run can reach it.
|
||||
it('includes a connected node with parameter issues', () => {
|
||||
const node = connectedNode({ issues: { parameters: { param1: ['Missing value'] } } });
|
||||
const { nodeValidationIssues } = useWorkflowDocumentNodesIssues(createDeps([node]));
|
||||
|
||||
expect(hasNodeValidationIssues.value).toBe(true);
|
||||
expect(nodeValidationIssues.value.length).toBeGreaterThan(0);
|
||||
});
|
||||
|
||||
it('includes parameter issues', () => {
|
||||
const node = connectedNode({
|
||||
issues: { parameters: { param1: ['Missing value'] } },
|
||||
it.each([
|
||||
['an output slot with no endpoints', emptyConnectionEntry],
|
||||
['an output slot that is null', nulledConnectionEntry],
|
||||
])('skips a node left with %s', (_label, connections) => {
|
||||
const node = connectedNode({ issues: { parameters: { param1: ['Missing value'] } } });
|
||||
const { nodeValidationIssues } = useWorkflowDocumentNodesIssues({
|
||||
...createDeps([node]),
|
||||
outgoingConnectionsByNodeName: connections,
|
||||
});
|
||||
const { hasNodeValidationIssues } = useWorkflowDocumentNodesIssues(createDeps([node]));
|
||||
|
||||
expect(hasNodeValidationIssues.value).toBe(true);
|
||||
expect(nodeValidationIssues.value).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
|
|
@ -80,5 +142,81 @@ describe('useWorkflowDocumentNodesIssues', () => {
|
|||
|
||||
expect(hasPublishBlockingIssues.value).toBe(true);
|
||||
});
|
||||
|
||||
it('does not block on a node no trigger can reach', () => {
|
||||
// Wired to another node, so connectedness would call it live, but there is
|
||||
// no trigger anywhere in the graph.
|
||||
const island = nodeWithIssues('Island Parser', {
|
||||
input: { ai_languageModel: ['Model input is not connected'] },
|
||||
});
|
||||
const neighbour = createTestNode({ name: 'Island Agent' }) as INodeUi;
|
||||
|
||||
const connections: IConnections = {
|
||||
'Island Parser': {
|
||||
ai_outputParser: [[{ node: 'Island Agent', type: 'ai_outputParser', index: 0 }]],
|
||||
},
|
||||
};
|
||||
|
||||
const { hasPublishBlockingIssues } = useWorkflowDocumentNodesIssues(
|
||||
createDeps([island, neighbour], true, {
|
||||
connectionsBySourceNode: computed(() => connections),
|
||||
connectionsByDestinationNode: computed<IConnections>(() => ({
|
||||
'Island Agent': {
|
||||
ai_outputParser: [[{ node: 'Island Parser', type: 'ai_outputParser', index: 0 }]],
|
||||
},
|
||||
})),
|
||||
isTriggerLike: () => false,
|
||||
}),
|
||||
);
|
||||
|
||||
expect(hasPublishBlockingIssues.value).toBe(false);
|
||||
});
|
||||
|
||||
it('blocks on an unmet input when the node it supplies runs', () => {
|
||||
const { nodes, overrides } = subNodeGraph();
|
||||
const { hasPublishBlockingIssues } = useWorkflowDocumentNodesIssues(
|
||||
createDeps(nodes, true, overrides),
|
||||
);
|
||||
|
||||
expect(hasPublishBlockingIssues.value).toBe(true);
|
||||
});
|
||||
|
||||
it('does not block on an unmet input when the only node it supplies is disabled', () => {
|
||||
const { nodes, overrides } = subNodeGraph({ agentDisabled: true });
|
||||
const { hasPublishBlockingIssues } = useWorkflowDocumentNodesIssues(
|
||||
createDeps(nodes, true, overrides),
|
||||
);
|
||||
|
||||
expect(hasPublishBlockingIssues.value).toBe(false);
|
||||
});
|
||||
|
||||
it('still blocks on a credential issue behind a disabled consumer', () => {
|
||||
// The inert-subnode exemption is for unmet inputs only. A credential is the
|
||||
// node's own problem, and the server blocks on it, so the button must too.
|
||||
const { nodes, overrides } = subNodeGraph({ agentDisabled: true });
|
||||
const parser = nodes.find((node) => node.name === 'Parser') as INodeUi;
|
||||
parser.issues = { credentials: { cred1: ['Not set'] } };
|
||||
|
||||
const { hasPublishBlockingIssues } = useWorkflowDocumentNodesIssues(
|
||||
createDeps(nodes, true, overrides),
|
||||
);
|
||||
|
||||
expect(hasPublishBlockingIssues.value).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe('publishBlockingNodes', () => {
|
||||
it('lists only the nodes that block, so the count matches the button', () => {
|
||||
const { nodes, overrides } = subNodeGraph();
|
||||
const unreachable = nodeWithIssues('Lone Parser', {
|
||||
input: { ai_languageModel: ['Model input is not connected'] },
|
||||
});
|
||||
|
||||
const { publishBlockingNodes } = useWorkflowDocumentNodesIssues(
|
||||
createDeps([...nodes, unreachable], true, overrides),
|
||||
);
|
||||
|
||||
expect(publishBlockingNodes.value.map((node) => node.name)).toEqual(['Parser']);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -2,7 +2,8 @@ import { computed, effectScope, shallowReactive, type ComputedRef, type ShallowR
|
|||
import { structuralComputed } from '@n8n/composables/structuralComputed';
|
||||
import isEqual from 'lodash/isEqual';
|
||||
import type { INodeUi, WorkflowValidationIssue } from '@/Interface';
|
||||
import type { INodeConnections, INodeIssues, INode } from 'n8n-workflow';
|
||||
import type { IConnections, INodeConnections, INodeIssues, INode } from 'n8n-workflow';
|
||||
import { getReachableNodeNames, onlySuppliesDisabledNodes } from 'n8n-workflow';
|
||||
import { CHANGE_ACTION } from './types';
|
||||
import type {
|
||||
NodeAddedPayload,
|
||||
|
|
@ -15,41 +16,81 @@ export type WorkflowDocumentNodesIssuesDeps = {
|
|||
allNodes: ComputedRef<INodeUi[]>;
|
||||
outgoingConnectionsByNodeName: (nodeName: string) => INodeConnections;
|
||||
incomingConnectionsByNodeName: (nodeName: string) => INodeConnections;
|
||||
connectionsBySourceNode: ComputedRef<IConnections>;
|
||||
connectionsByDestinationNode: ComputedRef<IConnections>;
|
||||
/** Whether this node starts a run. */
|
||||
isTriggerLike: (node: INodeUi) => boolean;
|
||||
/** Whether this node can produce a `main` output. Answers `true` when unsure. */
|
||||
canOutputMain: (node: INodeUi) => boolean;
|
||||
nodesById: ShallowRef<Map<string, INodeUi>>;
|
||||
onNodesChange: (cb: (event: NodesChangeEvent) => void) => void;
|
||||
nodeIssuesToString: (issues: INodeIssues, node?: INode) => string[];
|
||||
};
|
||||
|
||||
/**
|
||||
* Whether anything is actually wired to this node.
|
||||
*
|
||||
* A connection type can outlive its links: deleting the last connection leaves
|
||||
* `{ ai_outputParser: [[]] }` behind, so counting the type keys reports a node
|
||||
* wired to nothing as connected. Count the endpoints instead.
|
||||
*/
|
||||
function hasAnyConnection(connections: INodeConnections): boolean {
|
||||
return Object.values(connections).some((outputs) =>
|
||||
(outputs ?? []).some((targets) => (targets ?? []).length > 0),
|
||||
);
|
||||
}
|
||||
|
||||
export function useWorkflowDocumentNodesIssues(deps: WorkflowDocumentNodesIssuesDeps) {
|
||||
const nodesWithValidationIssues = computed<INodeUi[]>(() =>
|
||||
deps.allNodes.value.filter((node) => {
|
||||
const nodeHasIssues = Object.keys(node.issues ?? {}).length > 0;
|
||||
const isConnected =
|
||||
Object.keys(deps.outgoingConnectionsByNodeName(node.name)).length > 0 ||
|
||||
Object.keys(deps.incomingConnectionsByNodeName(node.name)).length > 0;
|
||||
/**
|
||||
* Nodes that stop the workflow being published, which is also what the
|
||||
* blocked-publish message counts, so the button and its reason always agree.
|
||||
*
|
||||
* Mirrors the server (`workflow-validation.service.ts`): a node only blocks if
|
||||
* a trigger can reach it, and an unmet input only blocks if something that
|
||||
* runs will ask for it. A node no run can reach cannot break one, so it must
|
||||
* not stop publishing. It keeps its own warning on the canvas either way,
|
||||
* which is what tells the user it is still unfinished.
|
||||
*/
|
||||
const publishBlockingNodes = computed<INodeUi[]>(() => {
|
||||
const nodes = deps.allNodes.value;
|
||||
const reachable = getReachableNodeNames(
|
||||
nodes,
|
||||
deps.connectionsBySourceNode.value,
|
||||
deps.connectionsByDestinationNode.value,
|
||||
deps.isTriggerLike,
|
||||
);
|
||||
|
||||
return !node.disabled && isConnected && nodeHasIssues;
|
||||
}),
|
||||
);
|
||||
return nodes.filter((node) => {
|
||||
if (node.disabled) return false;
|
||||
|
||||
const nodesWithValidationIssuesCount = computed(() => nodesWithValidationIssues.value.length);
|
||||
|
||||
const hasNodeValidationIssues = computed(() => nodesWithValidationIssuesCount.value > 0);
|
||||
|
||||
/** Whether any connected node has issues that should block publishing.
|
||||
* Execution issues are excluded — they are runtime errors, not configuration problems. */
|
||||
const hasPublishBlockingIssues = computed(() =>
|
||||
deps.allNodes.value.some((node) => {
|
||||
// Execution issues are runtime errors, not configuration problems.
|
||||
const { execution: _, ...configIssues } = node.issues ?? {};
|
||||
if (Object.keys(configIssues).length === 0) return false;
|
||||
const issueKinds = Object.keys(configIssues);
|
||||
if (issueKinds.length === 0) return false;
|
||||
|
||||
const isConnected =
|
||||
Object.keys(deps.outgoingConnectionsByNodeName(node.name)).length > 0 ||
|
||||
Object.keys(deps.incomingConnectionsByNodeName(node.name)).length > 0;
|
||||
if (!reachable.has(node.name)) return false;
|
||||
|
||||
return !node.disabled && isConnected;
|
||||
}),
|
||||
);
|
||||
// An unmet input only matters once something asks for it. Credential and
|
||||
// parameter issues belong to the node itself, so those block as soon as a
|
||||
// run can reach it.
|
||||
if (
|
||||
issueKinds.every((kind) => kind === 'input') &&
|
||||
!deps.isTriggerLike(node) &&
|
||||
onlySuppliesDisabledNodes(
|
||||
node,
|
||||
deps.connectionsBySourceNode.value,
|
||||
nodes,
|
||||
deps.canOutputMain,
|
||||
)
|
||||
) {
|
||||
return false;
|
||||
}
|
||||
|
||||
return true;
|
||||
});
|
||||
});
|
||||
|
||||
const hasPublishBlockingIssues = computed(() => publishBlockingNodes.value.length > 0);
|
||||
|
||||
const nodeValidationIssues = computed(() => {
|
||||
const issues: WorkflowValidationIssue[] = [];
|
||||
|
|
@ -61,8 +102,8 @@ export function useWorkflowDocumentNodesIssues(deps: WorkflowDocumentNodesIssues
|
|||
if (!node.issues || node.disabled) return;
|
||||
|
||||
const isConnected =
|
||||
Object.keys(deps.outgoingConnectionsByNodeName(node.name)).length > 0 ||
|
||||
Object.keys(deps.incomingConnectionsByNodeName(node.name)).length > 0;
|
||||
hasAnyConnection(deps.outgoingConnectionsByNodeName(node.name)) ||
|
||||
hasAnyConnection(deps.incomingConnectionsByNodeName(node.name));
|
||||
|
||||
if (!isConnected) return;
|
||||
|
||||
|
|
@ -164,9 +205,7 @@ export function useWorkflowDocumentNodesIssues(deps: WorkflowDocumentNodesIssues
|
|||
applyReconcileEntries(Array.from(deps.nodesById.value.keys()));
|
||||
|
||||
return {
|
||||
nodesWithValidationIssues,
|
||||
nodesWithValidationIssuesCount,
|
||||
hasNodeValidationIssues,
|
||||
publishBlockingNodes,
|
||||
hasPublishBlockingIssues,
|
||||
nodeValidationIssues,
|
||||
validationErrorsByNodeId,
|
||||
|
|
|
|||
|
|
@ -8,6 +8,7 @@ export type WorkflowObjectAccessors = Pick<
|
|||
Workflow,
|
||||
| 'id'
|
||||
| 'connectionsBySourceNode'
|
||||
| 'connectionsByDestinationNode'
|
||||
| 'expression'
|
||||
| 'pinData'
|
||||
| 'getNode'
|
||||
|
|
|
|||
|
|
@ -447,6 +447,7 @@ const valueChanged = (parameterData: IUpdateInformation) => {
|
|||
|
||||
nodeHelpers.updateNodeParameterIssuesByName(_node.name);
|
||||
nodeHelpers.updateNodeCredentialIssuesByName(_node.name);
|
||||
nodeHelpers.updateNodeInputIssuesByName(_node.name);
|
||||
}
|
||||
} else if (nameIsParameter(parameterData)) {
|
||||
// A node parameter changed
|
||||
|
|
|
|||
|
|
@ -171,6 +171,8 @@ export function useNodeSettingsParameters() {
|
|||
|
||||
nodeHelpers.updateNodeParameterIssuesByName(node.name);
|
||||
nodeHelpers.updateNodeCredentialIssuesByName(node.name);
|
||||
// A parameter can change which inputs the node exposes, e.g. `autoFix`.
|
||||
nodeHelpers.updateNodeInputIssuesByName(node.name);
|
||||
telemetry.trackNodeParametersValuesChange(nodeTypeDescription.name, parameterData);
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -63,9 +63,9 @@ const isReattempt = computed(
|
|||
workflowDocumentStore.value.publicationStatus === 'failed',
|
||||
);
|
||||
|
||||
const nodesWithValidationIssues = computed(
|
||||
() => workflowDocumentStore.value.nodesWithValidationIssues,
|
||||
);
|
||||
// The nodes that actually block publishing, so this list and the Publish button
|
||||
// cannot disagree about what needs fixing.
|
||||
const nodesWithValidationIssues = computed(() => workflowDocumentStore.value.publishBlockingNodes);
|
||||
|
||||
const hasNodeIssues = computed(() => workflowDocumentStore.value.hasPublishBlockingIssues);
|
||||
|
||||
|
|
|
|||
|
|
@ -0,0 +1,338 @@
|
|||
import type { IWorkflowBase } from 'n8n-workflow';
|
||||
import { nanoid } from 'nanoid';
|
||||
|
||||
import { test, expect } from '../../../fixtures/base';
|
||||
|
||||
const AGENT = '@n8n/n8n-nodes-langchain.agent';
|
||||
const PARSER = '@n8n/n8n-nodes-langchain.outputParserAutofixing';
|
||||
const LANGCHAIN_CODE = '@n8n/n8n-nodes-langchain.code';
|
||||
const VECTOR_STORE_TOOL = '@n8n/n8n-nodes-langchain.toolVectorStore';
|
||||
const IN_MEMORY_VECTOR_STORE = '@n8n/n8n-nodes-langchain.vectorStoreInMemory';
|
||||
const STRUCTURED_PARSER = '@n8n/n8n-nodes-langchain.outputParserStructured';
|
||||
|
||||
/**
|
||||
* A schedule trigger feeding a No-Op, plus an agent with an autofixing parser
|
||||
* attached. Neither the agent nor the parser has a language model, so both
|
||||
* declare an unmet required input.
|
||||
*
|
||||
* `reachable` decides whether the trigger feeds the agent. When it does not,
|
||||
* the agent and parser form an island: wired to each other, reached by nothing.
|
||||
*/
|
||||
function buildWorkflow(reachable: boolean): Partial<IWorkflowBase> {
|
||||
const nodes = [
|
||||
{
|
||||
id: nanoid(),
|
||||
name: 'Schedule Trigger',
|
||||
type: 'n8n-nodes-base.scheduleTrigger',
|
||||
typeVersion: 1.2,
|
||||
position: [0, 0] as [number, number],
|
||||
parameters: { rule: { interval: [{ field: 'days' }] } },
|
||||
},
|
||||
{
|
||||
id: nanoid(),
|
||||
name: 'No Op',
|
||||
type: 'n8n-nodes-base.noOp',
|
||||
typeVersion: 1,
|
||||
position: [220, 0] as [number, number],
|
||||
parameters: {},
|
||||
},
|
||||
{
|
||||
id: nanoid(),
|
||||
name: 'Agent',
|
||||
type: AGENT,
|
||||
typeVersion: 2.2,
|
||||
position: [220, 260] as [number, number],
|
||||
parameters: { promptType: 'define', text: 'hello', hasOutputParser: true },
|
||||
},
|
||||
{
|
||||
id: nanoid(),
|
||||
name: 'Parser',
|
||||
type: PARSER,
|
||||
typeVersion: 1,
|
||||
position: [420, 420] as [number, number],
|
||||
parameters: {},
|
||||
},
|
||||
];
|
||||
|
||||
const connections: IWorkflowBase['connections'] = {
|
||||
'Schedule Trigger': {
|
||||
main: [[{ node: reachable ? 'Agent' : 'No Op', type: 'main', index: 0 }]],
|
||||
},
|
||||
Parser: {
|
||||
ai_outputParser: [[{ node: 'Agent', type: 'ai_outputParser', index: 0 }]],
|
||||
},
|
||||
};
|
||||
|
||||
return {
|
||||
name: `required input reachability ${nanoid()}`,
|
||||
nodes,
|
||||
connections,
|
||||
settings: { executionOrder: 'v1' },
|
||||
};
|
||||
}
|
||||
|
||||
test.describe(
|
||||
'Publishing a workflow with unmet required inputs',
|
||||
{
|
||||
annotation: [{ type: 'owner', description: 'Catalysts' }],
|
||||
},
|
||||
() => {
|
||||
const cleanupWorkflowIds: string[] = [];
|
||||
const cleanupCredentialIds: string[] = [];
|
||||
|
||||
test.afterEach(async ({ api }) => {
|
||||
// The published cases leave a daily schedule active, which would keep
|
||||
// firing on a persistent stack.
|
||||
while (cleanupWorkflowIds.length > 0) {
|
||||
const workflowId = cleanupWorkflowIds.pop() as string;
|
||||
|
||||
// A refusal case never activated, so this is expected to fail there.
|
||||
try {
|
||||
await api.workflows.deactivate(workflowId);
|
||||
} catch {
|
||||
// not active
|
||||
}
|
||||
|
||||
// Archive first. The API refuses to delete a workflow that is not
|
||||
// archived, so deleting straight away leaves the row behind.
|
||||
await api.workflows.archive(workflowId);
|
||||
await api.workflows.delete(workflowId);
|
||||
}
|
||||
|
||||
while (cleanupCredentialIds.length > 0) {
|
||||
await api.credentials.deleteCredential(cleanupCredentialIds.pop() as string);
|
||||
}
|
||||
});
|
||||
|
||||
test('publishes when the nodes with unmet inputs cannot be reached', async ({ api }) => {
|
||||
const created = await api.workflows.createWorkflow(buildWorkflow(false));
|
||||
cleanupWorkflowIds.push(created.id);
|
||||
|
||||
const response = await api.workflows.activateRaw(created.id, created.versionId);
|
||||
|
||||
expect(
|
||||
response.ok(),
|
||||
`publish was refused over nodes no run can reach: ${await response.text()}`,
|
||||
).toBe(true);
|
||||
|
||||
// Activation lands through the publication outbox. Poll for it before the
|
||||
// teardown deactivates, so cleanup cannot race the publish.
|
||||
await expect
|
||||
.poll(async () => (await api.workflows.getPublicationStatus(created.id)).status, {
|
||||
timeout: 15_000,
|
||||
})
|
||||
.toBe('published');
|
||||
});
|
||||
|
||||
test('publishes a node whose inputs expression cannot be evaluated', async ({ api }) => {
|
||||
// The LangChain Code node builds its inputs by mapping over its `Inputs`
|
||||
// collection, which is empty here, so the expression throws. The engine
|
||||
// swallows that and runs the node with no inputs, so publishing must not
|
||||
// be refused. Deleting the last `Inputs` entry after wiring the node up
|
||||
// reaches this state from the editor.
|
||||
const created = await api.workflows.createWorkflow({
|
||||
name: `unresolvable inputs ${nanoid()}`,
|
||||
nodes: [
|
||||
{
|
||||
id: nanoid(),
|
||||
name: 'Schedule Trigger',
|
||||
type: 'n8n-nodes-base.scheduleTrigger',
|
||||
typeVersion: 1.2,
|
||||
position: [0, 0],
|
||||
parameters: { rule: { interval: [{ field: 'days' }] } },
|
||||
},
|
||||
{
|
||||
id: nanoid(),
|
||||
name: 'Custom Tool',
|
||||
type: LANGCHAIN_CODE,
|
||||
typeVersion: 1,
|
||||
position: [220, 0],
|
||||
parameters: { inputs: {}, outputs: { output: [{ type: 'main' }] } },
|
||||
},
|
||||
],
|
||||
connections: {
|
||||
'Schedule Trigger': {
|
||||
main: [[{ node: 'Custom Tool', type: 'main', index: 0 }]],
|
||||
},
|
||||
},
|
||||
settings: { executionOrder: 'v1' },
|
||||
});
|
||||
cleanupWorkflowIds.push(created.id);
|
||||
|
||||
const response = await api.workflows.activateRaw(created.id, created.versionId);
|
||||
|
||||
expect(
|
||||
response.ok(),
|
||||
`publish was refused over an inputs expression the engine ignores: ${await response.text()}`,
|
||||
).toBe(true);
|
||||
|
||||
await expect
|
||||
.poll(async () => (await api.workflows.getPublicationStatus(created.id)).status, {
|
||||
timeout: 15_000,
|
||||
})
|
||||
.toBe('published');
|
||||
});
|
||||
|
||||
test('publishes a subnode nested behind a disabled parent', async ({ api }) => {
|
||||
// The tool only feeds the disabled agent, and the vector store only feeds
|
||||
// the tool, so nothing ever asks the store for its Embedding input.
|
||||
// Stopping at the first consumer reads the store as live, because the tool
|
||||
// between them is enabled.
|
||||
const created = await api.workflows.createWorkflow({
|
||||
name: `nested behind a disabled parent ${nanoid()}`,
|
||||
nodes: [
|
||||
{
|
||||
id: nanoid(),
|
||||
name: 'Schedule Trigger',
|
||||
type: 'n8n-nodes-base.scheduleTrigger',
|
||||
typeVersion: 1.2,
|
||||
position: [0, 0],
|
||||
parameters: { rule: { interval: [{ field: 'days' }] } },
|
||||
},
|
||||
{
|
||||
id: nanoid(),
|
||||
name: 'Agent',
|
||||
type: AGENT,
|
||||
typeVersion: 2.2,
|
||||
position: [220, 0],
|
||||
parameters: { promptType: 'define', text: 'hello' },
|
||||
disabled: true,
|
||||
},
|
||||
{
|
||||
id: nanoid(),
|
||||
name: 'Store Tool',
|
||||
type: VECTOR_STORE_TOOL,
|
||||
typeVersion: 1,
|
||||
position: [220, 260],
|
||||
// Set so the parameter validator passes and the required-input
|
||||
// check is what this test actually exercises.
|
||||
parameters: { name: 'store_tool', description: 'company docs' },
|
||||
},
|
||||
{
|
||||
id: nanoid(),
|
||||
name: 'Store',
|
||||
type: IN_MEMORY_VECTOR_STORE,
|
||||
typeVersion: 1.3,
|
||||
position: [420, 420],
|
||||
parameters: {},
|
||||
},
|
||||
],
|
||||
connections: {
|
||||
'Schedule Trigger': { main: [[{ node: 'Agent', type: 'main', index: 0 }]] },
|
||||
'Store Tool': { ai_tool: [[{ node: 'Agent', type: 'ai_tool', index: 0 }]] },
|
||||
Store: {
|
||||
ai_vectorStore: [[{ node: 'Store Tool', type: 'ai_vectorStore', index: 0 }]],
|
||||
},
|
||||
},
|
||||
settings: { executionOrder: 'v1' },
|
||||
});
|
||||
cleanupWorkflowIds.push(created.id);
|
||||
|
||||
const response = await api.workflows.activateRaw(created.id, created.versionId);
|
||||
|
||||
expect(
|
||||
response.ok(),
|
||||
`publish was refused over a subnode nothing can reach: ${await response.text()}`,
|
||||
).toBe(true);
|
||||
|
||||
await expect
|
||||
.poll(async () => (await api.workflows.getPublicationStatus(created.id)).status, {
|
||||
timeout: 15_000,
|
||||
})
|
||||
.toBe('published');
|
||||
});
|
||||
|
||||
test('publishes a subnode left on its own beside a healthy agent', async ({ api }) => {
|
||||
// The chat model is reachable, so the pre-existing credential check applies to
|
||||
// it. Give it one, or the refusal under test is masked by a credential error.
|
||||
const credential = await api.credentials.createCredential({
|
||||
name: `anthropic ${nanoid()}`,
|
||||
type: 'anthropicApi',
|
||||
data: { apiKey: 'sk-ant-not-a-real-key' },
|
||||
});
|
||||
cleanupCredentialIds.push(credential.id);
|
||||
|
||||
// A Structured Output Parser dragged onto the canvas with Auto-Fix Format on
|
||||
// and never wired to anything. Its own `Model` input is required and unmet,
|
||||
// but no run reaches it, so it must not hold up the agent beside it. The
|
||||
// agent's own Output Parser input is exposed and empty, which is allowed:
|
||||
// only Chat Model and Fallback Model are required on the agent.
|
||||
const created = await api.workflows.createWorkflow({
|
||||
name: `lone subnode ${nanoid()}`,
|
||||
nodes: [
|
||||
{
|
||||
id: nanoid(),
|
||||
name: 'Schedule Trigger',
|
||||
type: 'n8n-nodes-base.scheduleTrigger',
|
||||
typeVersion: 1.2,
|
||||
position: [0, 0],
|
||||
parameters: { rule: { interval: [{ field: 'days' }] } },
|
||||
},
|
||||
{
|
||||
id: nanoid(),
|
||||
name: 'Sentiment Analyzer',
|
||||
type: AGENT,
|
||||
typeVersion: 2.2,
|
||||
position: [260, 0],
|
||||
parameters: { promptType: 'define', text: 'hello', hasOutputParser: true },
|
||||
},
|
||||
{
|
||||
id: nanoid(),
|
||||
name: 'Chat Model',
|
||||
type: '@n8n/n8n-nodes-langchain.lmChatAnthropic',
|
||||
typeVersion: 1.3,
|
||||
position: [200, 300],
|
||||
parameters: {
|
||||
model: { __rl: true, mode: 'list', value: 'claude-sonnet-4-5' },
|
||||
options: {},
|
||||
},
|
||||
credentials: { anthropicApi: { id: credential.id, name: credential.name } },
|
||||
},
|
||||
{
|
||||
id: nanoid(),
|
||||
name: 'Structured Output Parser',
|
||||
type: STRUCTURED_PARSER,
|
||||
typeVersion: 1.3,
|
||||
position: [560, 300],
|
||||
parameters: { jsonSchemaExample: '{ "sentiment": "positive" }', autoFix: true },
|
||||
},
|
||||
],
|
||||
connections: {
|
||||
'Schedule Trigger': {
|
||||
main: [[{ node: 'Sentiment Analyzer', type: 'main', index: 0 }]],
|
||||
},
|
||||
'Chat Model': {
|
||||
ai_languageModel: [
|
||||
[{ node: 'Sentiment Analyzer', type: 'ai_languageModel', index: 0 }],
|
||||
],
|
||||
},
|
||||
},
|
||||
settings: { executionOrder: 'v1' },
|
||||
});
|
||||
cleanupWorkflowIds.push(created.id);
|
||||
|
||||
const response = await api.workflows.activateRaw(created.id, created.versionId);
|
||||
|
||||
expect(
|
||||
response.ok(),
|
||||
`publish was refused over a subnode wired to nothing: ${await response.text()}`,
|
||||
).toBe(true);
|
||||
|
||||
await expect
|
||||
.poll(async () => (await api.workflows.getPublicationStatus(created.id)).status, {
|
||||
timeout: 15_000,
|
||||
})
|
||||
.toBe('published');
|
||||
});
|
||||
|
||||
test('refuses to publish when the same nodes are reachable', async ({ api }) => {
|
||||
const created = await api.workflows.createWorkflow(buildWorkflow(true));
|
||||
cleanupWorkflowIds.push(created.id);
|
||||
|
||||
const response = await api.workflows.activateRaw(created.id, created.versionId);
|
||||
|
||||
expect(response.ok()).toBe(false);
|
||||
expect(await response.text()).toContain('has no node connected to its required');
|
||||
});
|
||||
},
|
||||
);
|
||||
114
packages/workflow/src/common/get-runnable-nodes.ts
Normal file
114
packages/workflow/src/common/get-runnable-nodes.ts
Normal file
|
|
@ -0,0 +1,114 @@
|
|||
import { getExecutableNodeNames } from './get-executable-nodes';
|
||||
import type { IConnections, INode } from '../interfaces';
|
||||
|
||||
/**
|
||||
* Which nodes a run can actually involve.
|
||||
*
|
||||
* Publishing is gated on this in two places that must agree: the server refuses
|
||||
* activation over a node that would break a run, and the editor greys out the
|
||||
* Publish button for the same reason. A node no run can reach cannot break one,
|
||||
* so neither surface may block on it.
|
||||
*/
|
||||
|
||||
/** Nodes a node feeds directly, by name, in graph order. */
|
||||
function consumerNames(nodeName: string, connections: IConnections): string[] {
|
||||
const names: string[] = [];
|
||||
|
||||
for (const targetsByOutput of Object.values(connections[nodeName] ?? {})) {
|
||||
for (const targets of targetsByOutput ?? []) {
|
||||
for (const target of targets ?? []) {
|
||||
names.push(target.node);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return names;
|
||||
}
|
||||
|
||||
/**
|
||||
* Names of every node some trigger can reach, following the same rules the
|
||||
* engine uses: the forward `main` closure of each enabled trigger, plus the
|
||||
* sub-nodes hanging off anything in it.
|
||||
*
|
||||
* Disabled nodes are walked through rather than stopped at, because a disabled
|
||||
* node passes its items downstream, so what follows one still runs.
|
||||
*
|
||||
* @param isTriggerLike decides what starts a run. The server reads the node
|
||||
* type's implementation, the editor only has the description, so each caller
|
||||
* supplies its own.
|
||||
*/
|
||||
export function getReachableNodeNames(
|
||||
nodes: INode[],
|
||||
connectionsBySourceNode: IConnections,
|
||||
connectionsByDestinationNode: IConnections,
|
||||
isTriggerLike: (node: INode) => boolean,
|
||||
): Set<string> {
|
||||
const reachable = new Set<string>();
|
||||
|
||||
for (const node of nodes) {
|
||||
if (node.disabled) continue;
|
||||
if (!isTriggerLike(node)) continue;
|
||||
|
||||
for (const name of getExecutableNodeNames(
|
||||
connectionsBySourceNode,
|
||||
connectionsByDestinationNode,
|
||||
node.name,
|
||||
)) {
|
||||
reachable.add(name);
|
||||
}
|
||||
}
|
||||
|
||||
return reachable;
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether nothing will ever ask this node for its inputs, because every chain
|
||||
* of consumers out of it is disabled. Only ever true for supply-type nodes (LLM
|
||||
* models, parsers, memory, tools): anything that can produce a `main` output
|
||||
* belongs to the flow and runs regardless of what its consumers do.
|
||||
*
|
||||
* Consumers are followed transitively. A model feeding a parser feeding a
|
||||
* disabled agent is as inert as one feeding the disabled agent directly, and
|
||||
* stopping at the first hop would refuse to publish over it.
|
||||
*
|
||||
* `canOutputMain` reads the type's declared outputs rather than the
|
||||
* connections, since an unwired `main` output is still a main-path node. It must
|
||||
* answer `true` when unsure, so a half-built graph does not silently opt out of
|
||||
* the check.
|
||||
*/
|
||||
export function onlySuppliesDisabledNodes(
|
||||
node: INode,
|
||||
connections: IConnections,
|
||||
nodes: INode[],
|
||||
canOutputMain: (node: INode) => boolean,
|
||||
): boolean {
|
||||
if (canOutputMain(node)) return false;
|
||||
|
||||
const nodeByName = new Map(nodes.map((n) => [n.name, n]));
|
||||
// Doubles as cycle protection and keeps this linear over the sub-graph.
|
||||
const visited = new Set<string>();
|
||||
|
||||
/** Whether anything that actually runs asks `nodeName` for its output. */
|
||||
function isAsked(nodeName: string): boolean {
|
||||
if (visited.has(nodeName)) return false;
|
||||
visited.add(nodeName);
|
||||
|
||||
for (const consumerName of consumerNames(nodeName, connections)) {
|
||||
const consumer = nodeByName.get(consumerName);
|
||||
// An unknown target is treated as live.
|
||||
if (!consumer) return true;
|
||||
if (consumer.disabled) continue;
|
||||
// A consumer on the main path runs, so it will ask. A supply consumer is
|
||||
// itself only asked by its own consumers.
|
||||
if (canOutputMain(consumer) || isAsked(consumer.name)) return true;
|
||||
}
|
||||
|
||||
return false;
|
||||
}
|
||||
|
||||
// A supply node wired to nothing is left to the reachability check, which
|
||||
// skips it anyway, rather than exempted here.
|
||||
if (consumerNames(node.name, connections).length === 0) return false;
|
||||
|
||||
return !isAsked(node.name);
|
||||
}
|
||||
|
|
@ -4,5 +4,6 @@ export * from './get-connected-nodes';
|
|||
export * from './get-executable-nodes';
|
||||
export * from './get-node-by-name';
|
||||
export * from './get-parent-nodes';
|
||||
export * from './get-runnable-nodes';
|
||||
export * from './map-connections-by-destination';
|
||||
export * from './resolve-variables';
|
||||
|
|
|
|||
|
|
@ -1244,6 +1244,19 @@ export function getNodeInputs(
|
|||
workflow: WorkflowForNodeHelpers,
|
||||
node: INode,
|
||||
nodeTypeData: INodeTypeDescription,
|
||||
options: {
|
||||
/**
|
||||
* Surface a broken `inputs` expression instead of reporting no inputs.
|
||||
* Swallowing suits rendering; a caller judging validity wants the error.
|
||||
*
|
||||
* Only a thrown error counts. An expression that evaluates but yields a
|
||||
* non-list is left alone: that is almost always an unconfigured node
|
||||
* (the LangChain Code node maps over an empty `Inputs` collection), and
|
||||
* the engine reads the same case as "no inputs" and runs. Failing here
|
||||
* would block publishing a workflow the runtime is happy with.
|
||||
*/
|
||||
throwOnExpressionError?: boolean;
|
||||
} = {},
|
||||
): Array<NodeConnectionType | INodeInputConfiguration> {
|
||||
if (Array.isArray(nodeTypeData?.inputs)) {
|
||||
return nodeTypeData.inputs;
|
||||
|
|
@ -1258,6 +1271,7 @@ export function getNodeInputs(
|
|||
{},
|
||||
) || []) as NodeConnectionType[];
|
||||
} catch (e) {
|
||||
if (options.throwOnExpressionError) throw e;
|
||||
console.warn('Could not calculate inputs dynamically for node: ', node.name);
|
||||
return [];
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,5 +1,6 @@
|
|||
import type { INode, INodeType, IConnections } from './interfaces';
|
||||
import { displayParameter } from './node-helpers';
|
||||
import type { INode, INodeType, INodeInputConfiguration, INodeTypeDescription } from './interfaces';
|
||||
import { displayParameter, getNodeInputs } from './node-helpers';
|
||||
import type { Workflow } from './workflow';
|
||||
|
||||
export interface NodeValidationIssue {
|
||||
credential?: string;
|
||||
|
|
@ -12,6 +13,60 @@ export interface NodeCredentialIssue {
|
|||
credentialName: string;
|
||||
}
|
||||
|
||||
/** A `Pick` so both the editor's snapshot accessor and the engine's `Workflow` fit. */
|
||||
export type WorkflowForInputValidation = Pick<
|
||||
Workflow,
|
||||
'expression' | 'getNode' | 'connectionsByDestinationNode'
|
||||
>;
|
||||
|
||||
/**
|
||||
* Inputs a node declares as required with nothing enabled connected to them.
|
||||
*
|
||||
* Inputs can be conditional, so they are resolved against the node's current
|
||||
* parameters first. A disabled source counts as absent, matching the runtime
|
||||
* ("must be connected and enabled").
|
||||
*
|
||||
* Shared so the editor warning and the publish check agree; callers format their
|
||||
* own message.
|
||||
*/
|
||||
export function getUnconnectedRequiredInputs(
|
||||
workflow: WorkflowForInputValidation,
|
||||
node: INode,
|
||||
nodeTypeDescription: INodeTypeDescription,
|
||||
options: { throwOnExpressionError?: boolean } = {},
|
||||
): INodeInputConfiguration[] {
|
||||
const unconnected: INodeInputConfiguration[] = [];
|
||||
// Required by the type on purpose. A missing map is not an empty graph: it
|
||||
// would read every required input as unconnected. Better a loud TypeError
|
||||
// here than a silent over-report from an accessor cast past the type.
|
||||
const arrivals = workflow.connectionsByDestinationNode[node.name];
|
||||
|
||||
// A node can declare several inputs of one type — an agent's Chat Model and
|
||||
// Fallback Model are both `ai_languageModel` — and each is satisfied on its
|
||||
// own index. Counting position per type mirrors how the engine resolves them
|
||||
// (`validateInputConfiguration` filters by type, then indexes into the
|
||||
// type's connections), so one connected model cannot satisfy both.
|
||||
const indexByType = new Map<string, number>();
|
||||
|
||||
for (const input of getNodeInputs(workflow, node, nodeTypeDescription, options)) {
|
||||
const type = typeof input === 'string' ? input : input.type;
|
||||
const inputIndex = indexByType.get(type) ?? 0;
|
||||
indexByType.set(type, inputIndex + 1);
|
||||
|
||||
if (typeof input === 'string' || input.required !== true) continue;
|
||||
|
||||
const sources = arrivals?.[type]?.[inputIndex] ?? [];
|
||||
const hasEnabledSource = sources.some((source) => {
|
||||
const parent = workflow.getNode(source.node);
|
||||
return parent ? !parent.disabled : false;
|
||||
});
|
||||
|
||||
if (!hasEnabledSource) unconnected.push(input);
|
||||
}
|
||||
|
||||
return unconnected;
|
||||
}
|
||||
|
||||
/**
|
||||
* Validates that all required credentials are set for a node.
|
||||
* Respects displayOptions to only validate credentials that should be shown.
|
||||
|
|
@ -53,30 +108,6 @@ export function validateNodeCredentials(node: INode, nodeType: INodeType): NodeC
|
|||
return issues;
|
||||
}
|
||||
|
||||
/**
|
||||
* Checks if a node has any incoming or outgoing connections.
|
||||
*/
|
||||
export function isNodeConnected(
|
||||
nodeName: string,
|
||||
connections: IConnections,
|
||||
connectionsByDestination: IConnections,
|
||||
): boolean {
|
||||
// Check outgoing connections
|
||||
if (connections[nodeName] && Object.keys(connections[nodeName]).length > 0) {
|
||||
return true;
|
||||
}
|
||||
|
||||
// Check incoming connections
|
||||
if (
|
||||
connectionsByDestination[nodeName] &&
|
||||
Object.keys(connectionsByDestination[nodeName]).length > 0
|
||||
) {
|
||||
return true;
|
||||
}
|
||||
|
||||
return false;
|
||||
}
|
||||
|
||||
/**
|
||||
* Checks if a node type is a trigger-like node (trigger, webhook, or poll).
|
||||
* These nodes are workflow entry points and should always be validated.
|
||||
|
|
|
|||
|
|
@ -1,9 +1,10 @@
|
|||
import type { INode, INodeType, IConnections, INodeTypeDescription } from '../src/interfaces';
|
||||
import type { INode, INodeType, INodeTypeDescription } from '../src/interfaces';
|
||||
import {
|
||||
validateNodeCredentials,
|
||||
isNodeConnected,
|
||||
getUnconnectedRequiredInputs,
|
||||
isTriggerLikeNode,
|
||||
type NodeCredentialIssue,
|
||||
type WorkflowForInputValidation,
|
||||
} from '../src/node-validation';
|
||||
|
||||
describe('node-validation', () => {
|
||||
|
|
@ -198,77 +199,6 @@ describe('node-validation', () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe('isNodeConnected', () => {
|
||||
it('should return true when node has outgoing connections', () => {
|
||||
const connections: IConnections = {
|
||||
'Node A': {
|
||||
main: [[{ node: 'Node B', type: 'main', index: 0 }]],
|
||||
},
|
||||
};
|
||||
const connectionsByDestination: IConnections = {};
|
||||
|
||||
const result = isNodeConnected('Node A', connections, connectionsByDestination);
|
||||
|
||||
expect(result).toBe(true);
|
||||
});
|
||||
|
||||
it('should return true when node has incoming connections', () => {
|
||||
const connections: IConnections = {};
|
||||
const connectionsByDestination: IConnections = {
|
||||
'Node B': {
|
||||
main: [[{ node: 'Node A', type: 'main', index: 0 }]],
|
||||
},
|
||||
};
|
||||
|
||||
const result = isNodeConnected('Node B', connections, connectionsByDestination);
|
||||
|
||||
expect(result).toBe(true);
|
||||
});
|
||||
|
||||
it('should return true when node has both incoming and outgoing connections', () => {
|
||||
const connections: IConnections = {
|
||||
'Node B': {
|
||||
main: [[{ node: 'Node C', type: 'main', index: 0 }]],
|
||||
},
|
||||
};
|
||||
const connectionsByDestination: IConnections = {
|
||||
'Node B': {
|
||||
main: [[{ node: 'Node A', type: 'main', index: 0 }]],
|
||||
},
|
||||
};
|
||||
|
||||
const result = isNodeConnected('Node B', connections, connectionsByDestination);
|
||||
|
||||
expect(result).toBe(true);
|
||||
});
|
||||
|
||||
it('should return false when node has no connections', () => {
|
||||
const connections: IConnections = {
|
||||
'Node A': {
|
||||
main: [[{ node: 'Node B', type: 'main', index: 0 }]],
|
||||
},
|
||||
};
|
||||
const connectionsByDestination: IConnections = {};
|
||||
|
||||
const result = isNodeConnected('Node C', connections, connectionsByDestination);
|
||||
|
||||
expect(result).toBe(false);
|
||||
});
|
||||
|
||||
it('should return false when node exists but has empty connections', () => {
|
||||
const connections: IConnections = {
|
||||
'Node A': {},
|
||||
};
|
||||
const connectionsByDestination: IConnections = {
|
||||
'Node A': {},
|
||||
};
|
||||
|
||||
const result = isNodeConnected('Node A', connections, connectionsByDestination);
|
||||
|
||||
expect(result).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('isTriggerLikeNode', () => {
|
||||
it('should return true for node with trigger function', () => {
|
||||
const nodeType: INodeType = {
|
||||
|
|
@ -368,4 +298,219 @@ describe('node-validation', () => {
|
|||
expect(isTriggerLikeNode(nodeType)).toBe(false);
|
||||
});
|
||||
});
|
||||
describe('getUnconnectedRequiredInputs', () => {
|
||||
const parser: INode = {
|
||||
name: 'Parser',
|
||||
type: 'parser',
|
||||
id: 'parser-1',
|
||||
typeVersion: 1,
|
||||
position: [0, 0],
|
||||
parameters: { autoFix: true },
|
||||
};
|
||||
|
||||
const model: INode = {
|
||||
name: 'Model',
|
||||
type: 'model',
|
||||
id: 'model-1',
|
||||
typeVersion: 1,
|
||||
position: [0, 0],
|
||||
parameters: {},
|
||||
};
|
||||
|
||||
/** An expression, so inputs depend on the node's parameters. */
|
||||
const description = {
|
||||
inputs:
|
||||
'={{ $parameter.autoFix ? [{ displayName: "Model", type: "ai_languageModel", required: true }] : [] }}',
|
||||
} as unknown as INodeTypeDescription;
|
||||
|
||||
/**
|
||||
* `sourcesByIndex` lists the nodes arriving at each `ai_languageModel`
|
||||
* input, by input index — the same shape the engine indexes into, so a
|
||||
* source on index 0 cannot stand in for the input on index 1.
|
||||
*/
|
||||
const makeWorkflow = (
|
||||
sourcesByIndex: string[][],
|
||||
nodes: Record<string, INode>,
|
||||
): WorkflowForInputValidation =>
|
||||
({
|
||||
expression: {
|
||||
// Mirrors the engine: an undefined `inputs` resolves to undefined
|
||||
// rather than to a list.
|
||||
getSimpleParameterValue: (node: INode, value: unknown) =>
|
||||
value === undefined
|
||||
? undefined
|
||||
: (node.parameters as { autoFix?: boolean }).autoFix
|
||||
? [{ displayName: 'Model', type: 'ai_languageModel', required: true }]
|
||||
: [],
|
||||
},
|
||||
getNode: (name: string) => nodes[name] ?? null,
|
||||
connectionsByDestinationNode: {
|
||||
Parser: {
|
||||
ai_languageModel: sourcesByIndex.map((names) =>
|
||||
names.map((name) => ({ node: name, type: 'ai_languageModel', index: 0 })),
|
||||
),
|
||||
},
|
||||
},
|
||||
}) as unknown as WorkflowForInputValidation;
|
||||
|
||||
it('reports a required input with nothing connected', () => {
|
||||
const result = getUnconnectedRequiredInputs(
|
||||
makeWorkflow([], { Parser: parser }),
|
||||
parser,
|
||||
description,
|
||||
);
|
||||
|
||||
expect(result).toHaveLength(1);
|
||||
expect(result[0].type).toBe('ai_languageModel');
|
||||
});
|
||||
|
||||
it('reports nothing once an enabled node is connected', () => {
|
||||
const result = getUnconnectedRequiredInputs(
|
||||
makeWorkflow([['Model']], { Parser: parser, Model: model }),
|
||||
parser,
|
||||
description,
|
||||
);
|
||||
|
||||
expect(result).toEqual([]);
|
||||
});
|
||||
|
||||
it('treats a disabled source as not connected', () => {
|
||||
const result = getUnconnectedRequiredInputs(
|
||||
makeWorkflow([['Model']], { Parser: parser, Model: { ...model, disabled: true } }),
|
||||
parser,
|
||||
description,
|
||||
);
|
||||
|
||||
expect(result).toHaveLength(1);
|
||||
});
|
||||
|
||||
it('reports nothing when a failed inputs expression is treated as empty', () => {
|
||||
const throwing = {
|
||||
expression: {
|
||||
getSimpleParameterValue: () => {
|
||||
throw new Error('boom');
|
||||
},
|
||||
},
|
||||
getNode: () => parser,
|
||||
connectionsByDestinationNode: {},
|
||||
} as unknown as WorkflowForInputValidation;
|
||||
|
||||
expect(getUnconnectedRequiredInputs(throwing, parser, description)).toEqual([]);
|
||||
});
|
||||
|
||||
it('surfaces a failed inputs expression when asked to', () => {
|
||||
const throwing = {
|
||||
expression: {
|
||||
getSimpleParameterValue: () => {
|
||||
throw new Error('boom');
|
||||
},
|
||||
},
|
||||
getNode: () => parser,
|
||||
connectionsByDestinationNode: {},
|
||||
} as unknown as WorkflowForInputValidation;
|
||||
|
||||
expect(() =>
|
||||
getUnconnectedRequiredInputs(throwing, parser, description, {
|
||||
throwOnExpressionError: true,
|
||||
}),
|
||||
).toThrow('boom');
|
||||
});
|
||||
|
||||
// An expression that evaluates to a non-list is the normal state of an
|
||||
// unconfigured node: the LangChain Code node maps over its `Inputs`
|
||||
// collection, which is empty by default, and the engine swallows the
|
||||
// resulting TypeError into `null`. The engine then reads that as "no
|
||||
// inputs" and runs, so neither caller may treat it as an error — doing
|
||||
// so blocks publishing a workflow the runtime is happy with.
|
||||
describe('when the expression resolves to no list at all', () => {
|
||||
const unresolved = {
|
||||
expression: { getSimpleParameterValue: () => null },
|
||||
getNode: () => parser,
|
||||
connectionsByDestinationNode: {},
|
||||
} as unknown as WorkflowForInputValidation;
|
||||
|
||||
it('reports nothing even when asked to surface expression errors', () => {
|
||||
expect(
|
||||
getUnconnectedRequiredInputs(unresolved, parser, description, {
|
||||
throwOnExpressionError: true,
|
||||
}),
|
||||
).toEqual([]);
|
||||
});
|
||||
|
||||
it('reports nothing when swallowing suits the caller', () => {
|
||||
expect(getUnconnectedRequiredInputs(unresolved, parser, description)).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
it('reports nothing for a type that declares no inputs at all', () => {
|
||||
// Nothing to resolve, so this is "requires nothing", not "unknown".
|
||||
const noInputs = { properties: [] } as unknown as INodeTypeDescription;
|
||||
|
||||
expect(
|
||||
getUnconnectedRequiredInputs(makeWorkflow([], { Parser: parser }), parser, noInputs, {
|
||||
throwOnExpressionError: true,
|
||||
}),
|
||||
).toEqual([]);
|
||||
});
|
||||
|
||||
// An agent with a fallback declares two required `ai_languageModel` inputs;
|
||||
// the engine satisfies each on its own index.
|
||||
describe('with two required inputs of the same type', () => {
|
||||
const twoModels = {
|
||||
inputs: [
|
||||
{ displayName: 'Chat Model', type: 'ai_languageModel', required: true },
|
||||
{ displayName: 'Fallback Model', type: 'ai_languageModel', required: true },
|
||||
],
|
||||
} as unknown as INodeTypeDescription;
|
||||
|
||||
it('does not let a source on the first index satisfy the second', () => {
|
||||
const result = getUnconnectedRequiredInputs(
|
||||
makeWorkflow([['Model']], { Parser: parser, Model: model }),
|
||||
parser,
|
||||
twoModels,
|
||||
);
|
||||
|
||||
expect(result.map((input) => input.displayName)).toEqual(['Fallback Model']);
|
||||
});
|
||||
|
||||
it('reports nothing once each index has its own source', () => {
|
||||
const result = getUnconnectedRequiredInputs(
|
||||
makeWorkflow([['Primary'], ['Fallback']], {
|
||||
Parser: parser,
|
||||
Primary: model,
|
||||
Fallback: { ...model, name: 'Fallback' },
|
||||
}),
|
||||
parser,
|
||||
twoModels,
|
||||
);
|
||||
|
||||
expect(result).toEqual([]);
|
||||
});
|
||||
|
||||
it('treats a disabled source on the second index as absent', () => {
|
||||
const result = getUnconnectedRequiredInputs(
|
||||
makeWorkflow([['Primary'], ['Fallback']], {
|
||||
Parser: parser,
|
||||
Primary: model,
|
||||
Fallback: { ...model, name: 'Fallback', disabled: true },
|
||||
}),
|
||||
parser,
|
||||
twoModels,
|
||||
);
|
||||
|
||||
expect(result.map((input) => input.displayName)).toEqual(['Fallback Model']);
|
||||
});
|
||||
});
|
||||
|
||||
it('reports nothing when the parameters do not make the input required', () => {
|
||||
const off = { ...parser, parameters: { autoFix: false } };
|
||||
const result = getUnconnectedRequiredInputs(
|
||||
makeWorkflow([], { Parser: off }),
|
||||
off,
|
||||
description,
|
||||
);
|
||||
|
||||
expect(result).toEqual([]);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user