feat(core): Explicitly warn if tool passed binary data to Agent (#14071)

packages/core/src/execution-engine/node-execution-context/utils/__tests__/get-input-connection-data.test.ts

@@ -10,10 +10,12 @@ import type {
 	Workflow,
 	INodeType,
 	INodeTypes,
+	IExecuteFunctions,
 } from 'n8n-workflow';
 import { NodeConnectionTypes, NodeOperationError } from 'n8n-workflow';
 
 import { ExecuteContext } from '../../execute-context';
+import { makeHandleToolInvocation } from '../get-input-connection-data';
 
 describe('getInputConnectionData', () => {
 	const agentNode = mock<INode>({
@@ -364,3 +366,141 @@ describe('getInputConnectionData', () => {
 		});
 	});
 });
+
+describe('makeHandleToolInvocation', () => {
+	const connectedNode = mock<INode>({
+		name: 'Test Tool Node',
+		type: 'test.tool',
+	});
+	const execute = jest.fn();
+	const connectedNodeType = mock<INodeType>({
+		execute,
+	});
+	const contextFactory = jest.fn();
+	const toolArgs = { key: 'value' };
+
+	beforeEach(() => {
+		jest.clearAllMocks();
+	});
+	it('should return stringified results when execution is successful', async () => {
+		const mockContext = mock<IExecuteFunctions>();
+		contextFactory.mockReturnValue(mockContext);
+
+		const mockResult = [[{ json: { result: 'success' } }]];
+		execute.mockResolvedValueOnce(mockResult);
+
+		const handleToolInvocation = makeHandleToolInvocation(
+			contextFactory,
+			connectedNode,
+			connectedNodeType,
+		);
+		const result = await handleToolInvocation(toolArgs);
+
+		expect(result).toBe(JSON.stringify([{ result: 'success' }]));
+		expect(mockContext.addOutputData).toHaveBeenCalledWith(NodeConnectionTypes.AiTool, 0, [
+			[{ json: { response: [{ result: 'success' }] } }],
+		]);
+	});
+
+	it('should handle binary data and return a warning message', async () => {
+		const mockContext = mock<IExecuteFunctions>();
+		contextFactory.mockReturnValue(mockContext);
+
+		const mockResult = [[{ json: {}, binary: { file: 'data' } }]];
+		execute.mockResolvedValueOnce(mockResult);
+
+		const handleToolInvocation = makeHandleToolInvocation(
+			contextFactory,
+			connectedNode,
+			connectedNodeType,
+		);
+		const result = await handleToolInvocation(toolArgs);
+
+		expect(result).toBe(
+			'"Error: The Tool attempted to return binary data, which is not supported in Agents"',
+		);
+		expect(mockContext.addOutputData).toHaveBeenCalledWith(NodeConnectionTypes.AiTool, 0, [
+			[
+				{
+					json: {
+						response:
+							'Error: The Tool attempted to return binary data, which is not supported in Agents',
+					},
+				},
+			],
+		]);
+	});
+
+	it('should continue if json and binary data exist', async () => {
+		const warnFn = jest.fn();
+		const mockContext = mock<IExecuteFunctions>({
+			logger: {
+				warn: warnFn,
+			},
+		});
+		contextFactory.mockReturnValue(mockContext);
+
+		const mockResult = [[{ json: { a: 3 }, binary: { file: 'data' } }]];
+		execute.mockResolvedValueOnce(mockResult);
+
+		const handleToolInvocation = makeHandleToolInvocation(
+			contextFactory,
+			connectedNode,
+			connectedNodeType,
+		);
+		const result = await handleToolInvocation(toolArgs);
+
+		expect(result).toBe('[{"a":3}]');
+		expect(mockContext.addOutputData).toHaveBeenCalledWith(NodeConnectionTypes.AiTool, 0, [
+			[
+				{
+					json: {
+						response: [{ a: 3 }],
+					},
+				},
+			],
+		]);
+		expect(warnFn).toHaveBeenCalled();
+	});
+
+	it('should handle execution errors and return an error message', async () => {
+		const mockContext = mock<IExecuteFunctions>();
+		contextFactory.mockReturnValue(mockContext);
+
+		const error = new Error('Execution failed');
+		execute.mockRejectedValueOnce(error);
+
+		const handleToolInvocation = makeHandleToolInvocation(
+			contextFactory,
+			connectedNode,
+			connectedNodeType,
+		);
+		const result = await handleToolInvocation(toolArgs);
+
+		expect(result).toBe('Error during node execution: Execution failed');
+		expect(mockContext.addOutputData).toHaveBeenCalledWith(
+			NodeConnectionTypes.AiTool,
+			0,
+			expect.any(NodeOperationError),
+		);
+	});
+
+	it('should increment the toolRunIndex for each invocation', async () => {
+		const mockContext = mock<IExecuteFunctions>();
+		contextFactory.mockReturnValue(mockContext);
+
+		const handleToolInvocation = makeHandleToolInvocation(
+			contextFactory,
+			connectedNode,
+			connectedNodeType,
+		);
+
+		await handleToolInvocation(toolArgs);
+		await handleToolInvocation(toolArgs);
+		await handleToolInvocation(toolArgs);
+
+		expect(contextFactory).toHaveBeenCalledWith(0);
+		expect(contextFactory).toHaveBeenCalledWith(1);
+		expect(contextFactory).toHaveBeenCalledWith(2);
+	});
+});

packages/core/src/execution-engine/node-execution-context/utils/get-input-connection-data.ts

@@ -11,6 +11,10 @@ import type {
 	WorkflowExecuteMode,
 	SupplyData,
 	AINodeConnectionType,
+	IDataObject,
+	ISupplyDataFunctions,
+	INodeType,
+	INode,
 } from 'n8n-workflow';
 import {
 	NodeConnectionTypes,
@@ -24,6 +28,54 @@ import type { ExecuteContext, WebhookContext } from '../../node-execution-contex
 // eslint-disable-next-line import/no-cycle
 import { SupplyDataContext } from '../../node-execution-context/supply-data-context';
 
+export function makeHandleToolInvocation(
+	contextFactory: (runIndex: number) => ISupplyDataFunctions,
+	node: INode,
+	nodeType: INodeType,
+) {
+	/**
+	 * This keeps track of how many times this specific AI tool node has been invoked.
+	 * It is incremented on every invocation of the tool to keep the output of each invocation separate from each other.
+	 */
+	let toolRunIndex = 0;
+	return async (toolArgs: IDataObject) => {
+		const runIndex = toolRunIndex++;
+		const context = contextFactory(runIndex);
+		context.addInputData(NodeConnectionTypes.AiTool, [[{ json: toolArgs }]]);
+
+		try {
+			// Execute the sub-node with the proxied context
+			const result = await nodeType.execute?.call(context as unknown as IExecuteFunctions);
+
+			// Process and map the results
+			const mappedResults = result?.[0]?.flatMap((item) => item.json);
+			let response: string | typeof mappedResults = mappedResults;
+
+			// Warn if any (unusable) binary data was returned
+			if (result?.some((x) => x.some((y) => y.binary))) {
+				if (!mappedResults || mappedResults.flatMap((x) => Object.keys(x ?? {})).length === 0) {
+					response =
+						'Error: The Tool attempted to return binary data, which is not supported in Agents';
+				} else {
+					context.logger.warn(
+						`Response from Tool '${node.name}' included binary data, which is not supported in Agents. The binary data was omitted from the response.`,
+					);
+				}
+			}
+
+			// Add output data to the context
+			context.addOutputData(NodeConnectionTypes.AiTool, runIndex, [[{ json: { response } }]]);
+
+			// Return the stringified results
+			return JSON.stringify(response);
+		} catch (error) {
+			const nodeError = new NodeOperationError(node, error as Error);
+			context.addOutputData(NodeConnectionTypes.AiTool, runIndex, nodeError);
+			return 'Error during node execution: ' + (nodeError.description ?? nodeError.message);
+		}
+	};
+}
+
 export async function getInputConnectionData(
 	this: ExecuteContext | WebhookContext | SupplyDataContext,
 	workflow: Workflow,
@@ -93,41 +145,14 @@ export async function getInputConnectionData(
 
 		if (!connectedNodeType.supplyData) {
 			if (connectedNodeType.description.outputs.includes(NodeConnectionTypes.AiTool)) {
-				/**
-				 * This keeps track of how many times this specific AI tool node has been invoked.
-				 * It is incremented on every invocation of the tool to keep the output of each invocation separate from each other.
-				 */
-				let toolRunIndex = 0;
 				const supplyData = createNodeAsTool({
 					node: connectedNode,
 					nodeType: connectedNodeType,
-					handleToolInvocation: async (toolArgs) => {
+					handleToolInvocation: makeHandleToolInvocation(
-						const runIndex = toolRunIndex++;
+						(i) => contextFactory(i, {}),
-						const context = contextFactory(runIndex, {});
+						connectedNode,
-						context.addInputData(NodeConnectionTypes.AiTool, [[{ json: toolArgs }]]);
+						connectedNodeType,
-
+					),
-						try {
-							// Execute the sub-node with the proxied context
-							const result = await connectedNodeType.execute?.call(
-								context as unknown as IExecuteFunctions,
-							);
-
-							// Process and map the results
-							const mappedResults = result?.[0]?.flatMap((item) => item.json);
-
-							// Add output data to the context
-							context.addOutputData(NodeConnectionTypes.AiTool, runIndex, [
-								[{ json: { response: mappedResults } }],
-							]);
-
-							// Return the stringified results
-							return JSON.stringify(mappedResults);
-						} catch (error) {
-							const nodeError = new NodeOperationError(connectedNode, error as Error);
-							context.addOutputData(NodeConnectionTypes.AiTool, runIndex, nodeError);
-							return 'Error during node execution: ' + nodeError.description;
-						}
-					},
 				});
 				nodes.push(supplyData);
 			} else {