mirror of
https://github.com/n8n-io/n8n.git
synced 2026-10-17 05:29:44 +02:00
fix(AI Agent Node): Let an AI Agent Tool use its own tools under a pre-v3 parent agent (#39160)
This commit is contained in:
parent
da06957d53
commit
60d0598a69
|
|
@ -354,10 +354,11 @@ export async function toolsAgentExecute(
|
|||
if (Object.keys(additionalMetadata).length > 0) {
|
||||
this.logger.debug('Tracing metadata', { additionalMetadata });
|
||||
}
|
||||
const tracingConfig = isExecuteFunctions(this)
|
||||
? getTracingConfig(this, { additionalMetadata })
|
||||
: undefined;
|
||||
const executorWithTracing = tracingConfig ? executor.withConfig(tracingConfig) : executor;
|
||||
// `getTracingConfig` supports both context types, so sub-agents keep their
|
||||
// LangSmith run name and execution id.
|
||||
const executorWithTracing = executor.withConfig(
|
||||
getTracingConfig(this, { additionalMetadata }),
|
||||
);
|
||||
// Invoke with fallback logic
|
||||
const invokeParams = {
|
||||
input,
|
||||
|
|
|
|||
|
|
@ -19,7 +19,6 @@ import type {
|
|||
} from 'n8n-workflow';
|
||||
|
||||
import type { ItemContext } from './prepareItemContext';
|
||||
import { isExecuteFunctions } from '../../../utils';
|
||||
import { SYSTEM_MESSAGE } from '../../prompt';
|
||||
import type { AgentResult } from '../types';
|
||||
|
||||
|
|
@ -94,10 +93,9 @@ export async function runAgent(
|
|||
if (Object.keys(additionalMetadata).length > 0 && logger) {
|
||||
ctx.logger.debug('Tracing metadata', { additionalMetadata });
|
||||
}
|
||||
const tracingConfig = isExecuteFunctions(ctx)
|
||||
? getTracingConfig(ctx, { additionalMetadata })
|
||||
: undefined;
|
||||
const executorWithTracing = tracingConfig ? executor.withConfig(tracingConfig) : executor;
|
||||
// `getTracingConfig` supports both context types, so sub-agents keep their LangSmith
|
||||
// run name and execution id.
|
||||
const executorWithTracing = executor.withConfig(getTracingConfig(ctx, { additionalMetadata }));
|
||||
|
||||
// Check if streaming is actually available
|
||||
const isStreamingAvailable = 'isStreaming' in ctx ? ctx.isStreaming?.() : undefined;
|
||||
|
|
|
|||
|
|
@ -39,7 +39,6 @@ beforeEach(() => {
|
|||
vi.clearAllMocks();
|
||||
mockContext.getNode.mockReturnValue(mockNode);
|
||||
mockNode.typeVersion = 3;
|
||||
mockContext.getExecuteData = vi.fn() as any;
|
||||
(tracing.getTracingConfig as Mock).mockReturnValue({
|
||||
runName: '[Test Workflow] Test Node',
|
||||
metadata: { execution_id: 'test-123', workflow: {}, node: 'Test Node' },
|
||||
|
|
@ -416,8 +415,6 @@ describe('runAgent - tracing configuration', () => {
|
|||
isStreaming: vi.fn().mockReturnValue(true),
|
||||
getExecutionCancelSignal: vi.fn().mockReturnValue(new AbortController().signal),
|
||||
});
|
||||
streamingContext.getExecuteData = vi.fn() as any;
|
||||
|
||||
vi.spyOn(agentExecution, 'loadMemory').mockResolvedValue([]);
|
||||
vi.spyOn(agentExecution, 'processEventStream').mockResolvedValue({
|
||||
output: 'Streamed answer',
|
||||
|
|
@ -568,8 +565,6 @@ describe('runAgent - intermediate steps', () => {
|
|||
isStreaming: vi.fn().mockReturnValue(true),
|
||||
getExecutionCancelSignal: vi.fn().mockReturnValue(new AbortController().signal),
|
||||
});
|
||||
streamingContext.getExecuteData = vi.fn() as any;
|
||||
|
||||
vi.spyOn(agentExecution, 'loadMemory').mockResolvedValue([]);
|
||||
vi.spyOn(agentExecution, 'saveToMemory').mockResolvedValue();
|
||||
vi.spyOn(agentExecution, 'processEventStream').mockResolvedValue({
|
||||
|
|
|
|||
|
|
@ -9,11 +9,16 @@ import {
|
|||
|
||||
import type { ZodObjectAny } from '../../../../types/types';
|
||||
|
||||
/** `IExecuteFunctions` vs `ISupplyDataFunctions` (e.g. agent as a tool); only the former has `getExecuteData`. */
|
||||
/**
|
||||
* `IExecuteFunctions` vs `ISupplyDataFunctions` (e.g. agent as a tool). Discriminate on
|
||||
* `cloneWith`, which only the sub-node context adds: `SupplyDataContext` extends the same
|
||||
* base class as `ExecuteContext`, so every member of that base (`getExecuteData`, …) is on
|
||||
* both and cannot tell them apart.
|
||||
*/
|
||||
export function isExecuteFunctions(
|
||||
context: IExecuteFunctions | ISupplyDataFunctions,
|
||||
): context is IExecuteFunctions {
|
||||
return 'getExecuteData' in context;
|
||||
return !('cloneWith' in context);
|
||||
}
|
||||
|
||||
export async function extractParsedOutput(
|
||||
|
|
|
|||
|
|
@ -45,7 +45,6 @@ describe('toolsAgentExecute', () => {
|
|||
};
|
||||
mockContext.getWorkflow.mockReturnValue({ name: 'Test Workflow' } as any);
|
||||
mockContext.getExecutionId.mockReturnValue('exec-123');
|
||||
mockContext.getExecuteData.mockReturnValue({} as any);
|
||||
});
|
||||
|
||||
it('should process items sequentially when batchSize is not set', async () => {
|
||||
|
|
@ -1185,6 +1184,13 @@ describe('toolsAgentExecute', () => {
|
|||
// @ts-expect-error isStreaming is not supported by SupplyDataFunctions, but mock object still resolves it
|
||||
mockSupplyDataContext.isStreaming = undefined;
|
||||
|
||||
// Sub-node contexts carry `cloneWith`; `isExecuteFunctions` discriminates on it.
|
||||
Object.assign(mockSupplyDataContext, { cloneWith: vi.fn() });
|
||||
|
||||
// Sub-agents are traced too, so the context must answer the same calls as a top-level one.
|
||||
mockSupplyDataContext.getWorkflow.mockReturnValue({ name: 'Test Workflow' } as any);
|
||||
mockSupplyDataContext.getExecutionId.mockReturnValue('exec-123');
|
||||
|
||||
mockSupplyDataContext.logger = {
|
||||
debug: vi.fn(),
|
||||
info: vi.fn(),
|
||||
|
|
|
|||
|
|
@ -58,29 +58,20 @@ function withCommonFields(ctx: MockProxy<IExecuteFunctions>, node: INode): void
|
|||
}
|
||||
|
||||
/**
|
||||
* Sub-agent execution context — `isExecuteFunctions(this)` returns false.
|
||||
*
|
||||
* Intentionally does NOT set `getExecuteData`, so the V3 executor takes the
|
||||
* inline-resolution branch (delegates to `resolveSubAgentRequest`).
|
||||
* The shared top-level `mockContext` defined above gets `getExecuteData`
|
||||
* stamped in `beforeEach`; use this helper to deliberately opt out.
|
||||
* Sub-agent execution context, shaped like a real `SupplyDataContext`: it inherits
|
||||
* `getExecuteData` from `BaseExecuteContext` and adds `cloneWith`, which is what
|
||||
* `isExecuteFunctions` discriminates on.
|
||||
*/
|
||||
function mockSubAgentContext(): MockProxy<IExecuteFunctions> {
|
||||
const ctx = mock<IExecuteFunctions>();
|
||||
withCommonFields(ctx, mockNode);
|
||||
Object.assign(ctx, { getExecuteData: vi.fn(), cloneWith: vi.fn() });
|
||||
return ctx;
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
withCommonFields(mockContext, mockNode);
|
||||
// `isExecuteFunctions` checks `'getExecuteData' in context`. vitest-mock-extended
|
||||
// proxies property access but does not register own properties, so the `in`
|
||||
// check is false by default. Stamp `getExecuteData` here so the shared
|
||||
// `mockContext` exercises the top-level (engine-routed) request path;
|
||||
// sub-agent inline resolution is covered by `mockSubAgentContext()` below
|
||||
// and by `resolveSubAgentRequest.test.ts`.
|
||||
mockContext.getExecuteData = vi.fn() as never;
|
||||
});
|
||||
|
||||
describe('toolsAgentExecute V3 - Execute Function Logic', () => {
|
||||
|
|
@ -226,7 +217,10 @@ describe('toolsAgentExecute V3 - Execute Function Logic', () => {
|
|||
]);
|
||||
});
|
||||
|
||||
it('should delegate to resolveSubAgentRequest when running as a sub-agent (no getExecuteData)', async () => {
|
||||
it('should delegate to resolveSubAgentRequest when running as a sub-agent', async () => {
|
||||
// Regression (AI-2788): a nested AgentToolV3 runs on a real `SupplyDataContext`,
|
||||
// which inherits `getExecuteData`. The sub-agent must still resolve its own
|
||||
// EngineRequest instead of handing it to the engine.
|
||||
const subAgentContext = mockSubAgentContext();
|
||||
|
||||
const mockExecutionContext = {
|
||||
|
|
@ -272,6 +266,8 @@ describe('toolsAgentExecute V3 - Execute Function Logic', () => {
|
|||
expect.objectContaining({ runAgentBatch: expect.any(Function) }),
|
||||
);
|
||||
expect(result).toBe(resolvedOutput);
|
||||
// Tracing metadata belongs to the top-level node, not to a sub-agent run.
|
||||
expect(subAgentContext.setMetadata).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should return request when batch returns tool call request', async () => {
|
||||
|
|
@ -568,10 +564,6 @@ describe('toolsAgentExecute V3 - Execute Function Logic', () => {
|
|||
if (param === 'options.autoSaveHighlightedData') return false;
|
||||
return defaultValue;
|
||||
});
|
||||
// V3 only emits tracing metadata when invoked from an IExecuteFunctions-shaped
|
||||
// context (detected by the presence of `getExecuteData`).
|
||||
mockContext.getExecuteData.mockReturnValue({} as any);
|
||||
|
||||
vi.spyOn(helpers, 'buildExecutionContext').mockResolvedValue(mockExecutionContext);
|
||||
vi.spyOn(helpers, 'executeBatch').mockResolvedValue(mockBatchResult);
|
||||
|
||||
|
|
|
|||
|
|
@ -1,11 +1,12 @@
|
|||
import type { Tool } from '@langchain/classic/tools';
|
||||
import { DynamicStructuredTool } from '@langchain/classic/tools';
|
||||
import { ExecuteContext, SupplyDataContext } from 'n8n-core';
|
||||
import { NodeOperationError } from 'n8n-workflow';
|
||||
import type { INode } from 'n8n-workflow';
|
||||
import type { IExecuteFunctions, INode, ISupplyDataFunctions } from 'n8n-workflow';
|
||||
import { z } from 'zod';
|
||||
|
||||
import type { ZodObjectAny } from '../../../../types/types';
|
||||
import { checkForStructuredTools } from '../agents/utils';
|
||||
import { checkForStructuredTools, isExecuteFunctions } from '../agents/utils';
|
||||
import { getInputs } from '../utils';
|
||||
|
||||
describe('checkForStructuredTools', () => {
|
||||
|
|
@ -275,3 +276,22 @@ describe('getInputs', () => {
|
|||
]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('isExecuteFunctions', () => {
|
||||
// Asserted against the real context classes, not stand-ins: the predicate is duck-typed,
|
||||
// so it silently inverts if `cloneWith` is ever hoisted onto a shared base class. These
|
||||
// two cases fail if that happens. Constructing either class needs the whole execution
|
||||
// engine, so only the prototype chain (all `in` looks at) is built.
|
||||
|
||||
it('should return false for a sub-node context', () => {
|
||||
const context = Object.create(SupplyDataContext.prototype) as ISupplyDataFunctions;
|
||||
|
||||
expect(isExecuteFunctions(context)).toBe(false);
|
||||
});
|
||||
|
||||
it('should return true for a top-level execute context', () => {
|
||||
const context = Object.create(ExecuteContext.prototype) as IExecuteFunctions;
|
||||
|
||||
expect(isExecuteFunctions(context)).toBe(true);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user