diff --git a/packages/@n8n/agents/docs/agent-runtime-architecture.md b/packages/@n8n/agents/docs/agent-runtime-architecture.md index 3d5e59767e4..f958be2b219 100644 --- a/packages/@n8n/agents/docs/agent-runtime-architecture.md +++ b/packages/@n8n/agents/docs/agent-runtime-architecture.md @@ -325,7 +325,7 @@ suspension / persistence, repeat until finish or max iterations. ## HITL and suspend/resume -**HITL (approval):** tools can require approval (`requiresApproval` / +**HITL (approval):** tools can require approval (`requireApproval` / `needsApprovalFn`). The runtime treats approval outcomes like resume data: `approve()` / `deny()` delegate to `resume()` with `{ approved: true | false }`. diff --git a/packages/@n8n/agents/package.json b/packages/@n8n/agents/package.json index 6f165f9faaf..51803caa723 100644 --- a/packages/@n8n/agents/package.json +++ b/packages/@n8n/agents/package.json @@ -65,6 +65,7 @@ "@ai-sdk/xai": "^3.0.67", "@modelcontextprotocol/sdk": "catalog:", "@n8n/ai-utilities": "workspace:*", + "@n8n/utils": "workspace:*", "@openrouter/ai-sdk-provider": "catalog:", "ai": "^6.0.116", "ajv": "^8.18.0", diff --git a/packages/@n8n/agents/src/__tests__/agent-configuration.test.ts b/packages/@n8n/agents/src/__tests__/agent-configuration.test.ts index dafd630beca..31c8d07d58d 100644 --- a/packages/@n8n/agents/src/__tests__/agent-configuration.test.ts +++ b/packages/@n8n/agents/src/__tests__/agent-configuration.test.ts @@ -8,6 +8,13 @@ type WithPrivates = { }; describe('Agent.configuration()', () => { + it('does not expose an agent-wide tool approval setting', () => { + const agent = new Agent('test'); + + expect('requireToolApproval' in agent).toBe(false); + expect(agent.snapshot).not.toHaveProperty('requireToolApproval'); + }); + it('is chainable', () => { const agent = new Agent('test'); expect(agent.configuration({ maxIterations: 5 })).toBe(agent); diff --git a/packages/@n8n/agents/src/__tests__/integration/mcp-connection.test.ts b/packages/@n8n/agents/src/__tests__/integration/mcp-connection.test.ts index b3a1037eed1..20705a0332a 100644 --- a/packages/@n8n/agents/src/__tests__/integration/mcp-connection.test.ts +++ b/packages/@n8n/agents/src/__tests__/integration/mcp-connection.test.ts @@ -2,9 +2,8 @@ * Unit-style tests for McpConnection.listTools() approval wrapping. * * These tests use a real in-process MCP SSE server but do NOT require an LLM. - * They verify that the `requireApproval` field on McpServerConfig (and the - * global `shouldRequireToolApproval` constructor flag) correctly wrap the - * appropriate tools with a suspend/resume approval gate. + * They verify that the `requireApproval` field on McpServerConfig correctly + * wraps the appropriate tools with a suspend/resume approval gate. * * Tool names from the test server: echo, add, image (prefixed: tools_echo, tools_add, tools_image). */ @@ -149,38 +148,31 @@ describe('McpConnection.listTools() — requireApproval config', () => { expect(isApprovalWrapped(image!)).toBe(true); }); - // ----------------------------------------------------------------------- - // global shouldRequireToolApproval flag - // ----------------------------------------------------------------------- + it('ignores the removed legacy global flag and only uses server approval config', async () => { + type LegacyMcpConnectionConstructor = new ( + config: ConstructorParameters[0], + legacyGlobalApproval: boolean, + ) => McpConnection; + const LegacyMcpConnection = McpConnection as unknown as LegacyMcpConnectionConstructor; - it('wraps all tools when global shouldRequireToolApproval flag is true', async () => { - connection = new McpConnection({ name: 'tools', url: server.url }, true); - await connection.connect(); - const tools = await connection.listTools(); - - expect(tools.every((t) => isApprovalWrapped(t))).toBe(true); - }); - - // ----------------------------------------------------------------------- - // global flag + config.requireApproval interaction - // ----------------------------------------------------------------------- - - it('wraps all tools when global flag is true even if config.requireApproval names only some tools', async () => { - connection = new McpConnection( + connection = new LegacyMcpConnection( { name: 'tools', url: server.url, requireApproval: ['echo'] }, true, ); await connection.connect(); const tools = await connection.listTools(); - expect(tools.every((t) => isApprovalWrapped(t))).toBe(true); + const echo = tools.find((t) => t.name === 'tools_echo'); + const add = tools.find((t) => t.name === 'tools_add'); + const image = tools.find((t) => t.name === 'tools_image'); + + expect(isApprovalWrapped(echo!)).toBe(true); + expect(isApprovalWrapped(add!)).toBe(false); + expect(isApprovalWrapped(image!)).toBe(false); }); - it('wraps all tools when config.requireApproval: true even if global flag is false', async () => { - connection = new McpConnection( - { name: 'tools', url: server.url, requireApproval: true }, - false, - ); + it('wraps all tools when config.requireApproval: true', async () => { + connection = new McpConnection({ name: 'tools', url: server.url, requireApproval: true }); await connection.connect(); const tools = await connection.listTools(); diff --git a/packages/@n8n/agents/src/__tests__/integration/mcp-runtime.test.ts b/packages/@n8n/agents/src/__tests__/integration/mcp-runtime.test.ts index f13983f9fa0..afaf787f8ea 100644 --- a/packages/@n8n/agents/src/__tests__/integration/mcp-runtime.test.ts +++ b/packages/@n8n/agents/src/__tests__/integration/mcp-runtime.test.ts @@ -1,7 +1,7 @@ /** * Integration tests for MCP lifecycle via McpClient and the Agent builder. * Covers: McpClient constructor validation, connect/listTools/close, tool merge, - * name collision, requireToolApproval, and rich content handling. + * name collision, per-server approval, and rich content handling. * * Tests that don't require a real LLM run unconditionally. * Tests that call agent.generate() / agent.stream() are gated on ANTHROPIC_API_KEY. @@ -295,41 +295,6 @@ describe('MCP tool name collision detection', () => { }); }); -// --------------------------------------------------------------------------- -// requireToolApproval with MCP tools — requires ANTHROPIC_API_KEY -// --------------------------------------------------------------------------- - -describe_llm('requireToolApproval() with MCP tools', () => { - let server: TestServer; - - beforeAll(async () => { - server = await startSseServer(); - }); - - afterAll(async () => { - await server.close(); - }); - - it('suspends the MCP tool call when requireToolApproval is enabled', async () => { - const client = new McpClient([{ name: 'tools', url: server.url }]); - const agent = new Agent('approval-mcp-agent') - .model(getModel('anthropic')) - .instructions('Use tools_echo to echo messages. Be concise.') - .mcp(client) - .requireToolApproval() - .checkpoint('memory'); - - const { stream } = await agent.stream('Echo "needs approval" using tools_echo.'); - const chunks = await collectStreamChunks(stream); - - const suspendedChunks = chunksOfType(chunks, 'tool-call-suspended'); - expect(suspendedChunks.length).toBeGreaterThanOrEqual(1); - expect(suspendedChunks[0].toolName).toBe('tools_echo'); - - await client.close(); - }); -}); - // --------------------------------------------------------------------------- // McpServerConfig.requireApproval — builder validation (no LLM needed) // --------------------------------------------------------------------------- diff --git a/packages/@n8n/agents/src/runtime/__tests__/agent-runtime.test.ts b/packages/@n8n/agents/src/runtime/__tests__/agent-runtime.test.ts index 55def07ce97..bbe0c1a64c7 100644 --- a/packages/@n8n/agents/src/runtime/__tests__/agent-runtime.test.ts +++ b/packages/@n8n/agents/src/runtime/__tests__/agent-runtime.test.ts @@ -2622,7 +2622,7 @@ describe('AgentRuntime — tool approval (HITL wrapper)', () => { }); it('suspends when a tool has .requireApproval() set', async () => { - const approvalTool = new ToolBuilder('delete') + const builtApprovalTool = new ToolBuilder('delete') .description('Delete a record') .input(z.object({ id: z.string() })) .requireApproval() @@ -2630,6 +2630,10 @@ describe('AgentRuntime — tool approval (HITL wrapper)', () => { return await Promise.resolve({ deleted: id }); }) .build(); + const approvalTool = { + ...builtApprovalTool, + metadata: { displayName: 'Delete record' }, + }; generateText.mockResolvedValueOnce(makeGenerateWithToolCall('tc-1', 'delete', { id: 'rec-1' })); @@ -2648,6 +2652,7 @@ describe('AgentRuntime — tool approval (HITL wrapper)', () => { expect(result.pendingSuspend![0].suspendPayload).toMatchObject({ type: 'approval', toolName: 'delete', + displayName: 'Delete record', args: { id: 'rec-1' }, }); }); @@ -2723,6 +2728,136 @@ describe('AgentRuntime — tool approval (HITL wrapper)', () => { ); expect(resumeResult.finishReason).toBe('stop'); }); + + it('does not start tool execution before approval is granted', async () => { + const handler = vi.fn(async ({ id }: { id: string }) => { + return await Promise.resolve({ deleted: id }); + }); + const approvalTool = new ToolBuilder('delete') + .description('Delete a record') + .input(z.object({ id: z.string() })) + .requireApproval() + .handler(handler) + .build(); + + streamText.mockReturnValue({ + fullStream: makeChunkStream([{ type: 'text-delta', textDelta: 'checking...' }]), + finishReason: Promise.resolve('tool-calls'), + usage: Promise.resolve({ inputTokens: 10, outputTokens: 5, totalTokens: 15 }), + response: Promise.resolve({ + messages: [ + { + role: 'assistant', + content: [ + { + type: 'tool-call', + toolCallId: 'tc-1', + toolName: 'delete', + args: { id: 'rec-1' }, + }, + ], + }, + ], + }), + toolCalls: Promise.resolve([ + { toolCallId: 'tc-1', toolName: 'delete', input: { id: 'rec-1' } }, + ]), + }); + + const runtime = new AgentRuntime({ + name: 'test', + model: 'openai/gpt-4o-mini', + instructions: 'test', + tools: [approvalTool], + checkpointStorage: 'memory', + }); + + const { stream: readableStream } = await runtime.stream('Delete record rec-1'); + const chunks = await collectChunks(readableStream); + + expect(handler).not.toHaveBeenCalled(); + expect(chunks.map((c) => c.type)).toContain('tool-call-suspended'); + expect(chunks.map((c) => c.type)).not.toContain('tool-execution-start'); + }); + + it('starts tool execution when conditional approval allows the call immediately', async () => { + const handler = vi.fn(async ({ id }: { id: string }) => { + return await Promise.resolve({ found: id }); + }); + const conditionalApprovalTool = new ToolBuilder('lookup') + .description('Look up a record') + .input( + z.object({ + id: z.string(), + password: z.string(), + nested: z.object({ apiKey: z.string() }), + }), + ) + .needsApprovalFn(async ({ id }: { id: string }) => { + return await Promise.resolve(id === 'secret'); + }) + .handler(handler) + .build(); + const startEvents: Array = []; + const eventBus = new AgentEventBus(); + eventBus.on(AgentEvent.ToolExecutionStart, (event) => { + startEvents.push(event as AgentEventData & { type: AgentEvent.ToolExecutionStart }); + }); + const toolInput = { + id: 'public', + password: 'plain-secret-password', + nested: { apiKey: 'secret-api-key' }, + }; + + streamText + .mockReturnValueOnce({ + fullStream: makeChunkStream([{ type: 'text-delta', textDelta: 'checking...' }]), + finishReason: Promise.resolve('tool-calls'), + usage: Promise.resolve({ inputTokens: 10, outputTokens: 5, totalTokens: 15 }), + response: Promise.resolve({ + messages: [ + { + role: 'assistant', + content: [ + { + type: 'tool-call', + toolCallId: 'tc-1', + toolName: 'lookup', + args: toolInput, + }, + ], + }, + ], + }), + toolCalls: Promise.resolve([{ toolCallId: 'tc-1', toolName: 'lookup', input: toolInput }]), + }) + .mockReturnValueOnce(makeStreamSuccess('Done')); + + const runtime = new AgentRuntime({ + name: 'test', + model: 'openai/gpt-4o-mini', + instructions: 'test', + tools: [conditionalApprovalTool], + eventBus, + checkpointStorage: 'memory', + }); + + const { stream: readableStream } = await runtime.stream('Look up public'); + const chunks = await collectChunks(readableStream); + + expect(handler).toHaveBeenCalledWith( + toolInput, + expect.objectContaining({ toolCallId: 'tc-1' }), + ); + expect(startEvents[0]?.args).toEqual({ + id: 'public', + password: 'plain-secret-password', + nested: { apiKey: 'secret-api-key' }, + }); + expect(chunks.map((c) => c.type)).toContain('tool-execution-start'); + expect(chunks.map((c) => c.type)).toContain('tool-execution-end'); + expect(chunks.map((c) => c.type)).not.toContain('tool-call-suspended'); + }); }); // --------------------------------------------------------------------------- diff --git a/packages/@n8n/agents/src/runtime/agent-runtime.ts b/packages/@n8n/agents/src/runtime/agent-runtime.ts index f67c79e81a7..166b264f5b0 100644 --- a/packages/@n8n/agents/src/runtime/agent-runtime.ts +++ b/packages/@n8n/agents/src/runtime/agent-runtime.ts @@ -143,9 +143,7 @@ function summarizeToolForTelemetry(tool: BuiltTool): Record { description: tool.description, type: tool.mcpTool ? 'mcp' : 'local', ...(tool.mcpServerName ? { mcp_server: tool.mcpServerName } : {}), - ...(tool.suspendSchema || tool.resumeSchema || tool.withDefaultApproval - ? { approval: true } - : {}), + ...(tool.suspendSchema || tool.resumeSchema || tool.approval ? { approval: true } : {}), ...(tool.inputSchema ? { input_schema: getToolInputSchema(tool) } : {}), }; } @@ -161,6 +159,22 @@ function summarizeProviderToolForTelemetry(tool: BuiltProviderTool): Record { const localTools = (config.tools ?? []).map(summarizeToolForTelemetry); const providerTools = (config.providerTools ?? []).map(summarizeProviderToolForTelemetry); @@ -2130,13 +2144,6 @@ export class AgentRuntime { ): Promise { const builtTool = toolMap.get(toolName); - this.eventBus.emit({ - type: AgentEvent.ToolExecutionStart, - toolCallId, - toolName, - args: toolInput, - }); - const makeToolError = (error: unknown): ToolCallOutcome => { this.eventBus.emit({ type: AgentEvent.ToolExecutionEnd, @@ -2219,6 +2226,15 @@ export class AgentRuntime { toolInput = result.data as JSONValue; } + if (shouldEmitToolExecutionStart(builtTool, resumeData)) { + this.eventBus.emit({ + type: AgentEvent.ToolExecutionStart, + toolCallId, + toolName, + args: toolInput, + }); + } + let toolResult: unknown; try { toolResult = await this.withTelemetryToolSpan( @@ -2250,9 +2266,7 @@ export class AgentRuntime { const error = new Error(`Tool ${toolName} has no resume schema`); return makeToolError(error); } - const resumeSchema = isZodSchema(builtTool.resumeSchema) - ? zodToJsonSchema(builtTool.resumeSchema) - : builtTool.resumeSchema; + const resumeSchema = getToolResumeJsonSchema(builtTool); if (!resumeSchema) { return makeToolError(new Error('Invalid resume schema')); } diff --git a/packages/@n8n/agents/src/runtime/mcp-connection.ts b/packages/@n8n/agents/src/runtime/mcp-connection.ts index 341ef85314d..8bc4a2d64b5 100644 --- a/packages/@n8n/agents/src/runtime/mcp-connection.ts +++ b/packages/@n8n/agents/src/runtime/mcp-connection.ts @@ -60,15 +60,12 @@ export class McpConnection { private config: McpServerConfig; - private readonly shouldRequireToolApproval: boolean; - private connectionPromise: Promise | undefined = undefined; private disconnectPromise: Promise | undefined = undefined; private closed = false; - constructor(config: McpServerConfig, requireToolApproval = false) { + constructor(config: McpServerConfig) { this.config = config; - this.shouldRequireToolApproval = requireToolApproval; } async connect(): Promise { @@ -140,13 +137,10 @@ export class McpConnection { * Returns true when a resolved tool should be wrapped with an approval gate. * * A tool needs approval when either: - * - the global `shouldRequireToolApproval` flag (set via Agent.requireToolApproval()) is true, OR * - `config.requireApproval` is `true` (all tools on this server), OR * - `config.requireApproval` is a string array that includes the tool's original (un-prefixed) name. */ private needsApproval(tool: BuiltTool): boolean { - if (this.shouldRequireToolApproval) return true; - const { requireApproval } = this.config; if (requireApproval === true) return true; diff --git a/packages/@n8n/agents/src/sdk/__tests__/delegate-sub-agent-routing.test.ts b/packages/@n8n/agents/src/sdk/__tests__/delegate-sub-agent-routing.test.ts index 54e9908ac9a..5ecf85752a2 100644 --- a/packages/@n8n/agents/src/sdk/__tests__/delegate-sub-agent-routing.test.ts +++ b/packages/@n8n/agents/src/sdk/__tests__/delegate-sub-agent-routing.test.ts @@ -10,8 +10,14 @@ import { type DelegateSubAgentRunner, type DelegateSubAgentRunnerHelpers, } from '../../runtime/delegate-sub-agent-tool'; -import type { BuiltTool, GenerateResult, SerializableAgentState } from '../../types'; +import type { + BuiltTool, + GenerateResult, + InterruptibleToolContext, + SerializableAgentState, +} from '../../types'; import { Agent } from '../agent'; +import { wrapToolForApproval } from '../tool'; const runtimeConfigs: Array> = []; let inlineChildGenerateResult: GenerateResult | undefined; @@ -145,6 +151,38 @@ describe('delegate sub-agent routing', () => { expect(runtimeConfigs).toHaveLength(1); }); + it('preserves required approval when completing inline delegate tools', async () => { + const agent = new Agent('parent') + .model('openai', 'gpt-4o-mini') + .instructions('Delegate when needed.') + .checkpoint('memory') + .tool(wrapToolForApproval(createDelegateSubAgentTool(), { requireApproval: true })) + .tool(makeTool('lookup')); + + const runtimeConfig = await buildAgentConfig(agent); + + expect(runtimeConfigs).toHaveLength(0); + const builtTools = runtimeConfig.tools; + const delegateTool = builtTools?.find((tool) => tool.name === DELEGATE_SUB_AGENT_TOOL_NAME); + expect(delegateTool?.approval?.required).toBe(true); + + const suspend = vi.fn(async (payload: unknown) => { + return await Promise.resolve({ suspended: payload }); + }); + await delegateTool?.handler?.(delegateInput, { + suspend: suspend as unknown as InterruptibleToolContext['suspend'], + resumeData: undefined, + runId: 'parent-run-1', + }); + + expect(suspend).toHaveBeenCalledWith({ + type: 'approval', + toolName: DELEGATE_SUB_AGENT_TOOL_NAME, + args: delegateInput, + }); + expect(runtimeConfigs).toHaveLength(0); + }); + it('lets a host-style runner delegate inline through helpers from tool metadata', async () => { const runInlineSubAgent = vi .fn() diff --git a/packages/@n8n/agents/src/sdk/__tests__/tool.test.ts b/packages/@n8n/agents/src/sdk/__tests__/tool.test.ts index 496716d2fbf..b06e74305d8 100644 --- a/packages/@n8n/agents/src/sdk/__tests__/tool.test.ts +++ b/packages/@n8n/agents/src/sdk/__tests__/tool.test.ts @@ -48,6 +48,7 @@ describe('Tool builder — .requireApproval()', () => { expect(tool.suspendSchema).toBeDefined(); expect(tool.resumeSchema).toBeDefined(); + expect(tool.approval?.required).toBe(true); }); it('build() throws when .requireApproval() is combined with .suspend()/.resume()', () => { @@ -85,6 +86,7 @@ describe('Tool builder — .needsApprovalFn()', () => { expect(tool.suspendSchema).toBeDefined(); expect(tool.resumeSchema).toBeDefined(); + expect(tool.approval?.required).toBe(false); }); it('build() throws when .needsApprovalFn() is combined with .suspend()/.resume()', () => { @@ -174,6 +176,24 @@ describe('wrapToolForApproval — requireApproval: true', () => { }); }); + it('includes display metadata from the wrapped tool object when suspending', async () => { + const baseTool = makeBuiltTool(); + const wrapped = { + ...wrapToolForApproval(baseTool, { requireApproval: true }), + metadata: { displayName: 'Display test tool' }, + }; + const { ctx, suspendMock } = makeCtx(); + + await wrapped.handler!({ id: '1' }, ctx); + + expect(suspendMock).toHaveBeenCalledWith({ + type: 'approval', + toolName: 'testTool', + displayName: 'Display test tool', + args: { id: '1' }, + }); + }); + it('executes original handler when approved on resume', async () => { const baseTool = makeBuiltTool(); const wrapped = wrapToolForApproval(baseTool, { requireApproval: true }); @@ -247,13 +267,44 @@ describe('wrapToolForApproval — needsApprovalFn', () => { expect(suspendMock).not.toHaveBeenCalled(); expect(result).toEqual({ result: 'public' }); }); + + it('emits tool execution start with the original structured args when approval is not needed', async () => { + const baseTool = makeBuiltTool({ + inputSchema: z.object({ + id: z.string(), + password: z.string(), + nested: z.object({ apiKey: z.string() }), + }), + }); + const wrapped = wrapToolForApproval(baseTool, { + needsApprovalFn: async () => await Promise.resolve(false), + }); + const { ctx } = makeCtx(); + const emitEvent = vi.fn(); + ctx.toolCallId = 'tool-call-1'; + ctx.emitEvent = emitEvent; + const input = { + id: 'public', + password: 'plain-secret-password', + nested: { apiKey: 'secret-api-key' }, + }; + + await wrapped.handler!(input, ctx); + + expect(emitEvent).toHaveBeenCalledWith({ + type: 'tool_execution_start', + toolCallId: 'tool-call-1', + toolName: 'testTool', + args: input, + }); + }); }); // --------------------------------------------------------------------------- -// wrapToolForApproval — config: { requireApproval: true } (agent-level wrapping) +// wrapToolForApproval — config: { requireApproval: true } // --------------------------------------------------------------------------- -describe('wrapToolForApproval — config: { requireApproval: true } (agent-level wrapping)', () => { +describe('wrapToolForApproval — config: { requireApproval: true }', () => { it('always suspends regardless of original tool settings', async () => { const baseTool = makeBuiltTool(); const wrapped = wrapToolForApproval(baseTool, { requireApproval: true }); diff --git a/packages/@n8n/agents/src/sdk/agent.ts b/packages/@n8n/agents/src/sdk/agent.ts index bc903081e6b..b4962505808 100644 --- a/packages/@n8n/agents/src/sdk/agent.ts +++ b/packages/@n8n/agents/src/sdk/agent.ts @@ -108,8 +108,6 @@ export interface AgentSnapshot { thinking: ThinkingConfig | null; /** Tool-call concurrency limit if set, otherwise null. */ toolCallConcurrency: number | null; - /** Whether `.requireToolApproval()` was called. */ - requireToolApproval: boolean; } /** @@ -171,8 +169,6 @@ export class Agent implements BuiltAgent, AgentBuilder { private middlewares: AgentMiddleware[] = []; - private requireToolApprovalValue = false; - private mcpClients: McpClient[] = []; private defaultExecutionOptions?: ExecutionOptions; @@ -440,16 +436,6 @@ export class Agent implements BuiltAgent, AgentBuilder { return this; } - /** - * Require human approval before any tool executes. - * Tools that already have .suspend()/.resume() (suspendSchema) are skipped. - * Requires .checkpoint() to be set. - */ - requireToolApproval(): this { - this.requireToolApprovalValue = true; - return this; - } - /** * Attach a workspace to this agent. Workspace tools and instructions * are injected at build time. @@ -580,7 +566,6 @@ export class Agent implements BuiltAgent, AgentBuilder { hasEpisodicMemory: this.memoryConfig?.episodicMemory !== undefined, thinking: this.thinkingConfig ?? null, toolCallConcurrency: this.concurrencyValue ?? null, - requireToolApproval: this.requireToolApprovalValue, }; } @@ -814,27 +799,15 @@ export class Agent implements BuiltAgent, AgentBuilder { finalTools.push(...wsTools); } - let finalStaticTools = finalTools; - let finalDeferredTools = configuredDeferredTools; - if (this.requireToolApprovalValue) { - finalStaticTools = finalTools.map((t) => - RUNTIME_SKILL_TOOL_NAMES.has(t.name) || t.suspendSchema - ? t - : wrapToolForApproval(t, { requireApproval: true }), - ); - finalDeferredTools = configuredDeferredTools.map((t) => - t.suspendSchema ? t : wrapToolForApproval(t, { requireApproval: true }), - ); - } + const finalStaticTools = finalTools; + const finalDeferredTools = configuredDeferredTools; // Validate checkpoint requirement from static tools and known MCP approval config // before attempting any network connections (allows fast failure). const staticNeedsCheckpoint = finalStaticTools.some((t) => t.suspendSchema) || finalDeferredTools.some((t) => t.suspendSchema); - const mcpNeedsCheckpoint = - (this.requireToolApprovalValue && this.mcpClients.length > 0) || - this.mcpClients.some((c) => c.declaresApproval()); + const mcpNeedsCheckpoint = this.mcpClients.some((c) => c.declaresApproval()); if ((staticNeedsCheckpoint || mcpNeedsCheckpoint) && !this.checkpointStore) { throw new Error( `Agent "${this.name}" has tools requiring approval or suspend/resume but no checkpoint storage. ` + @@ -845,15 +818,7 @@ export class Agent implements BuiltAgent, AgentBuilder { // Resolve tools from all MCP clients. const mcpToolLists = await Promise.all(this.mcpClients.map(async (c) => await c.listTools())); - let mcpTools = mcpToolLists.flat(); - - // Apply global requireToolApproval to MCP tools (per-server approval is already - // handled inside McpClient/McpConnection.listTools()). - if (this.requireToolApprovalValue) { - mcpTools = mcpTools.map((t) => - t.suspendSchema ? t : wrapToolForApproval(t, { requireApproval: true }), - ); - } + const mcpTools = mcpToolLists.flat(); // Detect collisions between direct, deferred, and MCP tools. const staticCollisions = findDuplicateToolNames(finalStaticTools); @@ -1022,7 +987,7 @@ export class Agent implements BuiltAgent, AgentBuilder { }, }); - if (tool.withDefaultApproval) { + if (tool.approval?.required === true) { return wrapToolForApproval(completedTool, { requireApproval: true }); } return completedTool; diff --git a/packages/@n8n/agents/src/sdk/mcp-client.ts b/packages/@n8n/agents/src/sdk/mcp-client.ts index 44412913a6e..3af8251fdf9 100644 --- a/packages/@n8n/agents/src/sdk/mcp-client.ts +++ b/packages/@n8n/agents/src/sdk/mcp-client.ts @@ -39,10 +39,8 @@ export class McpClient { /** * @param configs - Server configurations. Each must have either `url` or `command`. * Duplicate names within the list are rejected. - * @param requireToolApproval - When true, every tool from every server is wrapped - * with a human-approval gate (requires `.checkpoint()` on the Agent). */ - constructor(configs: McpServerConfig[], requireToolApproval = false) { + constructor(configs: McpServerConfig[]) { for (const cfg of configs) { if (!cfg.url && !cfg.command) { throw new Error( @@ -63,7 +61,7 @@ export class McpClient { } this.configs = configs; - this.connections = configs.map((cfg) => new McpConnection(cfg, requireToolApproval)); + this.connections = configs.map((cfg) => new McpConnection(cfg)); } /** diff --git a/packages/@n8n/agents/src/sdk/tool.ts b/packages/@n8n/agents/src/sdk/tool.ts index 93c7b56c27b..c7e5b23f2bc 100644 --- a/packages/@n8n/agents/src/sdk/tool.ts +++ b/packages/@n8n/agents/src/sdk/tool.ts @@ -2,6 +2,7 @@ import type { JSONSchema7 } from 'json-schema'; import { z } from 'zod'; import type { BuiltTool, InterruptibleToolContext, ToolContext } from '../types'; +import { AgentEvent } from '../types/runtime/event'; import type { AgentMessage } from '../types/sdk/message'; import type { ToolDescriptor } from '../types/sdk/tool-descriptor'; import type { JSONObject } from '../types/utils/json'; @@ -10,6 +11,7 @@ import { isZodSchema, zodToJsonSchema } from '../utils/zod'; const APPROVAL_SUSPEND_SCHEMA = z.object({ type: z.literal('approval'), toolName: z.string(), + displayName: z.string().optional(), args: z.unknown(), }); @@ -26,10 +28,30 @@ export interface ApprovalConfig { needsApprovalFn?: (args: unknown) => Promise | boolean; } +function emitToolExecutionStart( + tool: BuiltTool, + input: unknown, + ctx: InterruptibleToolContext, +): void { + if (!ctx.toolCallId) return; + ctx.emitEvent?.({ + type: AgentEvent.ToolExecutionStart, + toolCallId: ctx.toolCallId, + toolName: tool.name, + args: input, + }); +} + +export function getToolApprovalDisplayName(tool: BuiltTool): string | undefined { + const metadata = tool.metadata; + const displayName = metadata?.displayName ?? metadata?.workflowName; + return typeof displayName === 'string' && displayName.length > 0 ? displayName : undefined; +} + /** * Wrap a BuiltTool with an approval gate that suspends before execution and * waits for human confirmation. Used by Tool.build() (when .requireApproval() - * or .needsApprovalFn() is set) and by Agent.build() (for global approval). + * or .needsApprovalFn() is set) and per-tool JSON config reconstruction. * * The wrapped tool has suspendSchema/resumeSchema set, making it an * interruptible tool that uses the existing suspend/resume mechanism. @@ -38,13 +60,18 @@ export interface ApprovalConfig { export function wrapToolForApproval(tool: BuiltTool, config: ApprovalConfig): BuiltTool { const originalHandler = tool.handler!; + const hasConditionalApproval = config.needsApprovalFn !== undefined; return { ...tool, - withDefaultApproval: true, + approval: { + required: config.requireApproval === true, + ...(hasConditionalApproval ? { conditional: true } : {}), + }, suspendSchema: APPROVAL_SUSPEND_SCHEMA, resumeSchema: APPROVAL_RESUME_SCHEMA, - handler: async (input, ctx) => { + async handler(this: BuiltTool | undefined, input, ctx) { + const currentTool = this ?? tool; // This handler is always called with InterruptibleToolContext because // wrapToolForApproval adds suspendSchema/resumeSchema. const interruptCtx = ctx as InterruptibleToolContext; @@ -54,14 +81,23 @@ export function wrapToolForApproval(tool: BuiltTool, config: ApprovalConfig): Bu needs = await config.needsApprovalFn(input); } if (needs) { - return await interruptCtx.suspend({ type: 'approval', toolName: tool.name, args: input }); + const displayName = getToolApprovalDisplayName(currentTool); + return await interruptCtx.suspend({ + type: 'approval', + toolName: currentTool.name, + ...(displayName ? { displayName } : {}), + args: input, + }); + } + if (hasConditionalApproval) { + emitToolExecutionStart(currentTool, input, interruptCtx); } return await originalHandler(input, interruptCtx as ToolContext); } const { approved } = interruptCtx.resumeData as z.infer; if (!approved) { - return { declined: true, message: `Tool "${tool.name}" was not approved` }; + return { declined: true, message: `Tool "${currentTool.name}" was not approved` }; } return await originalHandler(input, interruptCtx as ToolContext); }, diff --git a/packages/@n8n/agents/src/types/sdk/agent-builder.ts b/packages/@n8n/agents/src/types/sdk/agent-builder.ts index 983d2569fe9..3a6814c37ec 100644 --- a/packages/@n8n/agents/src/types/sdk/agent-builder.ts +++ b/packages/@n8n/agents/src/types/sdk/agent-builder.ts @@ -23,7 +23,6 @@ export interface AgentBuilder { providerTool(t: BuiltProviderTool): this; thinking(provider: string, config?: Record): this; toolCallConcurrency(n: number): this; - requireToolApproval(): this; memory(m: unknown): this; checkpoint(storage: 'memory' | CheckpointStore): this; inputGuardrail(g: BuiltGuardrail): this; diff --git a/packages/@n8n/agents/src/types/sdk/agent.ts b/packages/@n8n/agents/src/types/sdk/agent.ts index cfe14bc035c..e791783cc8b 100644 --- a/packages/@n8n/agents/src/types/sdk/agent.ts +++ b/packages/@n8n/agents/src/types/sdk/agent.ts @@ -272,11 +272,11 @@ export interface BuiltAgent { options: ResumeOptions & ExecutionOptions, ): Promise; - /** Approve a tool that uses requiresApproval or needsApprovalFn */ + /** Approve a tool that uses requireApproval or needsApprovalFn */ approve(method: 'generate', options: ResumeOptions & ExecutionOptions): Promise; approve(method: 'stream', options: ResumeOptions & ExecutionOptions): Promise; - /** Deny a tool that uses requiresApproval or needsApprovalFn */ + /** Deny a tool that uses requireApproval or needsApprovalFn */ deny(method: 'generate', options: ResumeOptions & ExecutionOptions): Promise; deny(method: 'stream', options: ResumeOptions & ExecutionOptions): Promise; } diff --git a/packages/@n8n/agents/src/types/sdk/mcp.ts b/packages/@n8n/agents/src/types/sdk/mcp.ts index 278513663e6..049b0c1d1db 100644 --- a/packages/@n8n/agents/src/types/sdk/mcp.ts +++ b/packages/@n8n/agents/src/types/sdk/mcp.ts @@ -34,8 +34,7 @@ export interface McpServerConfig { * - `true` — every tool from this server requires approval before execution. * - `string[]` — only the listed tools (by their original, un-prefixed names) * require approval; all other tools from the server run without interruption. - * - `false` / omitted — no per-server approval requirement (the global - * `.requireToolApproval()` flag on the Agent still applies). + * - `false` / omitted — no approval requirement. */ requireApproval?: string[] | boolean; diff --git a/packages/@n8n/agents/src/types/sdk/tool.ts b/packages/@n8n/agents/src/types/sdk/tool.ts index f28e63a2592..8db595a5b07 100644 --- a/packages/@n8n/agents/src/types/sdk/tool.ts +++ b/packages/@n8n/agents/src/types/sdk/tool.ts @@ -82,9 +82,12 @@ export interface BuiltTool { readonly systemInstruction?: string; readonly suspendSchema?: ZodType | JSONSchema7; readonly resumeSchema?: ZodType | JSONSchema7; + readonly approval?: { + readonly required: boolean; + readonly conditional?: boolean; + }; /** When `true`, the handler is called on cancellation with `ctx.cancellation` set instead of being bypassed. */ readonly handleCancellation?: boolean; - readonly withDefaultApproval?: boolean; readonly toMessage?: (output: unknown) => AgentMessage | undefined; /** * Transform the handler output before sending it to the LLM as a tool result. diff --git a/packages/@n8n/api-types/src/agent-builder-interactive.ts b/packages/@n8n/api-types/src/agent-builder-interactive.ts index 4f20ffb0a96..0d39f0cb166 100644 --- a/packages/@n8n/api-types/src/agent-builder-interactive.ts +++ b/packages/@n8n/api-types/src/agent-builder-interactive.ts @@ -11,6 +11,13 @@ import { z } from 'zod'; export const ASK_LLM_TOOL_NAME = 'ask_llm' as const; export const ASK_CREDENTIAL_TOOL_NAME = 'ask_credential' as const; export const ASK_QUESTION_TOOL_NAME = 'ask_question' as const; +/** + * Frontend-only discriminator for generic approval cards. + * + * Approval suspensions keep the underlying tool name on the wire, so the FE + * maps them to this value before dispatching to the approval card component. + */ +export const APPROVAL_TOOL_NAME = 'approval' as const; export const interactiveToolNameSchema = z.union([ z.literal(ASK_LLM_TOOL_NAME), diff --git a/packages/@n8n/api-types/src/agent-builder-tool-node-types.ts b/packages/@n8n/api-types/src/agent-builder-tool-node-types.ts new file mode 100644 index 00000000000..d083dc9dab5 --- /dev/null +++ b/packages/@n8n/api-types/src/agent-builder-tool-node-types.ts @@ -0,0 +1,13 @@ +import { AI_VENDOR_NODE_TYPES, CHAT_TOOL_NODE_TYPE } from 'n8n-workflow'; + +export const AGENT_BUILDER_HIDDEN_AVAILABLE_TOOL_NODE_TYPES: readonly string[] = [ + ...AI_VENDOR_NODE_TYPES.map((nodeType) => `${nodeType}Tool`), + CHAT_TOOL_NODE_TYPE, +]; + +export const AGENT_BUILDER_AVAILABLE_AI_UTILITY_TOOL_NODE_TYPES = [ + 'toolCalculator', + 'toolThink', + '@n8n/n8n-nodes-langchain.toolCalculator', + '@n8n/n8n-nodes-langchain.toolThink', +] as const; diff --git a/packages/@n8n/api-types/src/agents/dto.ts b/packages/@n8n/api-types/src/agents/dto.ts index 3d2dc65add5..8e87efec5df 100644 --- a/packages/@n8n/api-types/src/agents/dto.ts +++ b/packages/@n8n/api-types/src/agents/dto.ts @@ -61,6 +61,12 @@ export class AgentBuildResumeDto extends Z.class({ resumeData: interactiveResumeDataSchema, }) {} +export class AgentChatResumeDto extends Z.class({ + runId: z.string().min(1), + toolCallId: z.string().min(1), + resumeData: z.unknown(), +}) {} + export class AgentDisconnectIntegrationDto extends Z.class({ type: z.string().min(1), credentialId: z.string().min(1), diff --git a/packages/@n8n/api-types/src/agents/index.ts b/packages/@n8n/api-types/src/agents/index.ts index 79186dddb21..cd77f27a8ad 100644 --- a/packages/@n8n/api-types/src/agents/index.ts +++ b/packages/@n8n/api-types/src/agents/index.ts @@ -9,10 +9,15 @@ export * from './provider-capabilities'; export type * from './sub-agent.schema'; export * from './types'; export type { AgentSseEvent, AgentSseMessage, ToolSuspendedPayload } from '../agent-sse'; +export { + AGENT_BUILDER_AVAILABLE_AI_UTILITY_TOOL_NODE_TYPES, + AGENT_BUILDER_HIDDEN_AVAILABLE_TOOL_NODE_TYPES, +} from '../agent-builder-tool-node-types'; export { ASK_LLM_TOOL_NAME, ASK_CREDENTIAL_TOOL_NAME, ASK_QUESTION_TOOL_NAME, + APPROVAL_TOOL_NAME, interactiveToolNameSchema, askLlmInputSchema, askLlmResumeSchema, diff --git a/packages/@n8n/instance-ai/src/mcp/mcp-client-manager.ts b/packages/@n8n/instance-ai/src/mcp/mcp-client-manager.ts index 3a1bfd3b8b2..95b126b83b5 100644 --- a/packages/@n8n/instance-ai/src/mcp/mcp-client-manager.ts +++ b/packages/@n8n/instance-ai/src/mcp/mcp-client-manager.ts @@ -196,7 +196,7 @@ export class McpClientManager { logger: Logger | undefined, source: string, ): Promise { - const client = new McpClient(buildNativeMcpConfigs(configs), true); + const client = new McpClient(buildNativeMcpConfigs(configs)); this.clientsByKey.set(clientKey, client); const registry = toolsToRegistry(await client.listTools()); diff --git a/packages/cli/src/modules/agents/__tests__/agents-tools.service.test.ts b/packages/cli/src/modules/agents/__tests__/agents-tools.service.test.ts index 4e950b33011..b8df630ac7f 100644 --- a/packages/cli/src/modules/agents/__tests__/agents-tools.service.test.ts +++ b/packages/cli/src/modules/agents/__tests__/agents-tools.service.test.ts @@ -1,5 +1,9 @@ import { mock } from 'jest-mock-extended'; import type { CredentialProvider } from '@n8n/agents'; +import { + AGENT_BUILDER_AVAILABLE_AI_UTILITY_TOOL_NODE_TYPES, + AGENT_BUILDER_HIDDEN_AVAILABLE_TOOL_NODE_TYPES, +} from '@n8n/api-types'; import type { Logger } from '@n8n/backend-common'; import { validateNodeConfig } from '@n8n/workflow-sdk'; @@ -173,6 +177,18 @@ describe('AgentsToolsService', () => { expect(isAgentToolNodeType('@n8n/n8n-nodes-langchain.agent')).toBe(false); }); + it('rejects hidden agent-builder tool node IDs', () => { + for (const nodeType of AGENT_BUILDER_HIDDEN_AVAILABLE_TOOL_NODE_TYPES) { + expect(isAgentToolNodeType(nodeType)).toBe(false); + } + }); + + it('allows shared AI utility tool node IDs', () => { + for (const nodeType of AGENT_BUILDER_AVAILABLE_AI_UTILITY_TOOL_NODE_TYPES) { + expect(isAgentToolNodeType(nodeType)).toBe(true); + } + }); + it('does not allow MCP tool nodes', () => { expect(isAgentToolNodeType('@n8n/n8n-nodes-langchain.mcpClientTool')).toBe(false); expect(isAgentToolNodeType('@n8n/mcp-registry.notion')).toBe(false); diff --git a/packages/cli/src/modules/agents/__tests__/agents.controller.test.ts b/packages/cli/src/modules/agents/__tests__/agents.controller.test.ts index 0ebc9f08ac2..0d9b5bed97f 100644 --- a/packages/cli/src/modules/agents/__tests__/agents.controller.test.ts +++ b/packages/cli/src/modules/agents/__tests__/agents.controller.test.ts @@ -132,6 +132,7 @@ describe('AgentsController route access scopes', () => { ['deleteTask', 'agent:update'], ['runTaskNow', 'agent:execute'], ['listVersions', 'agent:read'], + ['chatResume', 'agent:execute'], ])('%s uses %s', (handlerName, scope) => { expect(metadata.routes.get(handlerName)?.accessScope?.scope).toBe(scope); }); diff --git a/packages/cli/src/modules/agents/__tests__/execution-recorder.test.ts b/packages/cli/src/modules/agents/__tests__/execution-recorder.test.ts index e105c70c822..fb94a103048 100644 --- a/packages/cli/src/modules/agents/__tests__/execution-recorder.test.ts +++ b/packages/cli/src/modules/agents/__tests__/execution-recorder.test.ts @@ -265,6 +265,70 @@ describe('ExecutionRecorder', () => { expect(record.assistantResponse).toBe('Hello world'); }); }); + + describe('secret scrubbing', () => { + it('sanitizes tool inputs and outputs in flat records and timeline entries', () => { + const recorder = new ExecutionRecorder(); + + recorder.record( + makeToolCallChunk('lookup', { + query: 'project status', + password: 'plain-secret-password', + nested: { apiKey: 'secret-api-key' }, + }), + ); + recorder.record( + makeToolResultChunk('lookup', { + result: 'password=hunter2', + authorization: 'Bearer secret-token-value', + }), + ); + + const record = recorder.getMessageRecord(); + const timelineEntry = record.timeline.find((e) => e.type === 'tool-call'); + + expect(record.toolCalls[0]).toEqual({ + name: 'lookup', + input: { + query: 'project status', + password: '[REDACTED]', + nested: { apiKey: '[REDACTED]' }, + }, + output: { + result: '[REDACTED]', + authorization: '[REDACTED]', + }, + }); + expect(timelineEntry).toMatchObject({ + input: { + query: 'project status', + password: '[REDACTED]', + nested: { apiKey: '[REDACTED]' }, + }, + output: { + result: '[REDACTED]', + authorization: '[REDACTED]', + }, + }); + }); + + it('sanitizes error outputs before recording them', () => { + const recorder = new ExecutionRecorder(); + + recorder.record(makeToolCallChunk('lookup', { query: 'project status' })); + recorder.record({ + type: 'tool-result', + toolCallId: 'tc1', + toolName: 'lookup', + output: new Error('password=hunter2'), + isError: true, + }); + + const record = recorder.getMessageRecord(); + + expect(record.toolCalls[0].output).toEqual({ error: '[REDACTED]' }); + }); + }); }); function wfTool(name: string, id: string, wfName: string, trigger = 'manual'): BuiltTool { @@ -422,6 +486,45 @@ describe('ExecutionRecorder — node-tool $fromAI resolution', () => { const tc = rec.getMessageRecord().timeline.find((e) => e.type === 'tool-call')!; expect((tc.nodeParameters as Record).field).toBe('={{ $fromAI(unbalanced '); }); + + it('sanitizes resolved node parameters before recording them', () => { + const registry = buildToolRegistry([ + nodeTool('send_secret', 'n8n-nodes-base.http', { + password: "={{ $fromAI('password', 'Password', 'string') }}", + body: { + apiKey: "={{ $fromAI('apiKey', 'API key', 'string') }}", + message: "={{ $fromAI('message', 'Message', 'string') }}", + }, + }), + ]); + const rec = new ExecutionRecorder(registry); + + rec.record({ + type: 'tool-call', + toolCallId: 't1', + toolName: 'send_secret', + input: { + password: 'plain-secret-password', + apiKey: 'secret-api-key', + message: 'visible', + }, + } as never); + rec.record({ + type: 'tool-result', + toolCallId: 't1', + toolName: 'send_secret', + output: { ok: true }, + } as never); + + const tc = rec.getMessageRecord().timeline.find((e) => e.type === 'tool-call')!; + expect(tc.nodeParameters).toEqual({ + password: '[REDACTED]', + body: { + apiKey: '[REDACTED]', + message: 'visible', + }, + }); + }); }); describe('ExecutionRecorder — workflow-tool timeline tags', () => { diff --git a/packages/cli/src/modules/agents/__tests__/from-json-config.test.ts b/packages/cli/src/modules/agents/__tests__/from-json-config.test.ts index a5a587fd370..a89f4939688 100644 --- a/packages/cli/src/modules/agents/__tests__/from-json-config.test.ts +++ b/packages/cli/src/modules/agents/__tests__/from-json-config.test.ts @@ -449,7 +449,7 @@ describe('buildFromJson()', () => { const tool = agent.declaredTools.find((t) => t.name === 'run_workflow'); expect(tool).toBeDefined(); - expect(tool!.withDefaultApproval).toBe(true); + expect(tool!.approval?.required).toBe(true); }); it('wraps node tool with approval when requireApproval is true', async () => { @@ -485,7 +485,7 @@ describe('buildFromJson()', () => { const tool = agent.declaredTools.find((t) => t.name === 'my_node_tool'); expect(tool).toBeDefined(); - expect(tool!.withDefaultApproval).toBe(true); + expect(tool!.approval?.required).toBe(true); }); it('does not wrap workflow tool with approval when requireApproval is not set', async () => { @@ -513,7 +513,7 @@ describe('buildFromJson()', () => { const tool = agent.declaredTools.find((t) => t.name === 'run_workflow'); expect(tool).toBeDefined(); - expect(tool!.withDefaultApproval).toBeUndefined(); + expect(tool!.approval).toBeUndefined(); }); it('falls back to marker tool when resolveTool is not provided for workflow tools', async () => { diff --git a/packages/cli/src/modules/agents/agents-tools.service.ts b/packages/cli/src/modules/agents/agents-tools.service.ts index 7035ad67001..3781f6f3d9c 100644 --- a/packages/cli/src/modules/agents/agents-tools.service.ts +++ b/packages/cli/src/modules/agents/agents-tools.service.ts @@ -1,5 +1,9 @@ import type { BuiltTool, CredentialProvider } from '@n8n/agents'; import { Tool } from '@n8n/agents/tool'; +import { + AGENT_BUILDER_AVAILABLE_AI_UTILITY_TOOL_NODE_TYPES, + AGENT_BUILDER_HIDDEN_AVAILABLE_TOOL_NODE_TYPES, +} from '@n8n/api-types'; import { Logger } from '@n8n/backend-common'; import { Service } from '@n8n/di'; import { validateNodeConfig } from '@n8n/workflow-sdk'; @@ -28,6 +32,11 @@ type NodeRequest = */ export const isExecutableNodeType = (nodeId: string): boolean => !isTriggerNodeType(nodeId); +const hiddenAgentToolNodeTypes = new Set(AGENT_BUILDER_HIDDEN_AVAILABLE_TOOL_NODE_TYPES); +const aiUtilityAgentToolNodeTypes = new Set( + AGENT_BUILDER_AVAILABLE_AI_UTILITY_TOOL_NODE_TYPES, +); + /** * Node IDs the agent builder should surface when configuring node-backed * tools. For regular nodes marked `usableAsTool`, the loader creates a @@ -36,6 +45,8 @@ export const isExecutableNodeType = (nodeId: string): boolean => !isTriggerNodeT * not approval-gated workflow steps. Provider nodes (OpenAI etc.) are * admitted via the explicit whitelist — they ship the full vendor API * (image, audio, …) but lack the `usableAsTool` flag. + * Frontend-hidden tool variants are excluded here too, so `search_nodes` + * cannot offer tools the modal intentionally hides. * * Exported as a stable reference so the catalog service can cache its * filtered search tool per filter identity. @@ -44,9 +55,14 @@ export const isAgentToolNodeType = (nodeId: string): boolean => { if (!isExecutableNodeType(nodeId)) { return false; } + if (hiddenAgentToolNodeTypes.has(nodeId)) { + return false; + } + + const isAllowedAiUtilityTool = aiUtilityAgentToolNodeTypes.has(nodeId); const isAllowedTool = isToolType(nodeId, { includeHitl: false }) && !isMcpToolNodeType(nodeId); const isAllowedProviderNode = isAgentProviderNode(nodeId); - return isAllowedTool || isAllowedProviderNode; + return isAllowedAiUtilityTool || isAllowedTool || isAllowedProviderNode; }; const MCP_CLIENT_TOOL_NODE_TYPE = '@n8n/n8n-nodes-langchain.mcpClientTool'; diff --git a/packages/cli/src/modules/agents/agents.controller.ts b/packages/cli/src/modules/agents/agents.controller.ts index 6942d3cb408..a5559c0c9c5 100644 --- a/packages/cli/src/modules/agents/agents.controller.ts +++ b/packages/cli/src/modules/agents/agents.controller.ts @@ -1,6 +1,7 @@ import { AgentBuildResumeDto, AgentChatMessageDto, + AgentChatResumeDto, AgentIntegrationSchema, type AgentBuilderMessagesResponse, type AgentIntegrationStatusResponse, @@ -607,7 +608,7 @@ export class AgentsController { } try { - await pumpChunks( + const suspended = await pumpChunks( this.agentsService.executeForChat({ agentId, projectId, @@ -620,7 +621,9 @@ export class AgentsController { }), send, ); - send({ type: 'done', sessionId: threadId }); + if (!suspended) { + send({ type: 'done', sessionId: threadId }); + } } catch (error) { const errorMessage = error instanceof Error ? error.message : 'Chat failed'; send({ type: 'error', message: errorMessage }); @@ -629,6 +632,42 @@ export class AgentsController { res.end(); } + @Post('/:agentId/chat/resume', { usesTemplates: true }) + @ProjectScope('agent:execute') + async chatResume( + req: AuthenticatedRequest<{ projectId: string }>, + res: FlushableResponse, + @Param('agentId') agentId: string, + @Body payload: AgentChatResumeDto, + ) { + const { projectId } = req.params; + const { runId, toolCallId, resumeData } = payload; + const { send } = initSseStream(res); + + try { + const suspended = await pumpChunks( + this.agentsService.resumeForChat({ + agentId, + projectId, + runId, + toolCallId, + resumeData, + userId: req.user.id, + usePublishedVersion: false, + }), + send, + ); + if (!suspended) { + send({ type: 'done' }); + } + } catch (error) { + const errorMessage = error instanceof Error ? error.message : 'Resume failed'; + send({ type: 'error', message: errorMessage }); + } + + res.end(); + } + @Get('/:agentId/chat/:threadId/messages') @ProjectScope('agent:read') async getChatMessages( diff --git a/packages/cli/src/modules/agents/agents.service.ts b/packages/cli/src/modules/agents/agents.service.ts index 09d51697fbd..9bb25fa9d71 100644 --- a/packages/cli/src/modules/agents/agents.service.ts +++ b/packages/cli/src/modules/agents/agents.service.ts @@ -117,6 +117,10 @@ export interface ResumeForChatConfig { runId: string; toolCallId: string; resumeData: unknown; + /** n8n user ID for in-app preview chat resumes. */ + userId?: string; + /** Defaults to true for external integrations; preview chat passes false. */ + usePublishedVersion?: boolean; /** * Required when the suspended turn invoked a platform-injected tool * (e.g. `rich_interaction`). Without it, `getRuntime` rebuilds the agent @@ -865,7 +869,16 @@ export class AgentsService { * a human-in-the-loop action (button click, modal submission). */ async *resumeForChat(config: ResumeForChatConfig): AsyncGenerator { - const { agentId, projectId, runId, toolCallId, resumeData, integrationType } = config; + const { + agentId, + projectId, + runId, + toolCallId, + resumeData, + integrationType, + userId, + usePublishedVersion = true, + } = config; const checkpointStatus = await this.n8nCheckpointStorage.getStatus(runId); if (checkpointStatus.status === 'expired') { @@ -886,7 +899,8 @@ export class AgentsService { const runtime = await this.getRuntime({ agentId, projectId, - usePublishedVersion: true, + ...(userId ? { n8nUserId: userId } : {}), + usePublishedVersion, integrationType, }); @@ -896,7 +910,7 @@ export class AgentsService { const resultStream = await agentInstance.resume('stream', resumeData, { runId, toolCallId, - executionCounter: this.createAgentExecutionCounter({ agentId }), + executionCounter: this.createAgentExecutionCounter({ agentId, userId }), }); for await (const value of streamAgentChunks(resultStream.stream)) { diff --git a/packages/cli/src/modules/agents/execution-recorder.ts b/packages/cli/src/modules/agents/execution-recorder.ts index 359b6432034..401d85352b5 100644 --- a/packages/cli/src/modules/agents/execution-recorder.ts +++ b/packages/cli/src/modules/agents/execution-recorder.ts @@ -1,4 +1,5 @@ import type { StreamChunk } from '@n8n/agents'; +import { scrubSecretsInText } from '@n8n/utils'; import { extractFromAICalls, isFromAIOnlyExpression } from 'n8n-workflow'; import type { ToolRegistry } from './tool-registry'; @@ -138,6 +139,44 @@ function normaliseToolErrorOutput(output: unknown): unknown { return output; } +const REDACTED_VALUE = '[REDACTED]'; +const CIRCULAR_VALUE = '[Circular]'; + +function isSecretKey(key: string): boolean { + const probe = `${key}=value`; + return scrubSecretsInText(probe) !== probe; +} + +function sanitizeExecutionLogValue(value: unknown, seen = new WeakSet()): unknown { + if (typeof value === 'string') return scrubSecretsInText(value); + + if (Array.isArray(value)) { + if (seen.has(value)) return CIRCULAR_VALUE; + seen.add(value); + const sanitized = value.map((item) => sanitizeExecutionLogValue(item, seen)); + seen.delete(value); + return sanitized; + } + + if (!isRecord(value)) return value; + + if (seen.has(value)) return CIRCULAR_VALUE; + seen.add(value); + + const sanitized: Record = {}; + for (const [key, item] of Object.entries(value)) { + sanitized[key] = isSecretKey(key) ? REDACTED_VALUE : sanitizeExecutionLogValue(item, seen); + } + + seen.delete(value); + return sanitized; +} + +function sanitizeExecutionLogRecord(value: unknown): Record | undefined { + const sanitized = sanitizeExecutionLogValue(value); + return isRecord(sanitized) ? sanitized : undefined; +} + export interface RecordedUsage { promptTokens: number; completionTokens: number; @@ -182,7 +221,7 @@ export type TimelineEvent = | { type: 'suspension'; toolName: string; toolCallId: string; timestamp: number }; function isRecord(v: unknown): v is Record { - return typeof v === 'object' && v !== null; + return typeof v === 'object' && v !== null && !Array.isArray(v); } /** @@ -340,7 +379,8 @@ export class ExecutionRecorder { private recordToolCall(toolCallId: string, name: string, input: unknown): void { this.flushTextBuffer(); - this.toolCalls.push({ name, input, output: undefined, toolCallId }); + const recordedInput = sanitizeExecutionLogValue(input); + this.toolCalls.push({ name, input: recordedInput, output: undefined, toolCallId }); const entry = this.registry.get(name); // Resolve both `$fromAI(...)` placeholders and simple `={{ $json.x }}` @@ -351,14 +391,14 @@ export class ExecutionRecorder { input !== null && typeof input === 'object' ? (input as Record) : {}; const resolvedNodeParameters = entry?.nodeParameters !== undefined - ? (resolveTemplatesInValue(entry.nodeParameters, llmArgs) as Record) + ? sanitizeExecutionLogRecord(resolveTemplatesInValue(entry.nodeParameters, llmArgs)) : undefined; this.timeline.push({ type: 'tool-call', kind: entry?.kind ?? 'tool', name, toolCallId, - input, + input: recordedInput, output: undefined as unknown, startTime: Date.now(), endTime: 0, @@ -442,7 +482,9 @@ export class ExecutionRecorder { output: unknown, isError: boolean, ): void { - const recordedOutput = isError ? normaliseToolErrorOutput(output) : output; + const recordedOutput = sanitizeExecutionLogValue( + isError ? normaliseToolErrorOutput(output) : output, + ); const pendingFlat = this.findOpenToolCall(toolCallId, name); if (pendingFlat) { @@ -495,7 +537,10 @@ export class ExecutionRecorder { nodeType: entry?.nodeType, nodeTypeVersion: entry?.nodeTypeVersion, nodeDisplayName: entry?.nodeDisplayName, - nodeParameters: entry?.nodeParameters, + nodeParameters: + entry?.nodeParameters !== undefined + ? sanitizeExecutionLogRecord(entry.nodeParameters) + : undefined, }; if (synthesized.kind === 'workflow' && isRecord(recordedOutput)) { const execId = recordedOutput.executionId; diff --git a/packages/cli/src/modules/agents/integrations/__tests__/agent-chat-bridge.test.ts b/packages/cli/src/modules/agents/integrations/__tests__/agent-chat-bridge.test.ts index e108299f5a7..88c1b3abfc5 100644 --- a/packages/cli/src/modules/agents/integrations/__tests__/agent-chat-bridge.test.ts +++ b/packages/cli/src/modules/agents/integrations/__tests__/agent-chat-bridge.test.ts @@ -216,6 +216,70 @@ describe('AgentChatBridge — consumeStream', () => { expect(thread.post).toHaveBeenNthCalledWith(3, { markdown: 'After resume.' }); }); + it('includes tool approval details when posting a suspension card', async () => { + const { bot, handlers } = makeBot(); + const thread = makeThread(); + componentMapper.toCard.mockResolvedValue({ kind: 'card' } as never); + + const agentExecutor = makeAgentExecutor([ + { + type: 'tool-call-suspended', + runId: 'run-1', + toolCallId: 'tool-1', + toolName: 'approval', + suspendPayload: { + type: 'approval', + toolName: 'giphy-gif-search', + displayName: 'GIPHY GIF Search', + args: { query: 'project status', limit: 3 }, + }, + }, + { type: 'finish', finishReason: 'stop' }, + ]); + + new AgentChatBridge( + bot as unknown as ChatBotLike, + 'agent-1', + agentExecutor as never, + componentMapper, + logger, + 'project-1', + bufferedIntegration, + ); + + await handlers.mention!(thread, { text: 'hi', author: { userId: 'u1', userName: 'user1' } }); + + expect(componentMapper.toCard).toHaveBeenCalledWith( + { + title: 'Approval required', + components: [ + { + type: 'section', + text: 'The agent wants to run this tool: GIPHY GIF Search', + }, + { + type: 'fields', + fields: [ + { label: 'Tool', value: 'GIPHY GIF Search' }, + { + label: 'Input', + value: '{\n "query": "project status",\n "limit": 3\n}', + }, + ], + }, + { type: 'button', label: 'Approve', value: 'true', style: 'primary' }, + { type: 'button', label: 'Deny', value: 'false', style: 'danger' }, + ], + }, + 'run-1', + 'tool-1', + undefined, + undefined, + 'test-buffered', + ); + expect(thread.post).toHaveBeenCalledWith({ card: { kind: 'card' } }); + }); + it('does not post when the buffer is only whitespace', async () => { const { bot, handlers } = makeBot(); const thread = makeThread(); diff --git a/packages/cli/src/modules/agents/integrations/agent-chat-bridge.ts b/packages/cli/src/modules/agents/integrations/agent-chat-bridge.ts index 7a94f9749ea..a8299f35519 100644 --- a/packages/cli/src/modules/agents/integrations/agent-chat-bridge.ts +++ b/packages/cli/src/modules/agents/integrations/agent-chat-bridge.ts @@ -64,6 +64,14 @@ interface AgentExecutor { const SLACK_THINKING_STATUS = 'Thinking...'; const SLACK_STATUS_RETRY_DELAY_MS = 750; +const APPROVAL_INPUT_MAX_LENGTH = 1500; + +interface ApprovalSuspendPayload { + type: 'approval'; + toolName: string; + displayName?: string; + args?: unknown; +} function isIntegrationActionSuspendPayload(value: unknown): boolean { return ( @@ -74,6 +82,68 @@ function isIntegrationActionSuspendPayload(value: unknown): boolean { ); } +function isRecord(value: unknown): value is Record { + return value !== null && typeof value === 'object' && !Array.isArray(value); +} + +function isApprovalSuspendPayload(value: unknown): value is ApprovalSuspendPayload { + return ( + isRecord(value) && + value.type === 'approval' && + typeof value.toolName === 'string' && + value.toolName.length > 0 + ); +} + +function truncateApprovalInput(value: string): string { + if (value.length <= APPROVAL_INPUT_MAX_LENGTH) return value; + return `${value.slice(0, APPROVAL_INPUT_MAX_LENGTH)}...`; +} + +function stringifyApprovalInput(value: unknown): string | undefined { + if (value === undefined) return undefined; + + if (typeof value === 'string') { + return truncateApprovalInput(value); + } + + try { + const serialized = JSON.stringify(value, null, 2); + return truncateApprovalInput(serialized ?? String(value)); + } catch { + return truncateApprovalInput(String(value)); + } +} + +function getApprovalToolLabel(payload: ApprovalSuspendPayload): string { + return typeof payload.displayName === 'string' && payload.displayName.length > 0 + ? payload.displayName + : payload.toolName; +} + +function buildApprovalCardPayload(payload: ApprovalSuspendPayload): { + title: string; + components: Array<{ type: string; [key: string]: unknown }>; +} { + const toolLabel = getApprovalToolLabel(payload); + const fields: Array<{ label: string; value: string }> = [{ label: 'Tool', value: toolLabel }]; + const input = stringifyApprovalInput(payload.args); + + if (input) { + fields.push({ label: 'Input', value: input }); + } + + return { + title: 'Approval required', + components: [ + { type: 'section', text: `The agent wants to run this tool: ${toolLabel}` }, + { type: 'fields', fields }, + { type: 'button', label: 'Approve', value: 'true', style: 'primary' }, + { type: 'button', label: 'Deny', value: 'false', style: 'danger' }, + ], + }; +} + function toIntegrationMessageSubject( subject: MessageSubject | null | undefined, ): IntegrationMessageSubject | undefined { @@ -660,7 +730,9 @@ export class AgentChatBridge { components: Array<{ type: string; [key: string]: unknown }>; }; - if (hasComponents) { + if (isApprovalSuspendPayload(payload)) { + cardPayload = buildApprovalCardPayload(payload); + } else if (hasComponents) { cardPayload = payload as RichSuspendPayload; } else { // Plain suspend payload — auto-generate approve/deny buttons @@ -1286,10 +1358,6 @@ function stripSlackSelfMention(text: string, userId: string): string { .trim(); } -function isRecord(value: unknown): value is Record { - return value !== null && typeof value === 'object' && !Array.isArray(value); -} - function isSlackAssistantStatusAdapter(value: unknown): value is SlackAssistantStatusAdapter { return isRecord(value) && typeof value.setAssistantStatus === 'function'; } diff --git a/packages/cli/src/modules/agents/tools/__tests__/node-tool-factory.test.ts b/packages/cli/src/modules/agents/tools/__tests__/node-tool-factory.test.ts index 0342c93c622..4383aa3ef56 100644 --- a/packages/cli/src/modules/agents/tools/__tests__/node-tool-factory.test.ts +++ b/packages/cli/src/modules/agents/tools/__tests__/node-tool-factory.test.ts @@ -157,4 +157,56 @@ describe('resolveNodeTool → tool name sanitization', () => { expect(tool.inputSchema).toBe(inputSchema); expect(introspectSupplyDataToolSchema).toHaveBeenCalled(); }); + + it('uses a string-compatible object schema when native string tool introspection returns null', async () => { + const executeInline = jest.fn().mockResolvedValue({ status: 'success', data: [] }); + const introspectSupplyDataToolSchema = jest.fn().mockResolvedValue(null); + Container.set(NodeTypes, { + getByNameAndVersion: jest.fn().mockReturnValue({ + description: { description: 'Think about something' }, + supplyData: jest.fn(), + }), + } as unknown as NodeTypes); + + const tool = await resolveNodeTool( + { + ...baseToolSchema, + description: 'Use this to think', + node: { + nodeType: '@n8n/n8n-nodes-langchain.toolThink', + nodeTypeVersion: 1.1, + nodeParameters: {}, + }, + }, + { + executor: { + executeInline, + introspectSupplyDataToolSchema, + } as unknown as EphemeralNodeExecutor, + projectId: 'p1', + }, + ); + + const schema = tool.inputSchema as z.ZodType; + const parsedString = schema.safeParse('thinking about this problem'); + const parsedObject = schema.safeParse({ input: 'thinking about this problem' }); + + expect(parsedString).toEqual({ + success: true, + data: { input: 'thinking about this problem' }, + }); + expect(parsedObject).toEqual({ + success: true, + data: { input: 'thinking about this problem' }, + }); + + if (!parsedString.success) throw new Error('Expected string input to parse'); + await tool.handler?.(parsedString.data, {} as never); + + expect(executeInline).toHaveBeenCalledWith( + expect.objectContaining({ + inputData: [{ json: { input: 'thinking about this problem' } }], + }), + ); + }); }); diff --git a/packages/cli/src/modules/agents/tools/node-tool-factory.ts b/packages/cli/src/modules/agents/tools/node-tool-factory.ts index 48cca1ad731..46c7c949eae 100644 --- a/packages/cli/src/modules/agents/tools/node-tool-factory.ts +++ b/packages/cli/src/modules/agents/tools/node-tool-factory.ts @@ -4,7 +4,7 @@ import { createZodSchemaFromArgs, extractFromAIParameters } from '@n8n/ai-utilit import type { JSONSchema7 } from 'json-schema'; import type { IDataObject, INodeParameters } from 'n8n-workflow'; import { isToolType, nodeNameToToolName } from 'n8n-workflow'; -import type { z } from 'zod'; +import { z } from 'zod'; import type { EphemeralNodeExecutor } from '@/node-execution'; import { NodeTypes } from '@/node-types'; @@ -48,11 +48,21 @@ function resolveToolNodeType(nodeType: string, nodeTypeVersion: number): string } } +function createNativeStringToolInputSchema(description: string): z.ZodType { + return z.preprocess( + (value) => (typeof value === 'string' ? { input: value } : value), + z.object({ + input: z.string().describe(description), + }), + ); +} + /** * Native tool nodes expose a LangChain tool via `supplyData`. The shape of its * schema depends on the class: - * - Base `Tool` / `DynamicTool` (toolWikipedia, toolCalculator, etc.) has no - * `.schema` — the input contract is the implicit `{ input: string }`. + * - Base `Tool` / `DynamicTool` (toolWikipedia, toolCalculator, etc.) is + * invoked by LangChain with a string but advertised to LLM providers as + * `{ input: string }`. * - `StructuredTool` / `DynamicStructuredTool` / `N8nTool` (ToolCode with a * configured schema, ToolWorkflow v1/v2, McpClientTool) carries a Zod * `.schema` with multi-field requirements. @@ -62,7 +72,9 @@ function resolveToolNodeType(nodeType: string, nodeTypeVersion: number): string * placeholders so the tool schema cannot drift from runtime params. * 2. Otherwise, ask the executor to instantiate the LangChain tool and * report its `.schema`. Zod and JSON schemas can be handed to the SDK as-is. - * 3. Fall back to the `{ input: string }` shape for plain `Tool` nodes. + * 3. Fall back to a string-compatible `{ input: string }` shape for plain + * `Tool` nodes. This keeps provider-facing schemas object-shaped while + * accepting raw string tool calls from models that emit the legacy shape. */ async function resolveInputSchema( toolSchema: Extract, @@ -100,19 +112,11 @@ async function resolveInputSchema( if (introspected) return introspected as NodeToolInputSchema; - return { - type: 'object', - properties: { - input: { - type: 'string', - description: - toolSchema.description ?? - nodeType.description.description ?? - `The query or input text to pass to ${toolSchema.node.nodeType}.`, - }, - }, - required: ['input'], - }; + return createNativeStringToolInputSchema( + toolSchema.description ?? + nodeType.description.description ?? + `The query or input text to pass to ${toolSchema.node.nodeType}.`, + ); } return { type: 'object', properties: {} }; diff --git a/packages/cli/src/node-execution/__tests__/ephemeral-node-executor.test.ts b/packages/cli/src/node-execution/__tests__/ephemeral-node-executor.test.ts index f158fa18a77..3f23fff5358 100644 --- a/packages/cli/src/node-execution/__tests__/ephemeral-node-executor.test.ts +++ b/packages/cli/src/node-execution/__tests__/ephemeral-node-executor.test.ts @@ -1,5 +1,6 @@ import { Logger } from '@n8n/backend-common'; import { mockInstance } from '@n8n/backend-test-utils'; +import { DynamicTool } from '@langchain/core/tools'; import { CredentialsRepository, SharedCredentialsRepository, @@ -549,11 +550,16 @@ describe('EphemeralNodeExecutor', () => { }); it('returns null when the tool has no structured schema (base Tool/DynamicTool)', async () => { + const dynamicTool = new DynamicTool({ + name: 'thinking_tool', + description: 'Think about something', + func: async (input) => input, + }); nodeTypes.getByNameAndVersion.mockReturnValue( mock({ description: toolDescription, supplyData: jest.fn().mockResolvedValue({ - response: { invoke: jest.fn() }, + response: dynamicTool, }), }), ); diff --git a/packages/cli/src/node-execution/ephemeral-node-executor.ts b/packages/cli/src/node-execution/ephemeral-node-executor.ts index 37f07113a00..5032d3adea2 100644 --- a/packages/cli/src/node-execution/ephemeral-node-executor.ts +++ b/packages/cli/src/node-execution/ephemeral-node-executor.ts @@ -1,7 +1,7 @@ import { Logger } from '@n8n/backend-common'; import { CredentialsRepository, SharedCredentialsRepository } from '@n8n/db'; import { Service } from '@n8n/di'; -import type { Tool } from '@langchain/core/tools'; +import { Tool as LangChainTool, type Tool as LangChainToolType } from '@langchain/core/tools'; import { ExecuteContext, StructuredToolkit, SupplyDataContext } from 'n8n-core'; import type { CloseFunction, @@ -404,7 +404,7 @@ export class EphemeralNodeExecutor { private async withSupplyDataTool( tool: EphemeralWorkflowToolLike, inputItems: INodeExecutionData[], - onTool: (response: Tool | StructuredToolkit) => Promise | T, + onTool: (response: LangChainToolType | StructuredToolkit) => Promise | T, ): Promise<{ ok: true; value: T } | { ok: false; error: string }> { const parts = await this.buildEphemeralContextParts(tool, inputItems); const closeFunctions: CloseFunction[] = []; @@ -430,7 +430,10 @@ export class EphemeralNodeExecutor { try { const supplyDataResult = await nodeType.supplyData.call(context, 0); - const response = supplyDataResult.response as Tool | StructuredToolkit | undefined; + const response = supplyDataResult.response as + | LangChainToolType + | StructuredToolkit + | undefined; if (response instanceof StructuredToolkit) { return { ok: true, value: await onTool(response) }; @@ -522,6 +525,7 @@ export class EphemeralNodeExecutor { // through to its `{ input: string }` default; proper per-method // introspection ships with multi-tool expansion. if (response instanceof StructuredToolkit) return null; + if (response instanceof LangChainTool) return null; const maybeSchema = (response as unknown as { schema?: unknown }).schema; return maybeSchema ?? null; }); diff --git a/packages/cli/src/tool-generation/__tests__/hitl-tools.test.ts b/packages/cli/src/tool-generation/__tests__/hitl-tools.test.ts index a99d6af28c4..fa9d373afee 100644 --- a/packages/cli/src/tool-generation/__tests__/hitl-tools.test.ts +++ b/packages/cli/src/tool-generation/__tests__/hitl-tools.test.ts @@ -296,7 +296,9 @@ describe('hitl-tools', () => { ); expect(messageProp).toBeDefined(); expect(messageProp?.type).toBe('string'); - expect(messageProp?.default).toBe('=The agent wants to call {{ $tool.name }}'); + expect(messageProp?.default).toBe( + '=The agent wants to run this tool: {{ $tool.name }}\n\nInput:\n{{ $tool.parameters }}', + ); }); it('should replace original message property with HITL message', () => { @@ -318,7 +320,9 @@ describe('hitl-tools', () => { ); // Should only have one message property (our HITL one, not the original) expect(messageProps).toHaveLength(1); - expect(messageProps[0].default).toBe('=The agent wants to call {{ $tool.name }}'); + expect(messageProps[0].default).toBe( + '=The agent wants to run this tool: {{ $tool.name }}\n\nInput:\n{{ $tool.parameters }}', + ); }); it('should set codex categories correctly for HITL', () => { diff --git a/packages/cli/src/tool-generation/hitl-tools.ts b/packages/cli/src/tool-generation/hitl-tools.ts index 9c29b11beef..b70b17e5ca7 100644 --- a/packages/cli/src/tool-generation/hitl-tools.ts +++ b/packages/cli/src/tool-generation/hitl-tools.ts @@ -88,7 +88,8 @@ function filterHitlToolProperties( displayName: 'Message', name: 'message', type: 'string', - default: '=The agent wants to call {{ $tool.name }}', + default: + '=The agent wants to run this tool: {{ $tool.name }}\n\nInput:\n{{ $tool.parameters }}', required: true, typeOptions: { rows: 3 }, description: diff --git a/packages/frontend/@n8n/i18n/src/locales/en.json b/packages/frontend/@n8n/i18n/src/locales/en.json index 234590523d9..19739f2669b 100644 --- a/packages/frontend/@n8n/i18n/src/locales/en.json +++ b/packages/frontend/@n8n/i18n/src/locales/en.json @@ -6221,6 +6221,13 @@ "agents.chat.askCredential.skip": "Skip", "agents.chat.toolNames.webSearch": "Web search", "agents.chat.toolNames.searchKnowledge": "Search knowledge", + "agents.chat.approval.title": "Approval required", + "agents.chat.approval.description": "The agent wants to run the {toolName} tool.", + "agents.chat.approval.approve": "Approve", + "agents.chat.approval.reject": "Reject", + "agents.chat.approval.approved": "Approved", + "agents.chat.approval.rejected": "Rejected", + "agents.chat.approval.inputPlaceholder": "Approve or reject to continue", "agents.chat.delegate.label": "Sub-agent · {name}", "agents.chat.delegate.labelFallback": "Sub-agent", "agents.chat.delegate.childSuspendUnsupported": "Sub-agent requested user input, which is not supported for delegated runs yet.", @@ -6296,6 +6303,9 @@ "agents.tools.search.placeholder": "Search tools", "agents.tools.availableTools": "Available tools ({count})", "agents.tools.availableWorkflows": "Workflows ({count})", + "agents.tools.availableAiTools": "AI tools ({count})", + "agents.tools.availableN8nTools": "n8n tools ({count})", + "agents.tools.availableExternalTools": "External tools ({count})", "agents.tools.noResults": "No tools available", "agents.tools.noResults.withQuery": "No tools match “{query}”. Try a different keyword.", "agents.tools.configure": "Configure", @@ -6303,6 +6313,7 @@ "agents.tools.added": "Tool added", "agents.tools.connected": "Connected", "agents.tools.addCredentials": "Add credentials", + "agents.tools.needsApproval": "Needs approval", "agents.tools.noCredentials": "No credentials", "agents.tools.workflow.incompatible.title": "Workflow is not compatible", "agents.tools.workflow.incompatible.message": "Workflow \"{name}\" contains nodes that can't be used as an agent tool: {nodes}.", @@ -6317,6 +6328,8 @@ "agents.toolConfig.workflow.allOutputs.hint": "When off, only the last node's output is returned.", "agents.toolConfig.save": "Save", "agents.toolConfig.cancel": "Cancel", + "agents.toolConfig.approval.label": "Require approval", + "agents.toolConfig.approval.hint": "Pause before this tool runs until a human approves it.", "agents.new.title": "New agent", "agents.new.defaultName": "New Agent", "agents.new.startBlank": "Start from blank", @@ -6348,7 +6361,7 @@ "agents.builder.agent.instructions.label": "Instructions", "agents.builder.agent.instructions.characterCount": "{count} characters", "agents.builder.advanced.title": "Advanced", - "agents.builder.advanced.description": "Execution tuning — reasoning depth, tool parallelism, and approval gating.", + "agents.builder.advanced.description": "Execution tuning — reasoning depth and tool parallelism.", "agents.builder.advanced.webSearch.label": "Web search", "agents.builder.advanced.webSearch.hint": "Let the model search the web when it needs up-to-date information.", "agents.builder.advanced.webSearch.maxUses.label": "Maximum searches", @@ -6372,8 +6385,6 @@ "agents.builder.advanced.reasoningEffort.label": "Reasoning effort", "agents.builder.advanced.concurrency.label": "Tool call concurrency", "agents.builder.advanced.concurrency.hint": "How many tool calls the agent can run in parallel.", - "agents.builder.advanced.approval.label": "Require tool approval", - "agents.builder.advanced.approval.hint": "Pause before running tool calls until a human approves.", "agents.builder.advanced.recentMessages.label": "Session memory window", "agents.builder.advanced.recentMessages.hint": "How many recent messages from this thread the agent sees on each turn.", "agents.builder.advanced.recentMessages.memoryDisabledTooltip": "Enable Session Memory in the Memory section to configure the window.", diff --git a/packages/frontend/editor-ui/src/features/agents/__tests__/AgentToolConfigModal.test.ts b/packages/frontend/editor-ui/src/features/agents/__tests__/AgentToolConfigModal.test.ts index a8e659730f4..5f318f88c4d 100644 --- a/packages/frontend/editor-ui/src/features/agents/__tests__/AgentToolConfigModal.test.ts +++ b/packages/frontend/editor-ui/src/features/agents/__tests__/AgentToolConfigModal.test.ts @@ -4,7 +4,7 @@ import { createTestingPinia } from '@pinia/testing'; import { mockedStore } from '@/__tests__/utils'; import { useUIStore } from '@/app/stores/ui.store'; import { fireEvent, waitFor } from '@testing-library/vue'; -import { defineComponent, onMounted, ref, nextTick } from 'vue'; +import { defineComponent, onMounted, nextTick } from 'vue'; import AgentToolConfigModal from '../components/AgentToolConfigModal.vue'; import type { AgentJsonToolRef, CustomToolEntry } from '../types'; @@ -31,18 +31,19 @@ function createToolSettingsStub(emitValid: boolean) { setup(props, { emit, expose }) { // Expose what the modal reads from ref(...). The stub carries through // the initialNode's credentials so we can assert the round-trip keeps them. + const node = { + id: 'mocked-uuid', + name: props.initialNode?.name ?? '', + type: props.initialNode?.type ?? '', + typeVersion: props.initialNode?.typeVersion ?? 1, + parameters: { edited: true }, + credentials: props.initialNode?.credentials, + position: [0, 0], + }; expose({ - node: ref({ - id: 'mocked-uuid', - name: props.initialNode?.name ?? '', - type: props.initialNode?.type ?? '', - typeVersion: props.initialNode?.typeVersion ?? 1, - parameters: { edited: true }, - credentials: props.initialNode?.credentials, - position: [0, 0], - }), + getNode: () => node, handleChangeName: vi.fn(), - nodeTypeDescription: ref({ name: 'n8n-nodes-base.slack', displayName: 'Slack' }), + getNodeTypeDescription: () => ({ name: 'n8n-nodes-base.slack', displayName: 'Slack' }), }); onMounted(() => { emit('update:valid', emitValid); @@ -50,19 +51,21 @@ function createToolSettingsStub(emitValid: boolean) { }); return {}; }, - template: '
', + template: ` +
+ `, }); } function createWorkflowToolConfigStub(emitValid: boolean) { return defineComponent({ - props: ['initialRef'], - emits: ['update:valid', 'update:node-name'], + props: ['initialRef', 'showApprovalSetting', 'approvalRequired'], + emits: ['update:valid', 'update:node-name', 'update:approvalRequired'], setup(props, { emit, expose }) { expose({ - name: ref(props.initialRef?.name ?? ''), - description: ref(props.initialRef?.description ?? ''), - allOutputs: ref(props.initialRef?.allOutputs ?? false), + getName: () => props.initialRef?.name ?? '', + getDescription: () => props.initialRef?.description ?? '', + getAllOutputs: () => props.initialRef?.allOutputs ?? false, handleChangeName: vi.fn(), }); onMounted(() => { @@ -71,7 +74,16 @@ function createWorkflowToolConfigStub(emitValid: boolean) { }); return {}; }, - template: '
', + template: ` +
+
+ `, }); } @@ -141,9 +153,15 @@ function renderModal({ stubs: { ElDialog: ElDialogStub, NodeIcon: { template: '
' }, - NodeToolSettingsContent: createToolSettingsStub(valid), - WorkflowToolConfigContent: createWorkflowToolConfigStub(valid), - AgentCustomToolViewer: { + AgentToolConfigNodeContent: createToolSettingsStub(valid), + AgentToolConfigWorkflowContent: createWorkflowToolConfigStub(valid), + N8nSwitch2: { + props: ['modelValue'], + emits: ['update:modelValue'], + template: + '', + }, + N8nCollapsiblePanel: { + props: ['modelValue', 'disableAnimation'], + emits: ['update:modelValue'], + template: ` +
+ +
+
+ `, + }, + N8nHeading: { + props: ['tag', 'size', 'color'], + template: '', + }, + N8nIcon: { + props: ['icon', 'size'], + template: '', + }, + N8nIconButton: { + props: ['icon', 'variant', 'text', 'ariaLabel'], + emits: ['click'], + template: + '', + }, + N8nInput: { + props: ['modelValue', 'placeholder', 'clearable'], + emits: ['update:modelValue'], + template: + '', + }, + N8nText: { + props: ['color', 'size'], + template: '', + }, + N8nTooltip: { + props: ['content'], + template: '
', + }, +})); + vi.mock('@n8n/i18n', () => { const i18n = { baseText: (key: string, opts?: { interpolate?: Record }) => { if (opts?.interpolate) { const { count, query } = opts.interpolate as { count?: number; query?: string }; if (key === 'agents.tools.availableTools') return `Available tools (${count})`; + if (key === 'agents.tools.availableAiTools') return `AI tools (${count})`; + if (key === 'agents.tools.availableN8nTools') return `n8n tools (${count})`; + if (key === 'agents.tools.availableExternalTools') return `External tools (${count})`; if (key === 'agents.tools.availableWorkflows') return `Workflows (${count})`; if (key === 'agents.tools.noResults.withQuery') return `No tools match “${query}”`; } @@ -54,6 +112,7 @@ vi.mock('@n8n/i18n', () => { 'agents.tools.configure': 'Configure', 'agents.tools.added': 'Tool added', 'agents.tools.addCredentials': 'Add credentials', + 'agents.tools.needsApproval': 'Needs approval', }; return map[key] ?? key; }, @@ -135,6 +194,110 @@ const WIKIPEDIA: INodeTypeDescription = { credentials: [], }; +function makeAiToolFixture( + name: string, + displayName: string, + description: string, +): INodeTypeDescription { + return { + ...SLACK, + displayName, + name, + description, + defaults: { name: displayName }, + credentials: [], + codex: { + categories: ['AI'], + subcategories: { + AI: ['Tools'], + Tools: ['Other Tools'], + }, + }, + }; +} + +const OPENAI: INodeTypeDescription = { + ...SLACK, + displayName: 'OpenAI', + name: '@n8n/n8n-nodes-langchain.openAi', + description: 'Use OpenAI models', + defaults: { name: 'OpenAI' }, + inputs: ['main'], + credentials: [], +}; + +const CODE_TOOL: INodeTypeDescription = { + ...SLACK, + displayName: 'Code Tool', + name: '@n8n/n8n-nodes-langchain.toolCode', + description: 'Run custom code', + defaults: { name: 'Code Tool' }, + credentials: [], + codex: { + categories: ['AI'], + subcategories: { + AI: ['Tools'], + Tools: ['Recommended Tools'], + }, + }, +}; + +const CALCULATOR = makeAiToolFixture('toolCalculator', 'Calculator', 'Do math'); + +const THINK_TOOL = makeAiToolFixture('toolThink', 'Think Tool', 'Pause to think'); + +const ANTHROPIC_TOOL = makeAiToolFixture( + '@n8n/n8n-nodes-langchain.anthropicTool', + 'Anthropic Tool', + 'Interact with Anthropic AI models', +); + +const GOOGLE_GEMINI_TOOL = makeAiToolFixture( + '@n8n/n8n-nodes-langchain.googleGeminiTool', + 'Google Gemini Tool', + 'Interact with Google Gemini AI models', +); + +const MINIMAX_TOOL = makeAiToolFixture( + '@n8n/n8n-nodes-langchain.minimaxTool', + 'MiniMax Tool', + 'Interact with MiniMax AI models', +); + +const MOONSHOT_TOOL = makeAiToolFixture( + '@n8n/n8n-nodes-langchain.moonshotTool', + 'Moonshot Kimi Tool', + 'Interact with Moonshot Kimi AI models', +); + +const OLLAMA_TOOL = makeAiToolFixture( + '@n8n/n8n-nodes-langchain.ollamaTool', + 'Ollama Tool', + 'Interact with Ollama AI models', +); + +const CHAT_TOOL = makeAiToolFixture( + '@n8n/n8n-nodes-langchain.chatTool', + 'Chat Tool', + 'Send a message into the chat', +); + +const MCP_TOOL: INodeTypeDescription = { + ...SLACK, + displayName: 'GitHub MCP', + name: 'mcpClientTool', + description: 'Connect to an MCP server', + defaults: { name: 'GitHub MCP' }, + credentials: [], + codex: { + categories: ['AI'], + subcategories: { + AI: ['Tools'], + Tools: ['Model Context Protocol'], + }, + }, +}; + const NODE_WITH_INPUTS: INodeTypeDescription = { ...SLACK, name: 'n8n-nodes-base.subagent', @@ -224,6 +387,17 @@ describe('AgentToolsModal', () => { if (name === GMAIL.name) return GMAIL; if (name === GITHUB.name) return GITHUB; if (name === WIKIPEDIA.name) return WIKIPEDIA; + if (name === OPENAI.name) return OPENAI; + if (name === CODE_TOOL.name) return CODE_TOOL; + if (name === CALCULATOR.name) return CALCULATOR; + if (name === THINK_TOOL.name) return THINK_TOOL; + if (name === ANTHROPIC_TOOL.name) return ANTHROPIC_TOOL; + if (name === GOOGLE_GEMINI_TOOL.name) return GOOGLE_GEMINI_TOOL; + if (name === MINIMAX_TOOL.name) return MINIMAX_TOOL; + if (name === MOONSHOT_TOOL.name) return MOONSHOT_TOOL; + if (name === OLLAMA_TOOL.name) return OLLAMA_TOOL; + if (name === CHAT_TOOL.name) return CHAT_TOOL; + if (name === MCP_TOOL.name) return MCP_TOOL; if (name === NODE_WITH_INPUTS.name) return NODE_WITH_INPUTS; return null; }); @@ -293,7 +467,7 @@ describe('AgentToolsModal', () => { it('lists available node-type tools, excluding nodes that take main inputs', () => { const { getByTestId, queryByText } = renderComponent(defaultProps()); - const available = getByTestId('agent-tools-available-list'); + const available = getByTestId('agent-tools-available-external-list'); expect(available.textContent).toContain('Slack'); expect(available.textContent).toContain('Gmail'); expect(available.textContent).toContain('GitHub'); @@ -312,6 +486,14 @@ describe('AgentToolsModal', () => { expect(connected.textContent).toContain(SLACK.name); }); + it('shows an approval badge for configured tools that require approval', () => { + const { getByTestId } = renderComponent( + defaultProps([{ ...toolRef(SLACK.name), requireApproval: true }]), + ); + const connected = getByTestId('agent-tools-connected-list'); + expect(connected.textContent).toContain('Needs approval'); + }); + it('surfaces the "Add credentials" chip on rows missing credentials', () => { const tool = toolRef(GMAIL.name, { credentials: undefined }); const { queryByTestId } = renderComponent(defaultProps([tool])); @@ -328,7 +510,7 @@ describe('AgentToolsModal', () => { // Users can add a 2nd Slack tool with a different name + credentials. // The config modal enforces tool-name uniqueness on save. const { getByTestId } = renderComponent(defaultProps([toolRef(SLACK.name)])); - const available = getByTestId('agent-tools-available-list'); + const available = getByTestId('agent-tools-available-external-list'); expect(available.textContent).toContain('Slack'); expect(available.textContent).toContain('Gmail'); }); @@ -341,7 +523,7 @@ describe('AgentToolsModal', () => { await typeInSearch(container, 'gmail'); await waitFor(() => { - const available = getByTestId('agent-tools-available-list'); + const available = getByTestId('agent-tools-available-external-list'); expect(available.textContent).toContain('Gmail'); expect(available.textContent).not.toContain('GitHub'); }); @@ -354,7 +536,7 @@ describe('AgentToolsModal', () => { await waitFor(() => { expect(queryByText(/No tools match.*zzzzz/)).not.toBeNull(); - expect(queryByTestId('agent-tools-available-list')).toBeNull(); + expect(queryByTestId('agent-tools-available-external-list')).toBeNull(); }); }); @@ -362,7 +544,7 @@ describe('AgentToolsModal', () => { const onConfirm = vi.fn(); const { getByTestId } = renderComponent(defaultProps([], onConfirm)); - const available = getByTestId('agent-tools-available-list'); + const available = getByTestId('agent-tools-available-external-list'); const addButton = available.querySelector('button'); expect(addButton).not.toBeNull(); await fireEvent.click(addButton!); @@ -387,7 +569,7 @@ describe('AgentToolsModal', () => { const onConfirm = vi.fn(); const { getByTestId } = renderComponent(defaultProps([], onConfirm)); - const available = getByTestId('agent-tools-available-list'); + const available = getByTestId('agent-tools-available-external-list'); await fireEvent.click(available.querySelector('button')!); expect(uiStore.openModalWithData).not.toHaveBeenCalled(); @@ -425,7 +607,7 @@ describe('AgentToolsModal', () => { const onConfirm = vi.fn(); const { getByTestId } = renderComponent(defaultProps([existing], onConfirm)); - const available = getByTestId('agent-tools-available-list'); + const available = getByTestId('agent-tools-available-external-list'); await fireEvent.click(available.querySelector('button')!); const [tools] = onConfirm.mock.calls[0]; @@ -436,7 +618,7 @@ describe('AgentToolsModal', () => { const onConfirm = vi.fn(); const { getByTestId } = renderComponent(defaultProps([], onConfirm)); - const available = getByTestId('agent-tools-available-list'); + const available = getByTestId('agent-tools-available-external-list'); await fireEvent.click(available.querySelector('button')!); const [payload] = (uiStore.openModalWithData as ReturnType).mock.calls[0]; @@ -464,7 +646,73 @@ describe('AgentToolsModal', () => { it('shows the available tools count in the section heading', () => { const { getByTestId } = renderComponent(defaultProps()); const wrapper = getByTestId('agent-tools-list'); - expect(wrapper.textContent).toContain('Available tools (3)'); + expect(wrapper.textContent).toContain('External tools (3)'); + }); + + it('groups available entries into workflows, AI tools, n8n tools, and external tools', async () => { + nodeTypesStore.visibleNodeTypesByOutputConnectionTypeNames = { + [NodeConnectionTypes.AiTool]: [ + SLACK.name, + CODE_TOOL.name, + MCP_TOOL.name, + NODE_WITH_INPUTS.name, + ], + }; + seedWorkflows([makeWorkflow({ id: 'wf-1', name: 'Daily sales digest' })]); + + const { getByTestId, queryByText } = renderComponent({ + props: { + modalName: MODAL_NAME, + data: { tools: [], projectId: 'p-42', onConfirm: vi.fn() }, + }, + }); + + const workflows = await waitFor(() => getByTestId('agent-tools-available-workflows-list')); + const aiTools = getByTestId('agent-tools-available-ai-list'); + const n8nTools = getByTestId('agent-tools-available-n8n-list'); + const externalTools = getByTestId('agent-tools-available-external-list'); + const wrapper = getByTestId('agent-tools-list'); + + expect(wrapper.textContent).toContain('Workflows (1)'); + expect(wrapper.textContent).toContain('AI tools (1)'); + expect(wrapper.textContent).toContain('n8n tools (1)'); + expect(wrapper.textContent).toContain('External tools (2)'); + expect(workflows.textContent).toContain('Daily sales digest'); + expect(aiTools.textContent).toContain('OpenAI'); + expect(n8nTools.textContent).toContain('Code Tool'); + expect(externalTools.textContent).toContain('Slack'); + expect(externalTools.textContent).toContain('GitHub MCP'); + expect(queryByText('Subagent')).toBeNull(); + }); + + it('keeps utility AI tools visible and hides generated AI model tool counterparts', () => { + nodeTypesStore.visibleNodeTypesByOutputConnectionTypeNames = { + [NodeConnectionTypes.AiTool]: [ + CALCULATOR.name, + THINK_TOOL.name, + ANTHROPIC_TOOL.name, + GOOGLE_GEMINI_TOOL.name, + MINIMAX_TOOL.name, + MOONSHOT_TOOL.name, + OLLAMA_TOOL.name, + CHAT_TOOL.name, + SLACK.name, + ], + }; + + const { getByTestId, queryByText } = renderComponent(defaultProps()); + const aiTools = getByTestId('agent-tools-available-ai-list'); + const externalTools = getByTestId('agent-tools-available-external-list'); + + expect(aiTools.textContent).toContain('Calculator'); + expect(aiTools.textContent).toContain('Think Tool'); + expect(externalTools.textContent).toContain('Slack'); + expect(queryByText('Anthropic Tool')).toBeNull(); + expect(queryByText('Google Gemini Tool')).toBeNull(); + expect(queryByText('MiniMax Tool')).toBeNull(); + expect(queryByText('Moonshot Kimi Tool')).toBeNull(); + expect(queryByText('Ollama Tool')).toBeNull(); + expect(queryByText('Chat Tool')).toBeNull(); }); it('opens the config modal with the clicked tool ref when the gear is clicked', async () => { diff --git a/packages/frontend/editor-ui/src/features/agents/__tests__/InteractiveCard.test.ts b/packages/frontend/editor-ui/src/features/agents/__tests__/InteractiveCard.test.ts new file mode 100644 index 00000000000..352a968015f --- /dev/null +++ b/packages/frontend/editor-ui/src/features/agents/__tests__/InteractiveCard.test.ts @@ -0,0 +1,112 @@ +/* eslint-disable import-x/no-extraneous-dependencies -- test-only */ +import { mount } from '@vue/test-utils'; +import { APPROVAL_TOOL_NAME } from '@n8n/api-types'; +import { describe, expect, it, vi } from 'vitest'; + +import InteractiveCard from '../components/interactive/InteractiveCard.vue'; +import type { InteractivePayload } from '../composables/agentChatMessages'; + +vi.mock('@n8n/i18n', () => { + const i18n = { + baseText: (key: string, options?: { interpolate?: Record }) => { + if (key === 'agents.chat.approval.title') return 'Approval required'; + if (key === 'agents.chat.approval.description') { + return `The agent wants to run the ${options?.interpolate?.toolName ?? ''} tool.`; + } + if (key === 'agents.chat.approval.approve') return 'Approve'; + if (key === 'agents.chat.approval.reject') return 'Reject'; + if (key === 'agents.chat.approval.approved') return 'Approved'; + if (key === 'agents.chat.approval.rejected') return 'Rejected'; + return key; + }, + }; + return { useI18n: () => i18n, i18n, i18nInstance: { install: vi.fn() } }; +}); + +function mountCard(payload: InteractivePayload) { + return mount(InteractiveCard, { + props: { payload }, + global: { + stubs: { + N8nCard: { template: '
' }, + N8nText: { template: '', props: ['tag', 'bold', 'size', 'color'] }, + N8nIcon: { template: '', props: ['icon', 'size', 'color'] }, + N8nButton: { + template: + '', + props: ['disabled', 'type', 'variant', 'size'], + emits: ['click'], + }, + }, + }, + }); +} + +const approvalPayload: InteractivePayload = { + toolName: APPROVAL_TOOL_NAME, + toolCallId: 'tc-approval', + runId: 'run-approval', + input: { + type: 'approval', + toolName: 'calculator', + displayName: 'Calculator', + args: { input: '2 + 2' }, + }, +}; + +describe('InteractiveCard', () => { + it('renders approval details and emits approved resume data', async () => { + const wrapper = mountCard(approvalPayload); + + expect(wrapper.text()).toContain('Approval required'); + expect(wrapper.text()).toContain('The agent wants to run the Calculator tool.'); + expect(wrapper.text()).not.toContain('calculator.'); + expect(wrapper.text()).toContain('2 + 2'); + + await wrapper.find('[data-testid="agent-approval-approve"]').trigger('click'); + + expect(wrapper.emitted('submit')).toEqual([[{ approved: true }]]); + }); + + it('renders approval args as provided by the backend', () => { + const wrapper = mountCard({ + ...approvalPayload, + input: { + ...approvalPayload.input, + args: { + query: 'project status', + password: 'super-secret-password', + nested: { + apiKey: 'api-key-value', + authorization: 'Bearer token-value', + }, + }, + }, + }); + + expect(wrapper.text()).toContain('project status'); + expect(wrapper.text()).toContain('super-secret-password'); + expect(wrapper.text()).toContain('api-key-value'); + expect(wrapper.text()).toContain('token-value'); + }); + + it('emits rejected resume data from the reject action', async () => { + const wrapper = mountCard(approvalPayload); + + await wrapper.find('[data-testid="agent-approval-reject"]').trigger('click'); + + expect(wrapper.emitted('submit')).toEqual([[{ approved: false }]]); + }); + + it('renders resolved approval state without active actions', () => { + const wrapper = mountCard({ + ...approvalPayload, + resolvedAt: 1, + resolvedValue: { approved: false }, + }); + + expect(wrapper.text()).toContain('Rejected'); + expect(wrapper.find('[data-testid="agent-approval-approve"]').exists()).toBe(false); + expect(wrapper.find('[data-testid="agent-approval-reject"]').exists()).toBe(false); + }); +}); diff --git a/packages/frontend/editor-ui/src/features/agents/__tests__/agentChatMessages.test.ts b/packages/frontend/editor-ui/src/features/agents/__tests__/agentChatMessages.test.ts index 2548d86be74..22195316d1c 100644 --- a/packages/frontend/editor-ui/src/features/agents/__tests__/agentChatMessages.test.ts +++ b/packages/frontend/editor-ui/src/features/agents/__tests__/agentChatMessages.test.ts @@ -3,6 +3,7 @@ import { ASK_CREDENTIAL_TOOL_NAME, ASK_LLM_TOOL_NAME, ASK_QUESTION_TOOL_NAME, + APPROVAL_TOOL_NAME, type AgentPersistedMessageContentPart, type AgentPersistedMessageDto, } from '@n8n/api-types'; @@ -77,6 +78,49 @@ describe('rebuildInteractiveFromHistory', () => { }); expect(result).toBeUndefined(); }); + + it('rebuilds an OPEN approval card from an approval suspend payload', () => { + const result = rebuildInteractiveFromHistory({ + tool: 'calculator', + toolCallId: 'call-approval-1', + input: { + type: 'approval', + toolName: 'calculator', + displayName: 'Calculator', + args: { input: '2 + 2' }, + }, + state: 'suspended', + }); + + expect(result).toBeTruthy(); + expect(result?.toolName).toBe(APPROVAL_TOOL_NAME); + expect(result?.input).toEqual({ + type: 'approval', + toolName: 'calculator', + displayName: 'Calculator', + args: { input: '2 + 2' }, + }); + expect(result?.resolvedAt).toBeUndefined(); + expect(result?.resolvedValue).toBeUndefined(); + }); + + it('rebuilds a rejected approval card from a declined tool result', () => { + const result = rebuildInteractiveFromHistory({ + tool: 'calculator', + toolCallId: 'call-approval-2', + input: { + type: 'approval', + toolName: 'calculator', + args: { input: '2 + 2' }, + }, + output: { declined: true, message: 'Tool "calculator" was not approved' }, + state: 'done', + }); + + expect(result?.toolName).toBe(APPROVAL_TOOL_NAME); + expect(result?.resolvedAt).toBeDefined(); + expect(result?.resolvedValue).toEqual({ approved: false }); + }); }); describe('convertDbMessages — interactive turn synthesis', () => { diff --git a/packages/frontend/editor-ui/src/features/agents/__tests__/useAgentChatStream.test.ts b/packages/frontend/editor-ui/src/features/agents/__tests__/useAgentChatStream.test.ts index c6d87367907..f663aab7ded 100644 --- a/packages/frontend/editor-ui/src/features/agents/__tests__/useAgentChatStream.test.ts +++ b/packages/frontend/editor-ui/src/features/agents/__tests__/useAgentChatStream.test.ts @@ -1,7 +1,12 @@ /* eslint-disable import-x/no-extraneous-dependencies -- test-only */ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { ref, nextTick } from 'vue'; -import { ASK_CREDENTIAL_TOOL_NAME, ASK_LLM_TOOL_NAME, type AgentSseEvent } from '@n8n/api-types'; +import { + APPROVAL_TOOL_NAME, + ASK_CREDENTIAL_TOOL_NAME, + ASK_LLM_TOOL_NAME, + type AgentSseEvent, +} from '@n8n/api-types'; vi.mock('@n8n/stores/useRootStore', () => ({ useRootStore: () => ({ restApiContext: { baseUrl: 'http://localhost:5678' } }), @@ -34,11 +39,11 @@ function makeSseResponse(events: AgentSseEvent[]): Response { }); } -function buildHook() { +function buildHook(endpoint: 'build' | 'chat' = 'build') { return useAgentChatStream({ projectId: ref('p1'), agentId: ref('a1'), - endpoint: ref<'build' | 'chat'>('build'), + endpoint: ref<'build' | 'chat'>(endpoint), }); } @@ -160,6 +165,113 @@ describe('useAgentChatStream — SDK-aligned event handling', () => { expect(assistant.status).toBe('awaitingUser'); }); + it('renders an approval card when preview chat suspends for tool approval', async () => { + const events: AgentSseEvent[] = [ + { + type: 'tool-call', + toolCallId: 'tc-approval', + toolName: 'calculator', + input: { input: '2 + 2' }, + }, + { + type: 'tool-call-suspended', + payload: { + toolCallId: 'tc-approval', + runId: 'run-approval', + toolName: 'calculator', + input: { + type: 'approval', + toolName: 'calculator', + args: { input: '2 + 2' }, + }, + }, + }, + { type: 'done' }, + ]; + globalThis.fetch = vi.fn(async () => makeSseResponse(events)) as typeof fetch; + + const hook = buildHook('chat'); + await hook.sendMessage('calculate 2 + 2'); + await nextTick(); + + const assistant = hook.messages.value[1]; + expect(assistant.status).toBe('awaitingUser'); + expect(assistant.toolCalls?.[0].state).toBe('suspended'); + expect(assistant.interactive?.toolName).toBe(APPROVAL_TOOL_NAME); + expect(assistant.interactive?.runId).toBe('run-approval'); + expect(assistant.interactive?.input).toEqual({ + type: 'approval', + toolName: 'calculator', + args: { input: '2 + 2' }, + }); + }); + + it('posts approval resumes to the chat resume endpoint in preview chat mode', async () => { + const fetchMock = vi + .fn() + .mockResolvedValueOnce( + makeSseResponse([ + { + type: 'tool-call', + toolCallId: 'tc-approval', + toolName: 'calculator', + input: { input: '2 + 2' }, + }, + { + type: 'tool-call-suspended', + payload: { + toolCallId: 'tc-approval', + runId: 'run-approval', + toolName: 'calculator', + input: { + type: 'approval', + toolName: 'calculator', + args: { input: '2 + 2' }, + }, + }, + }, + { type: 'done' }, + ]), + ) + .mockResolvedValueOnce( + makeSseResponse([ + { + type: 'tool-result', + toolCallId: 'tc-approval', + toolName: 'calculator', + output: { result: 4 }, + }, + { type: 'done' }, + ]), + ); + globalThis.fetch = fetchMock as unknown as typeof fetch; + + const hook = buildHook('chat'); + await hook.sendMessage('calculate 2 + 2'); + await nextTick(); + + await hook.resume({ + runId: 'run-approval', + toolCallId: 'tc-approval', + resumeData: { approved: true }, + }); + + expect(fetchMock).toHaveBeenNthCalledWith( + 2, + 'http://localhost:5678/projects/p1/agents/v2/a1/chat/resume', + expect.objectContaining({ + body: JSON.stringify({ + runId: 'run-approval', + toolCallId: 'tc-approval', + resumeData: { approved: true }, + }), + }), + ); + const assistant = hook.messages.value[1]; + expect(assistant.interactive?.resolvedValue).toEqual({ approved: true }); + expect(assistant.status).toBe('success'); + }); + it('breaks out of the consume loop on `done` so isStreaming flips back to false', async () => { const events: AgentSseEvent[] = [ { type: 'text-delta', id: 't-1', delta: 'hello' }, diff --git a/packages/frontend/editor-ui/src/features/agents/components/AgentChatPanel.vue b/packages/frontend/editor-ui/src/features/agents/components/AgentChatPanel.vue index 91db43483cc..cdc8745d4b7 100644 --- a/packages/frontend/editor-ui/src/features/agents/components/AgentChatPanel.vue +++ b/packages/frontend/editor-ui/src/features/agents/components/AgentChatPanel.vue @@ -2,6 +2,7 @@ import { computed, ref, toRef, watch, onMounted, onBeforeUnmount } from 'vue'; import { N8nButton, N8nCallout, N8nIconButton } from '@n8n/design-system'; import { useI18n } from '@n8n/i18n'; +import { APPROVAL_TOOL_NAME } from '@n8n/api-types'; import ChatInputBase from '@/features/ai/shared/components/ChatInputBase.vue'; import { useAgentChatStream } from '../composables/useAgentChatStream'; import AgentChatEmptyState from './AgentChatEmptyState.vue'; @@ -110,8 +111,15 @@ const missingFields = computed(() => { return fatalError.value.missing.map(humaniseMissingField).join(', '); }); -const hasOpenInteractiveQuestion = computed(() => - messages.value.some((message) => message.interactive && !message.interactive.resolvedAt), +const openInteractive = computed( + () => + messages.value.find((message) => message.interactive && !message.interactive.resolvedAt) + ?.interactive, +); +const hasOpenInteraction = computed(() => openInteractive.value !== undefined); +const hasOpenApproval = computed(() => openInteractive.value?.toolName === APPROVAL_TOOL_NAME); +const hasOpenInteractiveQuestion = computed( + () => hasOpenInteraction.value && !hasOpenApproval.value, ); const isBuilderReadOnly = computed(() => props.endpoint === 'build' && !props.canEditAgent); @@ -119,9 +127,11 @@ const isBuilderReadOnly = computed(() => props.endpoint === 'build' && !props.ca const chatPlaceholder = computed(() => isBuilderReadOnly.value ? locale.baseText('agents.builder.readonly.placeholder') - : hasOpenInteractiveQuestion.value - ? locale.baseText('agents.chat.answerQuestionPlaceholder') - : locale.baseText('agents.chat.input.placeholder'), + : hasOpenApproval.value + ? locale.baseText('agents.chat.approval.inputPlaceholder') + : hasOpenInteractiveQuestion.value + ? locale.baseText('agents.chat.answerQuestionPlaceholder') + : locale.baseText('agents.chat.input.placeholder'), ); function onOpenBuild() { @@ -133,7 +143,14 @@ watch(isStreaming, (v) => emit('update:streaming', v)); async function onSubmit() { const text = inputText.value.trim(); - if (!text || isStreaming.value || isPreparingToSend.value || isBuilderReadOnly.value) return; + if ( + !text || + isStreaming.value || + isPreparingToSend.value || + isBuilderReadOnly.value || + hasOpenApproval.value + ) + return; // When there is an open interactive question, the user's message cancels // the suspended tool and steers the agent in a new direction. @@ -173,6 +190,7 @@ async function onSubmit() { } function sendMessageFromOutside(message: string) { + if (hasOpenApproval.value) return; inputText.value = message; void onSubmit(); } @@ -287,10 +305,17 @@ onBeforeUnmount(() => { :placeholder="chatPlaceholder" :is-streaming="messagingState === 'receiving'" :can-submit=" - !isStreaming && !isPreparingToSend && !isBuilderReadOnly && inputText.trim().length > 0 + !hasOpenApproval && + !isStreaming && + !isPreparingToSend && + !isBuilderReadOnly && + inputText.trim().length > 0 " :disabled=" - isBuilderReadOnly || isPreparingToSend || (isStreaming && messagingState !== 'receiving') + isBuilderReadOnly || + hasOpenApproval || + isPreparingToSend || + (isStreaming && messagingState !== 'receiving') " data-testid="chat-input" @submit="onSubmit" diff --git a/packages/frontend/editor-ui/src/features/agents/components/AgentToolConfigApprovalSetting.vue b/packages/frontend/editor-ui/src/features/agents/components/AgentToolConfigApprovalSetting.vue new file mode 100644 index 00000000000..d720c02a750 --- /dev/null +++ b/packages/frontend/editor-ui/src/features/agents/components/AgentToolConfigApprovalSetting.vue @@ -0,0 +1,50 @@ + + + + + diff --git a/packages/frontend/editor-ui/src/features/agents/components/AgentToolConfigModal.vue b/packages/frontend/editor-ui/src/features/agents/components/AgentToolConfigModal.vue index df4194ef6ff..d77412ae15c 100644 --- a/packages/frontend/editor-ui/src/features/agents/components/AgentToolConfigModal.vue +++ b/packages/frontend/editor-ui/src/features/agents/components/AgentToolConfigModal.vue @@ -2,7 +2,7 @@ /** * Configure one agent tool entry (node/workflow/custom) or one MCP server. */ -import { computed, ref } from 'vue'; +import { computed, ref, watch } from 'vue'; import Modal from '@/app/components/Modal.vue'; import { useUIStore } from '@/app/stores/ui.store'; import { N8nButton, N8nIcon, N8nRadioButtons } from '@n8n/design-system'; @@ -22,6 +22,7 @@ import { } from '../composables/useAgentToolRefAdapter'; import { nodeToMcpServer } from '../composables/useMcpServerAdapter'; import AgentJsonEditor from './AgentJsonEditor.vue'; +import AgentToolConfigApprovalSetting from './AgentToolConfigApprovalSetting.vue'; import AgentToolConfigCustomContent from './AgentToolConfigCustomContent.vue'; import AgentToolConfigModalHeader from './AgentToolConfigModalHeader.vue'; import AgentToolConfigNodeContent from './AgentToolConfigNodeContent.vue'; @@ -74,6 +75,7 @@ const mcpContentRef = ref | null const workflowContentRef = ref | null>(null); const isValid = ref(false); const activeView = ref<'config' | 'raw'>('config'); +const approvalRequired = ref(false); const initialNode = computed(() => isMcpTool.value @@ -124,6 +126,16 @@ const viewOptions = computed(() => [ const canRender = computed( () => isCustomTool.value || isWorkflowTool.value || initialNode.value !== null, ); +const canSave = computed(() => isCustomTool.value || isValid.value); +const showApprovalSetting = computed(() => !isMcpTool.value && toolModalData.value !== null); + +watch( + () => toolModalData.value?.toolRef, + (toolRef) => { + approvalRequired.value = Boolean(toolRef?.requireApproval); + }, + { immediate: true }, +); const headerKind = computed<'node' | 'workflow' | 'custom' | 'mcp'>(() => { if (isCustomTool.value) return 'custom'; @@ -152,8 +164,21 @@ function closeDialog() { uiStore.closeModal(props.modalName); } +function withApprovalRequirement(ref: AgentJsonToolRef): AgentJsonToolRef { + const updatedRef = { ...ref }; + if (approvalRequired.value) { + updatedRef.requireApproval = true; + } else { + delete updatedRef.requireApproval; + } + return updatedRef; +} + function handleConfirm() { if (isCustomTool.value) { + const toolData = toolModalData.value; + if (!toolData) return; + toolData.onConfirm(withApprovalRequirement(toolData.toolRef)); closeDialog(); return; } @@ -179,7 +204,7 @@ function handleConfirm() { description: wc.getDescription(), allOutputs: wc.getAllOutputs(), }); - toolData.onConfirm(updatedRef); + toolData.onConfirm(withApprovalRequirement(updatedRef)); closeDialog(); return; } @@ -189,7 +214,7 @@ function handleConfirm() { if (!currentNode) return; if (!toolData) return; const updatedRef = updateToolRefFromNode(toolData.toolRef, currentNode); - toolData.onConfirm(updatedRef); + toolData.onConfirm(withApprovalRequirement(updatedRef)); closeDialog(); } @@ -251,6 +276,10 @@ function handleNodeNameUpdate(name: string) { :code="customToolCode" :class="$style.customToolViewer" /> +
@@ -311,16 +347,11 @@ function handleNodeNameUpdate(name: string) {
- {{ - isCustomTool - ? i18n.baseText('generic.close') - : i18n.baseText('agents.toolConfig.cancel') - }} + {{ i18n.baseText('agents.toolConfig.cancel') }} @@ -355,7 +386,8 @@ function handleNodeNameUpdate(name: string) { flex-direction: column; gap: var(--spacing--sm); max-height: var(--agent-tool-config-content-max-height); - overflow: hidden; + overflow-x: hidden; + overflow-y: auto; margin-right: calc(-1 * var(--spacing--lg)); padding: var(--spacing--md) 0; @@ -368,13 +400,14 @@ function handleNodeNameUpdate(name: string) { height: var(--agent-tool-config-content-max-height); margin-right: 0; padding-bottom: 0; + overflow: hidden; } .configureTab { display: flex; - flex: 1; min-height: 0; flex-direction: column; + gap: var(--spacing--sm); } .viewToggle { diff --git a/packages/frontend/editor-ui/src/features/agents/components/AgentToolConfigWorkflowContent.vue b/packages/frontend/editor-ui/src/features/agents/components/AgentToolConfigWorkflowContent.vue index 4c77203d4e1..e7fcd2cae1e 100644 --- a/packages/frontend/editor-ui/src/features/agents/components/AgentToolConfigWorkflowContent.vue +++ b/packages/frontend/editor-ui/src/features/agents/components/AgentToolConfigWorkflowContent.vue @@ -1,16 +1,20 @@ + + + + diff --git a/packages/frontend/editor-ui/src/features/agents/components/WorkflowToolConfigContent.vue b/packages/frontend/editor-ui/src/features/agents/components/WorkflowToolConfigContent.vue index 083eae6ba09..f64b1c334f8 100644 --- a/packages/frontend/editor-ui/src/features/agents/components/WorkflowToolConfigContent.vue +++ b/packages/frontend/editor-ui/src/features/agents/components/WorkflowToolConfigContent.vue @@ -98,6 +98,8 @@ defineExpose({ {{ i18n.baseText('agents.toolConfig.workflow.allOutputs.hint') }}
+ +
diff --git a/packages/frontend/editor-ui/src/features/agents/components/WorkflowToolRow.vue b/packages/frontend/editor-ui/src/features/agents/components/WorkflowToolRow.vue index 40997916e1f..8ef67fec94b 100644 --- a/packages/frontend/editor-ui/src/features/agents/components/WorkflowToolRow.vue +++ b/packages/frontend/editor-ui/src/features/agents/components/WorkflowToolRow.vue @@ -11,6 +11,7 @@ import { N8nButton, N8nIcon, N8nIconButton, N8nText, N8nTooltip } from '@n8n/design-system'; import { useI18n } from '@n8n/i18n'; +import ToolApprovalBadge from './ToolApprovalBadge.vue'; import ToolConnectedBadge from './ToolConnectedBadge.vue'; withDefaults( @@ -18,10 +19,16 @@ withDefaults( mode: 'configured' | 'available'; name: string; description?: string | null; + requireApproval?: boolean; rowTestId?: string; configureTestId?: string; }>(), - { description: undefined, rowTestId: undefined, configureTestId: undefined }, + { + description: undefined, + requireApproval: false, + rowTestId: undefined, + configureTestId: undefined, + }, ); defineEmits<{ @@ -50,6 +57,7 @@ const i18n = useI18n();