diff --git a/packages/@n8n/nodes-langchain/nodes/agents/Agent/agents/ToolsAgent/V2/execute.ts b/packages/@n8n/nodes-langchain/nodes/agents/Agent/agents/ToolsAgent/V2/execute.ts index 93efa944e89..af3c47ad695 100644 --- a/packages/@n8n/nodes-langchain/nodes/agents/Agent/agents/ToolsAgent/V2/execute.ts +++ b/packages/@n8n/nodes-langchain/nodes/agents/Agent/agents/ToolsAgent/V2/execute.ts @@ -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, diff --git a/packages/@n8n/nodes-langchain/nodes/agents/Agent/agents/ToolsAgent/V3/helpers/runAgent.ts b/packages/@n8n/nodes-langchain/nodes/agents/Agent/agents/ToolsAgent/V3/helpers/runAgent.ts index 854e55b9921..b4688944ec6 100644 --- a/packages/@n8n/nodes-langchain/nodes/agents/Agent/agents/ToolsAgent/V3/helpers/runAgent.ts +++ b/packages/@n8n/nodes-langchain/nodes/agents/Agent/agents/ToolsAgent/V3/helpers/runAgent.ts @@ -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; diff --git a/packages/@n8n/nodes-langchain/nodes/agents/Agent/agents/ToolsAgent/V3/helpers/tests/runAgent.test.ts b/packages/@n8n/nodes-langchain/nodes/agents/Agent/agents/ToolsAgent/V3/helpers/tests/runAgent.test.ts index 60a0883b5c6..449a7072026 100644 --- a/packages/@n8n/nodes-langchain/nodes/agents/Agent/agents/ToolsAgent/V3/helpers/tests/runAgent.test.ts +++ b/packages/@n8n/nodes-langchain/nodes/agents/Agent/agents/ToolsAgent/V3/helpers/tests/runAgent.test.ts @@ -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({ diff --git a/packages/@n8n/nodes-langchain/nodes/agents/Agent/agents/utils.ts b/packages/@n8n/nodes-langchain/nodes/agents/Agent/agents/utils.ts index 7b340caf759..5b3b2f44ee9 100644 --- a/packages/@n8n/nodes-langchain/nodes/agents/Agent/agents/utils.ts +++ b/packages/@n8n/nodes-langchain/nodes/agents/Agent/agents/utils.ts @@ -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( diff --git a/packages/@n8n/nodes-langchain/nodes/agents/Agent/test/ToolsAgent/ToolsAgentV2.test.ts b/packages/@n8n/nodes-langchain/nodes/agents/Agent/test/ToolsAgent/ToolsAgentV2.test.ts index 4cdb14d2128..178d0b41be2 100644 --- a/packages/@n8n/nodes-langchain/nodes/agents/Agent/test/ToolsAgent/ToolsAgentV2.test.ts +++ b/packages/@n8n/nodes-langchain/nodes/agents/Agent/test/ToolsAgent/ToolsAgentV2.test.ts @@ -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(), diff --git a/packages/@n8n/nodes-langchain/nodes/agents/Agent/test/ToolsAgent/ToolsAgentV3.test.ts b/packages/@n8n/nodes-langchain/nodes/agents/Agent/test/ToolsAgent/ToolsAgentV3.test.ts index 58c64a7e427..644ae73740c 100644 --- a/packages/@n8n/nodes-langchain/nodes/agents/Agent/test/ToolsAgent/ToolsAgentV3.test.ts +++ b/packages/@n8n/nodes-langchain/nodes/agents/Agent/test/ToolsAgent/ToolsAgentV3.test.ts @@ -58,29 +58,20 @@ function withCommonFields(ctx: MockProxy, 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 { const ctx = mock(); 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); diff --git a/packages/@n8n/nodes-langchain/nodes/agents/Agent/test/utils.test.ts b/packages/@n8n/nodes-langchain/nodes/agents/Agent/test/utils.test.ts index 2c264d6f322..35585be60df 100644 --- a/packages/@n8n/nodes-langchain/nodes/agents/Agent/test/utils.test.ts +++ b/packages/@n8n/nodes-langchain/nodes/agents/Agent/test/utils.test.ts @@ -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); + }); +});