fix(core): Fix AI Agent not seeing tools connected through HITL tools (#34271)

This commit is contained in:
Elias Meire 2026-07-17 11:53:50 +02:00 committed by GitHub
parent 8c1d17ff51
commit bdff8f4be9
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
11 changed files with 616 additions and 290 deletions

View File

@ -316,7 +316,7 @@
"n8n-core": "workspace:*",
"n8n-nodes-base": "workspace:*",
"n8n-workflow": "workspace:*",
"openai": "^6.34.0",
"openai": "catalog:",
"oracledb": "catalog:",
"pg": "catalog:",
"redis": "4.6.14",

View File

@ -219,18 +219,15 @@ export const getConnectedTools = async (
const tools = toolOrToolkit.tools;
// Add metadata to each tool from the toolkit
return tools.map((tool) => {
const sourceNode = parentNodes[index] ?? tool.name;
tool.metadata ??= {};
tool.metadata.isFromToolkit = true;
tool.metadata.sourceNodeName = sourceNode?.name;
tool.metadata.sourceNodeName = parentNodes[index]?.name ?? tool.name;
return tool;
});
} else {
const sourceNode = parentNodes[index] ?? toolOrToolkit.name;
toolOrToolkit.metadata ??= {};
toolOrToolkit.metadata.isFromToolkit = false;
toolOrToolkit.metadata.sourceNodeName = sourceNode?.name;
toolOrToolkit.metadata.sourceNodeName = parentNodes[index]?.name ?? toolOrToolkit.name;
}
return toolOrToolkit;

View File

@ -269,17 +269,17 @@ describe('getConnectedTools', () => {
{
name: 'tool1',
description: 'desc1',
metadata: { isFromToolkit: false, sourceNodeName: undefined },
metadata: { isFromToolkit: false, sourceNodeName: 'tool1' },
},
{
name: 'toolkitTool1',
description: 'toolkitToolDesc1',
metadata: { isFromToolkit: true, sourceNodeName: undefined },
metadata: { isFromToolkit: true, sourceNodeName: 'toolkitTool1' },
},
{
name: 'toolkitTool2',
description: 'toolkitToolDesc2',
metadata: { isFromToolkit: true, sourceNodeName: undefined },
metadata: { isFromToolkit: true, sourceNodeName: 'toolkitTool2' },
},
]);
});
@ -342,6 +342,84 @@ describe('getConnectedTools', () => {
});
});
describe('toolkit detection across duplicated n8n-core copies', () => {
class ForeignStructuredToolkit {
constructor(readonly tools: Tool[]) {}
getTools(): Tool[] {
return this.tools;
}
}
it('should flatten a toolkit whose class identity differs from the local StructuredToolkit', async () => {
const gatedTool = { name: 'gmail_send', description: 'Send an email' } as Tool;
mockExecuteFunctions.getInputConnectionData = vi
.fn()
.mockResolvedValue([new ForeignStructuredToolkit([gatedTool])]);
mockExecuteFunctions.getParentNodes = vi.fn().mockReturnValue([{ name: 'Gmail HITL' }]);
const tools = await getConnectedTools(mockExecuteFunctions, false);
expect(tools).toHaveLength(1);
expect(tools[0].name).toBe('gmail_send');
expect(tools[0].metadata).toEqual({
isFromToolkit: true,
sourceNodeName: 'Gmail HITL',
});
});
it('should keep plain tools untouched when connected alongside a foreign-identity toolkit', async () => {
const directTool = { name: 'direct_tool', description: 'Direct tool' } as Tool;
const gatedTool1 = { name: 'gated_tool_1', description: 'Gated tool 1' } as Tool;
const gatedTool2 = { name: 'gated_tool_2', description: 'Gated tool 2' } as Tool;
mockExecuteFunctions.getInputConnectionData = vi
.fn()
.mockResolvedValue([directTool, new ForeignStructuredToolkit([gatedTool1, gatedTool2])]);
mockExecuteFunctions.getParentNodes = vi
.fn()
.mockReturnValue([{ name: 'Direct Tool' }, { name: 'Gmail HITL' }]);
const tools = await getConnectedTools(mockExecuteFunctions, false);
expect(tools.map((tool) => tool.name)).toEqual([
'direct_tool',
'gated_tool_1',
'gated_tool_2',
]);
expect(tools[0].metadata).toEqual({
isFromToolkit: false,
sourceNodeName: 'Direct Tool',
});
expect(tools[1].metadata).toEqual({
isFromToolkit: true,
sourceNodeName: 'Gmail HITL',
});
expect(tools[2].metadata).toEqual({
isFromToolkit: true,
sourceNodeName: 'Gmail HITL',
});
});
it('should fall back to the tool name when no parent node is found for a toolkit', async () => {
const gatedTool = { name: 'gmail_send', description: 'Send an email' } as Tool;
mockExecuteFunctions.getInputConnectionData = vi
.fn()
.mockResolvedValue([new ForeignStructuredToolkit([gatedTool])]);
mockExecuteFunctions.getParentNodes = vi.fn().mockReturnValue([]);
const tools = await getConnectedTools(mockExecuteFunctions, false);
expect(tools).toHaveLength(1);
expect(tools[0].metadata).toEqual({
isFromToolkit: true,
sourceNodeName: 'gmail_send',
});
});
});
it('should map source node names correctly when a disabled tool node is still connected', async () => {
// getParentNodes returns ALL parents including disabled ones,
// while getInputConnectionData filters disabled nodes out.

View File

@ -1428,6 +1428,24 @@ describe('createHitlToolkit', () => {
expect(result.tools[1].metadata?.sourceNodeName).toBe('HITL Node');
});
it('should extract tools from a toolkit whose class identity differs', () => {
const tool = new DynamicStructuredTool({
name: 'foreign_tool',
description: 'Tool from a foreign toolkit',
schema: z.object({ a: z.string() }),
func: async () => 'result',
metadata: { sourceNodeName: 'Node 1' },
});
const foreignToolkit = { tools: [tool], getTools: () => [tool] };
const result = createHitlToolkit([foreignToolkit as unknown as StructuredToolkit], hitlNode);
expect(result.tools).toHaveLength(1);
expect(result.tools[0].name).toBe('foreign_tool');
expect(result.tools[0].metadata?.sourceNodeName).toBe('HITL Node');
expect(result.tools[0].metadata?.gatedToolNodeName).toBe('Node 1');
});
it('should handle mixed array of tools and toolkits', () => {
const standaloneTool = new DynamicStructuredTool({
name: 'standalone_tool',
@ -1617,6 +1635,17 @@ describe('extendResponseMetadata', () => {
extendResponseMetadata(toolkit, { name: 'HITL Node' } as INode);
expect(toolkit.tools[0].metadata?.sourceNodeName).toBe('HITL Node');
});
it('should extend metadata for toolkits whose class identity differs (duplicated n8n-core)', () => {
const tool = new DynamicStructuredTool({
name: 'test_tool',
description: 'Test tool',
schema: z.object({ input: z.string() }),
func: async () => 'result',
});
const foreignToolkit = { tools: [tool], getTools: () => [tool] };
extendResponseMetadata(foreignToolkit, { name: 'HITL Node' } as INode);
expect(tool.metadata?.sourceNodeName).toBe('HITL Node');
});
it('should extend metadata for tools', () => {
const tool = new DynamicStructuredTool({
name: 'test_tool',
@ -1627,4 +1656,14 @@ describe('extendResponseMetadata', () => {
extendResponseMetadata(tool, { name: 'HITL Node' } as INode);
expect(tool.metadata?.sourceNodeName).toBe('HITL Node');
});
it('should extend metadata for a single tool whose class identity differs (duplicated @langchain/core)', () => {
const foreignTool = {
name: 'foreign_tool',
description: 'Tool from another copy',
invoke: async () => 'x',
metadata: {} as Record<string, unknown>,
};
extendResponseMetadata(foreignTool, { name: 'HITL Node' } as INode);
expect(foreignTool.metadata.sourceNodeName).toBe('HITL Node');
});
});

View File

@ -17,6 +17,17 @@ export class StructuredToolkit extends BaseToolkit {
getTools(): StructuredToolInterface[] {
return this.tools;
}
// The packaged app can materialize more than one copy of n8n-core,
// so `instanceof` misses toolkits built by another copy of n8n-core
static [Symbol.hasInstance](value: unknown): value is StructuredToolkit {
return (
typeof value === 'object' &&
value !== null &&
Array.isArray((value as { tools?: unknown }).tools) &&
typeof (value as { getTools?: unknown }).getTools === 'function'
);
}
}
/**

View File

@ -48,6 +48,19 @@ function ensureArray<T>(value: T | T[] | undefined): T[] {
return Array.isArray(value) ? value : [value];
}
// Tools are matched structurally, not by class identity
function isTool(value: unknown): value is StructuredTool | Tool {
if (value instanceof StructuredTool || value instanceof Tool) return true;
if (value instanceof StructuredToolkit) return false;
return (
typeof value === 'object' &&
value !== null &&
typeof (value as { name?: unknown }).name === 'string' &&
typeof (value as { description?: unknown }).description === 'string' &&
typeof (value as { invoke?: unknown }).invoke === 'function'
);
}
export function createHitlToolkit(
connectedToolsOrToolkits: SupplyDataToolResponse[] | SupplyDataToolResponse | undefined,
hitlNode: INode,
@ -364,7 +377,7 @@ function validateInputConfiguration(
// Extends metadata for tools and toolkits to include the source node name that is used for HITL routing
export function extendResponseMetadata(response: unknown, connectedNode: INode) {
// Ensure sourceNodeName is set for proper routing
if (response instanceof StructuredTool || response instanceof Tool) {
if (isTool(response)) {
response.metadata ??= {};
response.metadata.sourceNodeName = connectedNode.name;
}

View File

@ -1,3 +1,5 @@
import { nanoid } from 'nanoid';
import {
AGENT_NODE_NAME,
AI_LANGUAGE_MODEL_OPENAI_CHAT_MODEL_NODE_NAME,
@ -41,6 +43,21 @@ async function setEditorText(n8n: n8nPage, parameterName: string, value: string)
await codeEditor.fill(value);
}
const ANTHROPIC_RESPONSE = {
id: 'msg_hitl_visibility',
type: 'message',
role: 'assistant',
model: 'claude-sonnet-4-5-20250929',
content: [{ type: 'text', text: 'Here is my answer.' }],
stop_reason: 'end_turn',
stop_sequence: null,
usage: { input_tokens: 10, output_tokens: 5 },
};
interface AnthropicMessagesRequest {
tools?: Array<{ name?: string }>;
}
const hitlForToolsTestConfig = {
capability: {
services: ['proxy'],
@ -148,5 +165,58 @@ test.describe(
await approveButton.click({ button: 'middle' });
await waitForWorkflowSuccess(n8n);
});
// The duplicated-module bug this guards against only reproduces in the packaged image (container mode)
test('agent should send HITL-wrapped tools to the model on the first turn, before approval', async ({
n8n,
api,
services,
}) => {
// Mock Anthropic so the agent completes one turn; only the request matters
await services.proxy.createExpectation({
httpRequest: { method: 'POST', path: '/v1/messages' },
httpResponse: {
statusCode: 200,
headers: { 'Content-Type': ['application/json'] },
body: JSON.stringify(ANTHROPIC_RESPONSE),
},
});
const credential = await api.credentials.createCredential({
name: `Anthropic account ${nanoid()}`,
type: 'anthropicApi',
data: { apiKey: 'test-key' },
});
const imported = await api.workflows.importWorkflowFromFile('hitl-wrapped-tool.json', {
transform: (workflow) => {
const modelNode = workflow.nodes?.find((node) => node.name === 'Anthropic Chat Model');
if (!modelNode) throw new Error('Anthropic Chat Model node not found in fixture');
modelNode.credentials = { anthropicApi: { id: credential.id, name: credential.name } };
return workflow;
},
});
await n8n.start.fromExistingWorkflow(imported.workflowId);
await n8n.canvas.clickManualChatButton();
await n8n.canvas.logsPanel.sendManualChatMessage('What tools do you have access to?');
const getMessagesRequests = async () =>
(await services.proxy.getAllRequestsMade()).filter(
(request) => request.httpRequest?.path === '/v1/messages',
);
await expect
.poll(async () => (await getMessagesRequests()).length, { timeout: 30_000 })
.toBeGreaterThan(0);
const body = (await getMessagesRequests())[0]?.httpRequest?.body as {
json?: AnthropicMessagesRequest;
};
const toolNames = (body?.json?.tools ?? []).map((tool) => tool.name);
expect(toolNames).toContain('Direct_Tool');
expect(toolNames).toContain('Get_secret_message');
});
},
);

View File

@ -0,0 +1,86 @@
{
"name": "HITL wrapped tool visibility",
"nodes": [
{
"parameters": { "options": {} },
"type": "@n8n/n8n-nodes-langchain.chatTrigger",
"typeVersion": 1.4,
"position": [0, 0],
"id": "11111111-1111-1111-1111-111111111111",
"name": "When chat message received",
"webhookId": "aaaaaaaa-1111-1111-1111-111111111111"
},
{
"parameters": { "options": {} },
"type": "@n8n/n8n-nodes-langchain.agent",
"typeVersion": 3.1,
"position": [220, 0],
"id": "22222222-2222-2222-2222-222222222222",
"name": "AI Agent"
},
{
"parameters": {
"model": {
"__rl": true,
"mode": "list",
"value": "claude-sonnet-4-5-20250929",
"cachedResultName": "Claude Sonnet 4.5"
},
"options": {}
},
"type": "@n8n/n8n-nodes-langchain.lmChatAnthropic",
"typeVersion": 1.5,
"position": [80, 220],
"id": "33333333-3333-3333-3333-333333333333",
"name": "Anthropic Chat Model"
},
{
"parameters": { "options": {} },
"type": "@n8n/n8n-nodes-langchain.chatHitlTool",
"typeVersion": 1.3,
"position": [200, 220],
"id": "44444444-4444-4444-4444-444444444444",
"name": "Human Review",
"webhookId": "bbbbbbbb-2222-2222-2222-222222222222"
},
{
"parameters": {
"description": "Returns the secret message",
"jsCode": "return 'secret';"
},
"type": "@n8n/n8n-nodes-langchain.toolCode",
"typeVersion": 1.3,
"position": [140, 440],
"id": "55555555-5555-5555-5555-555555555555",
"name": "Get secret message"
},
{
"parameters": {
"description": "Look up the current time",
"jsCode": "return '12:00';"
},
"type": "@n8n/n8n-nodes-langchain.toolCode",
"typeVersion": 1.3,
"position": [420, 220],
"id": "77777777-7777-7777-7777-777777777777",
"name": "Direct Tool"
}
],
"connections": {
"When chat message received": {
"main": [[{ "node": "AI Agent", "type": "main", "index": 0 }]]
},
"Anthropic Chat Model": {
"ai_languageModel": [[{ "node": "AI Agent", "type": "ai_languageModel", "index": 0 }]]
},
"Human Review": {
"ai_tool": [[{ "node": "AI Agent", "type": "ai_tool", "index": 0 }]]
},
"Get secret message": {
"ai_tool": [[{ "node": "Human Review", "type": "ai_tool", "index": 0 }]]
},
"Direct Tool": {
"ai_tool": [[{ "node": "AI Agent", "type": "ai_tool", "index": 0 }]]
}
}
}

File diff suppressed because it is too large Load Diff

View File

@ -169,7 +169,7 @@ catalog:
nanoid: 3.3.8
nock: 14.0.14
nodemailer: 8.0.10
openai: 6.19.0
openai: 6.46.0
oracledb: 6.10.0
oxlint: ^1.61.0
oxlint-tsgolint: ^0.21.1

View File

@ -26,8 +26,7 @@ const CLOUD = {
// n8n-cloud:packages/instance-controller/charts/n8napp/{n8n,values-v1.yaml}
chartPath: process.env.CLOUD_CHART_PATH || 'packages/instance-controller/charts/n8napp/n8n',
valuesPath:
process.env.CLOUD_CHART_VALUES ||
'packages/instance-controller/charts/n8napp/values-v1.yaml',
process.env.CLOUD_CHART_VALUES || 'packages/instance-controller/charts/n8napp/values-v1.yaml',
};
async function fetchCloudInvocations() {
@ -59,6 +58,68 @@ async function fetchCloudInvocations() {
}
}
// Do not add to this list
const KNOWN_DUPLICATED = new Map([
// pnpm materializes a package once per distinct peer resolution, and
// @langchain/community has optional peers (qdrant, mongodb, redis, …) that only
// nodes-langchain installs, so it splits into two variants, and its dependent
// @n8n/ai-utilities gets materialized twice with it. Versions already align;
// the fix is dropping ai-utilities' @langchain/community dependency
['@n8n/ai-utilities', 2],
]);
function reportFailure(name, err) {
echo(chalk.red(`✗ ${name}`));
echo(chalk.dim(` ${(err.stderr ?? err.message ?? '').slice(0, 1200)}`));
return false;
}
// `pnpm deploy` materializes a workspace package more than once when a peer
// resolves differently per importer, breaking cross-package `instanceof`.
async function runWorkspaceDedupCheck() {
const name = 'workspace packages materialized once in image';
try {
const { stdout } = await $({
timeout: TIMEOUT,
})`docker run --rm --entrypoint ls ${IMAGE} /usr/local/lib/node_modules/n8n/node_modules/.pnpm`;
const variants = new Map();
for (const entry of stdout.split('\n')) {
const match = entry.match(/^(.+?)@file\+packages\+/);
if (!match) continue;
const pkg = match[1].replace('+', '/');
variants.set(pkg, [...(variants.get(pkg) ?? []), entry]);
}
if (variants.size === 0) {
throw new Error(
'no injected workspace packages matched — the .pnpm depPath encoding may have changed; update the regex',
);
}
for (const [pkg, expected] of KNOWN_DUPLICATED) {
const found = variants.get(pkg)?.length ?? 0;
if (found < expected) {
throw new Error(
`expected ${expected} copies of ${pkg} but found ${found} — if the duplication was fixed, update KNOWN_DUPLICATED`,
);
}
}
const duplicated = [...variants].filter(
([pkg, dirs]) => dirs.length > (KNOWN_DUPLICATED.get(pkg) ?? 1),
);
if (duplicated.length > 0) {
const details = duplicated.map(([pkg, dirs]) => `${pkg}:\n ${dirs.join('\n ')}`);
throw new Error(
'workspace package(s) materialized more than once — align the offending peer ' +
`dependency's version across the workspace:\n ${details.join('\n ')}`,
);
}
echo(chalk.green(`✓ ${name} (${variants.size} injected packages)`));
return true;
} catch (err) {
return reportFailure(name, err);
}
}
async function run({ name, user, entrypoint, args }) {
const dockerArgs = ['run', '--rm', '--user', `${user}:${user}`];
if (entrypoint) dockerArgs.push('--entrypoint', entrypoint);
@ -72,16 +133,16 @@ async function run({ name, user, entrypoint, args }) {
echo(chalk.green(`✓ ${name}`));
return true;
} catch (err) {
echo(chalk.red(`✗ ${name}`));
echo(chalk.dim(` ${(err.stderr ?? err.message ?? '').slice(0, 600)}`));
return false;
return reportFailure(name, err);
}
}
const cloud = process.env.SMOKE_SKIP_CLOUD === 'true' ? [] : await fetchCloudInvocations();
if (process.env.SMOKE_SKIP_CLOUD !== 'true' && cloud.length === 0) {
echo(chalk.red('Cloud chart rendered no n8n containers — chart structure may have moved.'));
echo(chalk.dim(' Check CLOUD_CHART_PATH / CLOUD_CHART_VALUES, or run with SMOKE_SKIP_CLOUD=true.'));
echo(
chalk.dim(' Check CLOUD_CHART_PATH / CLOUD_CHART_VALUES, or run with SMOKE_SKIP_CLOUD=true.'),
);
process.exit(1);
}
@ -91,5 +152,5 @@ const invocations = [
];
echo(chalk.bold(`Verifying ${IMAGE} against ${invocations.length} deployment pattern(s)`));
const ok = (await Promise.all(invocations.map(run))).every(Boolean);
const ok = (await Promise.all([...invocations.map(run), runWorkspaceDedupCheck()])).every(Boolean);
if (!ok) process.exit(1);