From 1da2c1b5eb98f9bf5e58606c88d20ea3e0847f35 Mon Sep 17 00:00:00 2001 From: yehorkardash Date: Thu, 28 May 2026 10:54:52 +0300 Subject: [PATCH] feat: Support agent mcp servers (no-changelog) (#31070) --- .../runtime/__tests__/mcp-connection.test.ts | 118 ++++ .../@n8n/agents/src/runtime/mcp-connection.ts | 11 +- packages/@n8n/agents/src/sdk/agent.ts | 17 +- packages/@n8n/agents/src/types/sdk/agent.ts | 12 + packages/@n8n/agents/src/types/sdk/mcp.ts | 25 + .../src/agents/agent-json-config.schema.ts | 103 ++++ .../@n8n/config/src/configs/agents.config.ts | 10 +- .../nodes-langchain/nodes/mcp/shared/types.ts | 9 +- .../agents-builder-tools.service.test.ts | 37 ++ .../agents-service-reconstruct-gating.test.ts | 102 +++- .../__tests__/agents-service-sync.test.ts | 1 + .../__tests__/agents-tools.service.test.ts | 5 + .../agents/__tests__/agents.service.test.ts | 3 +- .../agents/__tests__/from-json-config.test.ts | 192 +++++++ .../modules/agents/agents-tools.service.ts | 19 +- .../cli/src/modules/agents/agents.service.ts | 68 ++- ...ents-builder-model-recommendations.test.ts | 5 +- .../__tests__/agents-builder-prompts.test.ts | 52 ++ .../__tests__/verify-mcp-server.tool.test.ts | 196 +++++++ .../agents/builder/agents-builder-prompts.ts | 32 +- .../builder/agents-builder-tools.service.ts | 21 +- .../agents/builder/agents-builder.service.ts | 9 +- .../agents/builder/builder-tool-names.ts | 1 + .../builder/prompts/config-mutation.prompt.ts | 4 +- .../builder/prompts/config-rules.prompt.ts | 14 +- .../modules/agents/builder/skills/index.ts | 13 +- .../agents/builder/skills/mcp.skill.ts | 92 ++++ .../agents/builder/verify-mcp-server.tool.ts | 102 ++++ .../__tests__/mcp-client-factory.test.ts | 505 ++++++++++++++++++ .../agents/json-config/from-json-config.ts | 26 + .../agents/json-config/mcp-client-factory.ts | 201 +++++++ .../src/oauth/__tests__/oauth.service.test.ts | 236 ++++++++ packages/cli/src/oauth/oauth.service.ts | 72 +++ packages/workflow/src/index.ts | 1 + packages/workflow/src/mcp-helpers.ts | 12 + packages/workflow/test/mcp-helpers.test.ts | 24 + 36 files changed, 2296 insertions(+), 54 deletions(-) create mode 100644 packages/@n8n/agents/src/runtime/__tests__/mcp-connection.test.ts create mode 100644 packages/cli/src/modules/agents/builder/__tests__/agents-builder-prompts.test.ts create mode 100644 packages/cli/src/modules/agents/builder/__tests__/verify-mcp-server.tool.test.ts create mode 100644 packages/cli/src/modules/agents/builder/skills/mcp.skill.ts create mode 100644 packages/cli/src/modules/agents/builder/verify-mcp-server.tool.ts create mode 100644 packages/cli/src/modules/agents/json-config/__tests__/mcp-client-factory.test.ts create mode 100644 packages/cli/src/modules/agents/json-config/mcp-client-factory.ts create mode 100644 packages/workflow/src/mcp-helpers.ts create mode 100644 packages/workflow/test/mcp-helpers.test.ts diff --git a/packages/@n8n/agents/src/runtime/__tests__/mcp-connection.test.ts b/packages/@n8n/agents/src/runtime/__tests__/mcp-connection.test.ts new file mode 100644 index 00000000000..7a0be6f251c --- /dev/null +++ b/packages/@n8n/agents/src/runtime/__tests__/mcp-connection.test.ts @@ -0,0 +1,118 @@ +import { McpConnection } from '../mcp-connection'; + +const sseCtor = jest.fn(); +const streamableHttpCtor = jest.fn(); +const stdioCtor = jest.fn(); + +const clientConnect = jest.fn().mockResolvedValue(undefined); +const clientListTools = jest.fn().mockResolvedValue({ + tools: [ + { name: 'echo', description: '', inputSchema: { type: 'object' } }, + { name: 'add', description: '', inputSchema: { type: 'object' } }, + { name: 'subtract', description: '', inputSchema: { type: 'object' } }, + ], +}); +const clientClose = jest.fn().mockResolvedValue(undefined); + +class FakeClient { + connect = clientConnect; + listTools = clientListTools; + close = clientClose; +} + +jest.mock('@modelcontextprotocol/sdk/client/index.js', () => ({ + Client: jest.fn(() => new FakeClient()), +})); + +jest.mock('@modelcontextprotocol/sdk/client/sse.js', () => ({ + SSEClientTransport: jest.fn((url: URL, options: unknown) => { + sseCtor(url, options); + return { type: 'sse', url, options }; + }), +})); + +jest.mock('@modelcontextprotocol/sdk/client/stdio.js', () => ({ + StdioClientTransport: jest.fn((options: unknown) => { + stdioCtor(options); + return { type: 'stdio', options }; + }), +})); + +jest.mock('@modelcontextprotocol/sdk/client/streamableHttp.js', () => ({ + StreamableHTTPClientTransport: jest.fn((url: URL, options: unknown) => { + streamableHttpCtor(url, options); + return { type: 'streamableHttp', url, options }; + }), +})); + +jest.mock('@modelcontextprotocol/sdk/types.js', () => ({ + CallToolResultSchema: {}, +})); + +// --------------------------------------------------------------------------- +// Tests +// --------------------------------------------------------------------------- + +describe('McpConnection — custom fetch forwarding', () => { + beforeEach(() => { + sseCtor.mockClear(); + streamableHttpCtor.mockClear(); + stdioCtor.mockClear(); + clientConnect.mockClear(); + clientListTools.mockClear(); + }); + + it('forwards `fetch` to StreamableHTTPClientTransport when provided', async () => { + const customFetch = jest.fn(); + const conn = new McpConnection({ + name: 's1', + url: 'https://example.test/mcp', + transport: 'streamableHttp', + fetch: customFetch as unknown as typeof fetch, + }); + + await conn.connect(); + + expect(streamableHttpCtor).toHaveBeenCalledTimes(1); + const [, options] = streamableHttpCtor.mock.calls[0] as [URL, { fetch?: typeof fetch }]; + expect(options.fetch).toBe(customFetch); + }); + + it('forwards `fetch` to SSEClientTransport and to its eventSourceInit when provided', async () => { + const customFetch = jest.fn(); + const conn = new McpConnection({ + name: 's2', + url: 'https://example.test/mcp', + transport: 'sse', + fetch: customFetch as unknown as typeof fetch, + }); + + await conn.connect(); + + expect(sseCtor).toHaveBeenCalledTimes(1); + const [, options] = sseCtor.mock.calls[0] as [ + URL, + { fetch?: typeof fetch; eventSourceInit?: { fetch?: typeof fetch } }, + ]; + expect(options.fetch).toBe(customFetch); + expect(options.eventSourceInit?.fetch).toBe(customFetch); + }); + + it('omits `fetch` and `eventSourceInit` when no custom fetch is provided', async () => { + const conn = new McpConnection({ + name: 's3', + url: 'https://example.test/mcp', + transport: 'sse', + }); + + await conn.connect(); + + expect(sseCtor).toHaveBeenCalledTimes(1); + const [, options] = sseCtor.mock.calls[0] as [ + URL, + { fetch?: typeof fetch; eventSourceInit?: { fetch?: typeof fetch } }, + ]; + expect(options.fetch).toBeUndefined(); + expect(options.eventSourceInit).toBeUndefined(); + }); +}); diff --git a/packages/@n8n/agents/src/runtime/mcp-connection.ts b/packages/@n8n/agents/src/runtime/mcp-connection.ts index 02a31a5378b..341ef85314d 100644 --- a/packages/@n8n/agents/src/runtime/mcp-connection.ts +++ b/packages/@n8n/agents/src/runtime/mcp-connection.ts @@ -216,10 +216,17 @@ export class McpConnection { : undefined; if (config.transport === 'streamableHttp') { - return new sdk.StreamableHTTPClientTransport(url, { requestInit }); + return new sdk.StreamableHTTPClientTransport(url, { + requestInit, + fetch: config.fetch, + }); } - return new sdk.SSEClientTransport(url, { requestInit }); + return new sdk.SSEClientTransport(url, { + requestInit, + fetch: config.fetch, + eventSourceInit: config.fetch ? { fetch: config.fetch } : undefined, + }); } throw new Error(`MCP server "${config.name}": provide either "url" or "command"`); } diff --git a/packages/@n8n/agents/src/sdk/agent.ts b/packages/@n8n/agents/src/sdk/agent.ts index 1c89fde0a90..0ee9c8fc8f7 100644 --- a/packages/@n8n/agents/src/sdk/agent.ts +++ b/packages/@n8n/agents/src/sdk/agent.ts @@ -617,12 +617,21 @@ export class Agent implements BuiltAgent, AgentBuilder { } /** - * Wait for any in-flight background tasks (title generation, future - * observer cycles) to settle. Call before letting the agent go out of - * scope to ensure deferred writes land. Safe to call multiple times. + * Close the agent and release all held resources. + * + * - Waits for any in-flight background tasks (title generation, observer + * cycles) to settle via the runtime's `dispose()`. + * - Disconnects every MCP client attached via `.mcp()`. Errors from + * individual client disconnects are swallowed so a single misbehaving + * server does not prevent the others from closing. + * + * Safe to call multiple times. */ async close(): Promise { - if (this.runtime) await this.runtime.dispose(); + const tasks: Array> = []; + if (this.runtime) tasks.push(this.runtime.dispose()); + tasks.push(...this.mcpClients.map(async (c) => await c.close())); + await Promise.allSettled(tasks); } /** Generate a response (non-streaming). Lazy-builds on first call. */ diff --git a/packages/@n8n/agents/src/types/sdk/agent.ts b/packages/@n8n/agents/src/types/sdk/agent.ts index 765defaa8ca..5d977baa39c 100644 --- a/packages/@n8n/agents/src/types/sdk/agent.ts +++ b/packages/@n8n/agents/src/types/sdk/agent.ts @@ -233,6 +233,18 @@ export interface BuiltAgent { /** Cancel the currently running agent. Synchronous — sets an abort flag that the agentic loop checks asynchronously. */ abort(): void; + /** + * Close the agent and release all held resources. + * + * - Waits for any in-flight background tasks (title generation, observer + * cycles) to settle via the runtime's `dispose()`. + * - Disconnects every MCP client that was attached via `.mcp()`. + * + * Safe to call multiple times. Should be called when the agent is no + * longer needed so MCP transports are not left open indefinitely. + */ + close(): Promise; + /** Resume a tool with custom resume data */ resume( method: 'generate', diff --git a/packages/@n8n/agents/src/types/sdk/mcp.ts b/packages/@n8n/agents/src/types/sdk/mcp.ts index c0ce23ff319..278513663e6 100644 --- a/packages/@n8n/agents/src/types/sdk/mcp.ts +++ b/packages/@n8n/agents/src/types/sdk/mcp.ts @@ -38,4 +38,29 @@ export interface McpServerConfig { * `.requireToolApproval()` flag on the Agent still applies). */ requireApproval?: string[] | boolean; + + /** + * Custom fetch implementation used by URL-based transports (SSE, + * StreamableHTTP). + * Ignored for stdio transport. When omitted, the SDK transports fall back + * to the global `fetch`. + */ + fetch?: typeof fetch; + + /** + * Restrict which tools from this server are surfaced to the agent. + * + * Tools are matched by their original (un-prefixed) name. + * + * - `{ mode: 'allow', tools: [...] }` — only the listed tools are surfaced. + * - `{ mode: 'exclude', tools: [...] }` — every tool except the listed ones + * is surfaced. + * - omitted — every tool the server advertises is surfaced. + * + * An empty `tools` array is a no-op for both modes — i.e. an empty allow + * list does not hide everything, and an empty exclude list does not hide + * anything. This matches the JSON-config semantics ("no filter applied" + * is expressed by omitting the field). + */ + toolFilter?: { mode: 'allow' | 'exclude'; tools: string[] }; } diff --git a/packages/@n8n/api-types/src/agents/agent-json-config.schema.ts b/packages/@n8n/api-types/src/agents/agent-json-config.schema.ts index e70948f78c4..70a00a665ac 100644 --- a/packages/@n8n/api-types/src/agents/agent-json-config.schema.ts +++ b/packages/@n8n/api-types/src/agents/agent-json-config.schema.ts @@ -100,6 +100,101 @@ const AgentJsonSkillConfigSchema = z.object({ .regex(/^[A-Za-z0-9_-]+$/), }); +/** + * Configuration for a single MCP (Model Context Protocol) server attached to + * an agent. Tool entries from MCP servers are sourced separately from the + * `tools[]` array — the SDK's `McpClient` prefixes each tool name with the + * server name to avoid collisions. + */ +const McpServerConfigSchema = z + .object({ + /** + * Unique-per-agent server name. Also used as the SDK tool-name prefix + * (e.g. a server named `github` exposes its `create_issue` tool as + * `github_create_issue` to the LLM). + */ + name: z + .string() + .min(1) + .max(64) + .regex(/^[a-zA-Z0-9_-]+$/), + description: z.string().max(512).optional(), + url: z.string().min(1), + transport: z.enum(['sse', 'streamableHttp']).default('streamableHttp'), + /** + * Authentication method. In addition to the five named variants, any string + * ending in `McpOAuth2Api` is accepted to accommodate registry-specific + * credential types (e.g. `notionMcpOAuth2Api`, `githubMcpOAuth2Api`). + */ + authentication: z + .union([ + z.enum(['none', 'bearerAuth', 'headerAuth', 'multipleHeadersAuth', 'mcpOAuth2Api']), + z.string().endsWith('McpOAuth2Api'), + ]) + .default('none'), + /** Credential id required when `authentication` is anything other than `none`. */ + credential: z.string().optional(), + metadata: z + .object({ + /** + * The node-type name this server was created from (e.g. `@n8n/mcp-registry.github`). + * When present the config modal renders with the registry node type's form + * (correct credential selector, preset field visibility) instead of the generic + * MCP Client Tool form. + */ + nodeTypeName: z.string().optional(), + }) + .optional(), + /** + * Restricts which tools from this server are surfaced to the agent. + * Tools are matched by their original (un-prefixed) name. + * + * - `{ mode: 'allow', tools: [...] }` — only the listed tools are surfaced. + * - `{ mode: 'exclude', tools: [...] }` — every tool except the listed ones is surfaced. + * - absent — every tool the server advertises is surfaced. + */ + toolFilter: z + .discriminatedUnion('mode', [ + z + .object({ + mode: z.literal('allow'), + tools: z.array(z.string().min(1)).default([]), + }) + .strict(), + z + .object({ + mode: z.literal('exclude'), + tools: z.array(z.string().min(1)).default([]), + }) + .strict(), + ]) + .optional(), + /** + * Human-in-the-loop approval requirements. + * + * - absent — no approval required (default). + * - `{ mode: 'global' }` — every tool from this server requires approval. + * - `{ mode: 'selected', tools: [...] }` — only listed tool names + * (un-prefixed) require approval; non-empty. + * + * Maps onto the SDK's `McpServerConfig.requireApproval` + * (`true` / `string[]`) at reconstruction time. + */ + approval: z + .discriminatedUnion('mode', [ + z.object({ mode: z.literal('global') }).strict(), + z + .object({ + mode: z.literal('selected'), + tools: z.array(z.string().min(1)).min(1), + }) + .strict(), + ]) + .optional(), + connectionTimeoutMs: z.number().int().min(1).max(120_000).optional(), + }) + .strict(); + const AgentJsonToolConfigSchema = z.discriminatedUnion('type', [ z.object({ type: z.literal('custom'), @@ -144,6 +239,13 @@ export const AgentJsonConfigSchema = z.object({ skills: z.array(AgentJsonSkillConfigSchema).optional(), providerTools: z.record(z.record(z.unknown())).optional(), integrations: z.array(AgentIntegrationSchema).optional(), + mcpServers: z + .array(McpServerConfigSchema) + .max(20) + .refine((servers) => new Set(servers.map((s) => s.name)).size === servers.length, { + message: 'MCP server names must be unique within an agent', + }) + .optional(), config: z .object({ thinking: ThinkingConfigSchema.optional(), @@ -184,6 +286,7 @@ export type AgentJsonCustomToolConfig = Extract; export type AgentJsonMemoryConfig = z.infer; export type NodeToolConfig = z.infer; +export type AgentJsonMcpServerConfig = z.infer; export interface ConfigValidationError { path: string; diff --git a/packages/@n8n/config/src/configs/agents.config.ts b/packages/@n8n/config/src/configs/agents.config.ts index 28ff58fe7ca..813ccd7da85 100644 --- a/packages/@n8n/config/src/configs/agents.config.ts +++ b/packages/@n8n/config/src/configs/agents.config.ts @@ -6,7 +6,7 @@ import { Config, Env } from '../decorators'; * `N8N_AGENTS_MODULES`. The backend fails fast on unknown tokens so typos * surface at startup instead of silently disabling a feature. */ -export const AGENTS_MODULE_NAMES = ['node-tools-searcher'] as const; +export const AGENTS_MODULE_NAMES = ['node-tools-searcher', 'mcp'] as const; export type AgentsModuleName = (typeof AGENTS_MODULE_NAMES)[number]; @@ -33,8 +33,12 @@ export class AgentsConfig { /** * Comma-separated list of agent sub-feature modules to enable. Each entry * gates a specific frontend/runtime capability inside the agents module. - * Currently known: `node-tools-searcher` (surfaces the "Built-in node tools" - * toggle in the agent editor). + * Currently known: + * - `node-tools-searcher` — surfaces the "Built-in node tools" toggle in + * the agent editor. + * - `mcp` — enables the "MCP servers" section in the agent editor and + * the runtime wiring that builds `McpClient` instances from + * `config.mcpServers[]`. * * Gates the UI surface only — existing agents persisted with a given * capability turned on continue to run even if its token is removed here. diff --git a/packages/@n8n/nodes-langchain/nodes/mcp/shared/types.ts b/packages/@n8n/nodes-langchain/nodes/mcp/shared/types.ts index 4d13a54e44f..8c415ec4c2c 100644 --- a/packages/@n8n/nodes-langchain/nodes/mcp/shared/types.ts +++ b/packages/@n8n/nodes-langchain/nodes/mcp/shared/types.ts @@ -1,11 +1,10 @@ import type { JSONSchema7 } from 'json-schema'; +import { isMcpOAuth2Authentication, type McpOAuth2CredentialType } from 'n8n-workflow'; export type McpTool = { name: string; description?: string; inputSchema: JSONSchema7 }; export type McpServerTransport = 'sse' | 'httpStreamable'; -export type McpOAuth2CredentialType = 'mcpOAuth2Api' | `${string}McpOAuth2Api`; - export type McpAuthenticationOption = | 'none' | 'headerAuth' @@ -13,8 +12,4 @@ export type McpAuthenticationOption = | 'multipleHeadersAuth' | McpOAuth2CredentialType; -export function isMcpOAuth2Authentication( - authentication: string, -): authentication is McpOAuth2CredentialType { - return authentication === 'mcpOAuth2Api' || authentication.endsWith('McpOAuth2Api'); -} +export { isMcpOAuth2Authentication, type McpOAuth2CredentialType }; diff --git a/packages/cli/src/modules/agents/__tests__/agents-builder-tools.service.test.ts b/packages/cli/src/modules/agents/__tests__/agents-builder-tools.service.test.ts index ec2ebc781d3..0e11c9f6c66 100644 --- a/packages/cli/src/modules/agents/__tests__/agents-builder-tools.service.test.ts +++ b/packages/cli/src/modules/agents/__tests__/agents-builder-tools.service.test.ts @@ -37,6 +37,7 @@ function makeService() { workflowRepository, agentsToolsService, builderModelLookupService, + mock(), credentialTypes, ); @@ -601,4 +602,40 @@ describe('AgentsBuilderToolsService', () => { expect(agentsService.createSkill).not.toHaveBeenCalled(); }); }); + + describe('MCP module gating', () => { + it('does not include verify_mcp_server when the "mcp" module is not enabled', () => { + const { service } = makeService(); + + const tools = service.getTools(agentId, projectId, credentialProvider, user).json; + + expect(tools.find((t) => t.name === BUILDER_TOOLS.VERIFY_MCP_SERVER)).toBeUndefined(); + }); + + it('does not include verify_mcp_server when enabledModules is an empty list', () => { + const { service } = makeService(); + + const tools = service.getTools(agentId, projectId, credentialProvider, user, []).json; + + expect(tools.find((t) => t.name === BUILDER_TOOLS.VERIFY_MCP_SERVER)).toBeUndefined(); + }); + + it('includes verify_mcp_server when the "mcp" module is enabled', () => { + const { service } = makeService(); + + const tools = service.getTools(agentId, projectId, credentialProvider, user, ['mcp']).json; + + expect(tools.find((t) => t.name === BUILDER_TOOLS.VERIFY_MCP_SERVER)).toBeDefined(); + }); + + it('does not include verify_mcp_server when other modules are enabled but not "mcp"', () => { + const { service } = makeService(); + + const tools = service.getTools(agentId, projectId, credentialProvider, user, [ + 'someOtherModule', + ]).json; + + expect(tools.find((t) => t.name === BUILDER_TOOLS.VERIFY_MCP_SERVER)).toBeUndefined(); + }); + }); }); diff --git a/packages/cli/src/modules/agents/__tests__/agents-service-reconstruct-gating.test.ts b/packages/cli/src/modules/agents/__tests__/agents-service-reconstruct-gating.test.ts index 550d7ca6311..0479c2601f6 100644 --- a/packages/cli/src/modules/agents/__tests__/agents-service-reconstruct-gating.test.ts +++ b/packages/cli/src/modules/agents/__tests__/agents-service-reconstruct-gating.test.ts @@ -35,8 +35,16 @@ import type { AgentSecureRuntime } from '../runtime/agent-secure-runtime'; const builtAgent = mock(); builtAgent.hasCheckpointStorage.mockReturnValue(true); // skip checkpoint injection branch +const buildFromJsonMock = jest.fn().mockImplementation(async () => builtAgent); jest.mock('../json-config/from-json-config', () => ({ - buildFromJson: jest.fn().mockImplementation(async () => builtAgent), + buildFromJson: (...args: unknown[]) => buildFromJsonMock(...args), +})); + +const buildMcpClientForServerMock = jest + .fn() + .mockImplementation(async () => mock()); +jest.mock('../json-config/mcp-client-factory', () => ({ + buildMcpClientForServer: (...args: unknown[]) => buildMcpClientForServerMock(...args), })); // Avoid loading the rich-interaction tool (its import path resolves to runtime code). @@ -72,15 +80,20 @@ function makeService( mock(), mock(), mock(), + mock(), ); } -function makeAgentEntity(schemaConfig?: AgentJsonConfig['config']): Agent { +function makeAgentEntity( + schemaConfig?: AgentJsonConfig['config'], + overrides?: Partial, +): Agent { const schema: AgentJsonConfig = { name: 'Test', model: 'anthropic/claude-sonnet-4-5', instructions: 'Be helpful', ...(schemaConfig !== undefined ? { config: schemaConfig } : {}), + ...(overrides ?? {}), }; return { id: 'agent-1', @@ -172,3 +185,88 @@ describe('AgentsService.reconstructFromConfig — node tools gating', () => { } }); }); + +describe('AgentsService.reconstructFromConfig — MCP gating', () => { + beforeEach(() => { + jest.clearAllMocks(); + builtAgent.hasCheckpointStorage.mockReturnValue(true); + buildFromJsonMock.mockImplementation(async (_config, _descriptors, options) => { + // Drive the buildMcpClient callback exactly once per configured server, + // matching what the real buildFromJson does — this is what lets the + // gating test assert how many MCP clients were created. + const cfg = _config as AgentJsonConfig; + if (options?.buildMcpClient && cfg.mcpServers) { + for (const server of cfg.mcpServers) { + await options.buildMcpClient(server); + } + } + return builtAgent; + }); + }); + + function setup(options: { mcpModuleEnabled?: boolean } = {}) { + const agentsToolsService = mock(); + agentsToolsService.getRuntimeTools.mockReturnValue([] as BuiltTool[]); + const credentialProvider = mock(); + const service = makeService(agentsToolsService, options.mcpModuleEnabled ? ['mcp'] : []); + return { service, credentialProvider }; + } + + it('does not call the MCP factory when no mcpServers are configured', async () => { + const { service, credentialProvider } = setup({ mcpModuleEnabled: true }); + const entity = makeAgentEntity(); + + await (service as unknown as Reconstructable).reconstructFromConfig(entity, credentialProvider); + + expect(buildMcpClientForServerMock).not.toHaveBeenCalled(); + }); + + it('skips MCP wiring when the module is disabled, even if mcpServers are present', async () => { + const { service, credentialProvider } = setup({ mcpModuleEnabled: false }); + const entity = makeAgentEntity(undefined, { + mcpServers: [ + { + name: 'github', + url: 'https://api.example.test/mcp', + transport: 'streamableHttp', + authentication: 'none', + }, + ], + }); + + // We pass an `options` to buildFromJson with `buildMcpClient: undefined`, + // so the mock's loop is a no-op and the factory is never called. + await (service as unknown as Reconstructable).reconstructFromConfig(entity, credentialProvider); + + expect(buildMcpClientForServerMock).not.toHaveBeenCalled(); + const lastCall = buildFromJsonMock.mock.calls.at(-1) as unknown[]; + const opts = lastCall[2] as { buildMcpClient: unknown }; + expect(opts.buildMcpClient).toBeUndefined(); + }); + + it('builds one MCP client per configured server when the module is enabled', async () => { + const { service, credentialProvider } = setup({ mcpModuleEnabled: true }); + const entity = makeAgentEntity(undefined, { + mcpServers: [ + { + name: 'github', + url: 'https://api.example.test/mcp', + transport: 'streamableHttp', + authentication: 'none', + }, + { + name: 'fs', + url: 'https://fs.example.test/mcp', + transport: 'sse', + authentication: 'none', + }, + ], + }); + + await (service as unknown as Reconstructable).reconstructFromConfig(entity, credentialProvider); + + expect(buildMcpClientForServerMock).toHaveBeenCalledTimes(2); + expect(buildMcpClientForServerMock.mock.calls[0][0]).toMatchObject({ name: 'github' }); + expect(buildMcpClientForServerMock.mock.calls[1][0]).toMatchObject({ name: 'fs' }); + }); +}); diff --git a/packages/cli/src/modules/agents/__tests__/agents-service-sync.test.ts b/packages/cli/src/modules/agents/__tests__/agents-service-sync.test.ts index 885be114459..46d369441a0 100644 --- a/packages/cli/src/modules/agents/__tests__/agents-service-sync.test.ts +++ b/packages/cli/src/modules/agents/__tests__/agents-service-sync.test.ts @@ -82,6 +82,7 @@ describe('AgentsService — updateName / updateDescription schema sync', () => { mock(), mock(), mock(), + mock(), ); }); 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 bfe6ec867de..4e950b33011 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 @@ -172,6 +172,11 @@ describe('AgentsToolsService', () => { expect(isAgentToolNodeType('@n8n/n8n-nodes-langchain.lmChatOpenAi')).toBe(false); expect(isAgentToolNodeType('@n8n/n8n-nodes-langchain.agent')).toBe(false); }); + + it('does not allow MCP tool nodes', () => { + expect(isAgentToolNodeType('@n8n/n8n-nodes-langchain.mcpClientTool')).toBe(false); + expect(isAgentToolNodeType('@n8n/mcp-registry.notion')).toBe(false); + }); }); describe('get_node_types handler', () => { diff --git a/packages/cli/src/modules/agents/__tests__/agents.service.test.ts b/packages/cli/src/modules/agents/__tests__/agents.service.test.ts index 15904ac987f..05909fa331f 100644 --- a/packages/cli/src/modules/agents/__tests__/agents.service.test.ts +++ b/packages/cli/src/modules/agents/__tests__/agents.service.test.ts @@ -131,6 +131,7 @@ describe('AgentsService', () => { globalConfig, telemetry, chatIntegrationService, + mock(), ); }); @@ -1082,7 +1083,7 @@ describe('AgentsService', () => { }); jest.spyOn(service as never, 'compileIsolated').mockResolvedValue({ ok: true, - agent: { name: 'Test Agent', stream }, + agent: { name: 'Test Agent', stream, close: jest.fn().mockResolvedValue(undefined) }, } as never); await service.executeForWorkflow( 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 37faf36f3f1..e2d218f1e82 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 @@ -995,6 +995,101 @@ describe('buildFromJson()', () => { expect(agent.snapshot.hasMemory).toBe(false); expect(getMemoryConfig(agent)).toBeUndefined(); }); + + // ------------------------------------------------------------------------- + // MCP servers + // ------------------------------------------------------------------------- + + describe('mcpServers', () => { + it('does not invoke buildMcpClient when mcpServers is absent', async () => { + const buildMcpClient = jest.fn(); + await buildFromJson( + makeConfig(), + {}, + { + toolExecutor: makeMockToolExecutor(), + credentialProvider: makeMockCredentialProvider(), + memoryFactory: makeMockMemoryFactory(), + buildMcpClient, + }, + ); + expect(buildMcpClient).not.toHaveBeenCalled(); + }); + + it('does not invoke buildMcpClient when mcpServers is an empty array', async () => { + const buildMcpClient = jest.fn(); + await buildFromJson( + makeConfig({ mcpServers: [] }), + {}, + { + toolExecutor: makeMockToolExecutor(), + credentialProvider: makeMockCredentialProvider(), + memoryFactory: makeMockMemoryFactory(), + buildMcpClient, + }, + ); + expect(buildMcpClient).not.toHaveBeenCalled(); + }); + + it('silently skips MCP wiring when no buildMcpClient is provided', async () => { + // No buildMcpClient -> the loop is a no-op; build still succeeds. + await expect( + buildFromJson( + makeConfig({ + mcpServers: [ + { + name: 'github', + url: 'https://api.example.test/mcp', + transport: 'streamableHttp', + authentication: 'none', + }, + ], + }), + {}, + { + toolExecutor: makeMockToolExecutor(), + credentialProvider: makeMockCredentialProvider(), + memoryFactory: makeMockMemoryFactory(), + }, + ), + ).resolves.toBeDefined(); + }); + + it('calls buildMcpClient once per configured server and passes each entry through', async () => { + const buildMcpClient = jest + .fn() + .mockImplementation(async () => ({ close: jest.fn() }) as never); + await buildFromJson( + makeConfig({ + mcpServers: [ + { + name: 'github', + url: 'https://api.example.test/mcp', + transport: 'streamableHttp', + authentication: 'none', + }, + { + name: 'fs', + url: 'https://fs.example.test/mcp', + transport: 'sse', + authentication: 'none', + }, + ], + }), + {}, + { + toolExecutor: makeMockToolExecutor(), + credentialProvider: makeMockCredentialProvider(), + memoryFactory: makeMockMemoryFactory(), + buildMcpClient, + }, + ); + + expect(buildMcpClient).toHaveBeenCalledTimes(2); + expect(buildMcpClient.mock.calls[0][0]).toMatchObject({ name: 'github' }); + expect(buildMcpClient.mock.calls[1][0]).toMatchObject({ name: 'fs' }); + }); + }); }); // --------------------------------------------------------------------------- @@ -1315,4 +1410,101 @@ describe('AgentJsonConfigSchema', () => { credentialId: 'cred-1', }); }); + + describe('mcpServers', () => { + const base = { + name: 'test', + model: 'anthropic/claude-sonnet-4-5', + credential: 'my-key', + instructions: 'Be helpful.', + }; + + it('parses a minimal MCP server entry with defaults', () => { + const parsed = AgentJsonConfigSchema.parse({ + ...base, + mcpServers: [{ name: 'github', url: 'https://api.example.test/mcp' }], + }); + expect(parsed.mcpServers?.[0]).toMatchObject({ + name: 'github', + url: 'https://api.example.test/mcp', + transport: 'streamableHttp', + authentication: 'none', + }); + }); + + it('rejects duplicate MCP server names', () => { + expect(() => + AgentJsonConfigSchema.parse({ + ...base, + mcpServers: [ + { name: 'dup', url: 'https://a.example.test/mcp' }, + { name: 'dup', url: 'https://b.example.test/mcp' }, + ], + }), + ).toThrow(); + }); + + it('rejects names with invalid characters', () => { + expect(() => + AgentJsonConfigSchema.parse({ + ...base, + mcpServers: [{ name: 'has spaces', url: 'https://a.example.test/mcp' }], + }), + ).toThrow(); + }); + + it('rejects more than 20 MCP server entries', () => { + const servers = Array.from({ length: 21 }, (_, i) => ({ + name: `s${i}`, + url: 'https://a.example.test/mcp', + })); + expect(() => AgentJsonConfigSchema.parse({ ...base, mcpServers: servers })).toThrow(); + }); + + it('parses an allow-mode toolFilter', () => { + const parsed = AgentJsonConfigSchema.parse({ + ...base, + mcpServers: [ + { + name: 'github', + url: 'https://a.example.test/mcp', + toolFilter: { mode: 'allow', tools: ['search_repositories'] }, + }, + ], + }); + expect(parsed.mcpServers?.[0].toolFilter).toEqual({ + mode: 'allow', + tools: ['search_repositories'], + }); + }); + + it('parses approval mode "global"', () => { + const parsed = AgentJsonConfigSchema.parse({ + ...base, + mcpServers: [ + { + name: 'github', + url: 'https://a.example.test/mcp', + approval: { mode: 'global' }, + }, + ], + }); + expect(parsed.mcpServers?.[0].approval).toEqual({ mode: 'global' }); + }); + + it('rejects approval mode "selected" with an empty tools array', () => { + expect(() => + AgentJsonConfigSchema.parse({ + ...base, + mcpServers: [ + { + name: 'github', + url: 'https://a.example.test/mcp', + approval: { mode: 'selected', tools: [] }, + }, + ], + }), + ).toThrow(); + }); + }); }); diff --git a/packages/cli/src/modules/agents/agents-tools.service.ts b/packages/cli/src/modules/agents/agents-tools.service.ts index 91e40af9f83..7035ad67001 100644 --- a/packages/cli/src/modules/agents/agents-tools.service.ts +++ b/packages/cli/src/modules/agents/agents-tools.service.ts @@ -7,8 +7,10 @@ import { isToolType, isTriggerNodeType } from 'n8n-workflow'; import type { IDataObject, INodeParameters } from 'n8n-workflow'; import { z } from 'zod'; -import { EphemeralNodeExecutor, isAgentProviderNode } from '@/node-execution'; +import { MCP_REGISTRY_PACKAGE_NAME } from '../mcp-registry/node-description-transform'; + import { NodeCatalogService } from '@/node-catalog'; +import { EphemeralNodeExecutor, isAgentProviderNode } from '@/node-execution'; type NodeRequest = | string @@ -38,9 +40,18 @@ export const isExecutableNodeType = (nodeId: string): boolean => !isTriggerNodeT * Exported as a stable reference so the catalog service can cache its * filtered search tool per filter identity. */ -export const isAgentToolNodeType = (nodeId: string): boolean => - isExecutableNodeType(nodeId) && - (isToolType(nodeId, { includeHitl: false }) || isAgentProviderNode(nodeId)); +export const isAgentToolNodeType = (nodeId: string): boolean => { + if (!isExecutableNodeType(nodeId)) { + return false; + } + const isAllowedTool = isToolType(nodeId, { includeHitl: false }) && !isMcpToolNodeType(nodeId); + const isAllowedProviderNode = isAgentProviderNode(nodeId); + return isAllowedTool || isAllowedProviderNode; +}; + +const MCP_CLIENT_TOOL_NODE_TYPE = '@n8n/n8n-nodes-langchain.mcpClientTool'; +const isMcpToolNodeType = (nodeId: string): boolean => + nodeId === MCP_CLIENT_TOOL_NODE_TYPE || nodeId.startsWith(MCP_REGISTRY_PACKAGE_NAME); const searchNodesInputSchema = z.object({ queries: z.array(z.string()).min(1).describe('Search queries (e.g., ["gmail", "slack", "http"])'), diff --git a/packages/cli/src/modules/agents/agents.service.ts b/packages/cli/src/modules/agents/agents.service.ts index 4288b4ddac8..b1c0ab62fef 100644 --- a/packages/cli/src/modules/agents/agents.service.ts +++ b/packages/cli/src/modules/agents/agents.service.ts @@ -19,6 +19,7 @@ import { type AgentCredentialIntegrationConfig, type AgentIntegrationConfig, type AgentJsonConfig, + type AgentJsonMcpServerConfig, type AgentJsonMemoryConfig, type AgentJsonToolConfig, type AgentSkill, @@ -55,6 +56,7 @@ import { ConflictError } from '@/errors/response-errors/conflict.error'; import { NotFoundError } from '@/errors/response-errors/not-found.error'; import { resolveBuiltinNodeDefinitionDirs } from '@/modules/instance-ai/node-definition-resolver'; import { EphemeralNodeExecutor } from '@/node-execution'; +import { OauthService } from '@/oauth/oauth.service'; import type { PubSubCommandMap } from '@/scaling/pubsub/pubsub.event-map'; import { Publisher } from '@/scaling/pubsub/publisher.service'; import { UrlService } from '@/services/url.service'; @@ -85,6 +87,7 @@ import { type MemoryFactory, type ToolResolver, } from './json-config/from-json-config'; +import { buildMcpClientForServer } from './json-config/mcp-client-factory'; import { AgentHistoryRepository } from './repositories/agent-history.repository'; import { AgentRepository } from './repositories/agent.repository'; import { AgentSecureRuntime } from './runtime/agent-secure-runtime'; @@ -231,7 +234,9 @@ export class AgentsService { private clearRuntimes(agentId: string, options: { skipBroadcast?: boolean } = {}): void { for (const key of this.runtimes.keys()) { if (key === agentId || key.startsWith(`${agentId}:`)) { + const entry = this.runtimes.get(key); this.runtimes.delete(key); + if (entry) this.closeAgentResources(entry.agent, agentId); } } @@ -286,12 +291,31 @@ export class AgentsService { private readonly globalConfig: GlobalConfig, private readonly telemetry: Telemetry, private readonly chatIntegrationService: ChatIntegrationService, + private readonly oauthService: OauthService, ) {} private isNodeToolsModuleEnabled(): boolean { return this.agentsConfig.modules.includes('node-tools-searcher'); } + private isMcpModuleEnabled(): boolean { + return this.agentsConfig.modules.includes('mcp'); + } + + /** + * Best-effort close of an agent instance. Delegates to `agent.close()` + * which disposes the runtime and disconnects any attached MCP clients. + * Errors are logged but never thrown. + */ + private closeAgentResources(agent: { close(): Promise }, agentId: string): void { + agent.close().catch((error) => { + this.logger.warn('[AgentsService] Failed to close agent resources on eviction', { + agentId, + error: error instanceof Error ? error.message : String(error), + }); + }); + } + private createAgentExecutionCounter({ agentId, userId, @@ -1360,18 +1384,6 @@ export class AgentsService { } } - /** - * Execute an SDK agent within a workflow execution context. - * - * Streams the run rather than calling `.generate()` so the same - * `ExecutionRecorder` used by chat/Slack/schedule paths can collect a full - * `MessageRecord` (timeline, tool calls, usage). Without this, sessions - * triggered from a workflow node never appear in the agent's session list - * because nothing creates the agent execution thread row. - * - * Compiles a fresh isolated agent per call for credential isolation (does - * not use or affect the shared runtime cache). - */ async executeForWorkflow( agentId: string, message: string, @@ -1539,6 +1551,22 @@ export class AgentsService { }; } + const mcpServers = config.mcpServers ?? []; + if (mcpServers.length > 0 && !this.isMcpModuleEnabled()) { + return { + valid: false, + error: 'MCP servers require the "mcp" agents module to be enabled.', + }; + } + for (const server of mcpServers) { + if (server.authentication !== 'none' && !server.credential) { + return { + valid: false, + error: `MCP server "${server.name}" requires a credential when authentication is not "none".`, + }; + } + } + try { this.validateNodeToolExpressions(config); } catch (error) { @@ -1606,6 +1634,7 @@ export class AgentsService { const memoryProvided = result.config.memory !== undefined; const providerToolsProvided = result.config.providerTools !== undefined; const configBlockProvided = result.config.config !== undefined; + const mcpServersProvided = result.config.mcpServers !== undefined; const { schemaConfig: decomposedSchema, integrations: decomposedIntegrations } = decomposeJsonConfig(result.config); @@ -1627,6 +1656,7 @@ export class AgentsService { ...(skillsProvided ? { skills: decomposedSchema.skills } : {}), ...(providerToolsProvided ? { providerTools: decomposedSchema.providerTools } : {}), ...(configBlockProvided ? { config: decomposedSchema.config } : {}), + ...(mcpServersProvided ? { mcpServers: decomposedSchema.mcpServers } : {}), }; entity.schema = nextSchema; @@ -1996,6 +2026,19 @@ export class AgentsService { const resolvedTools: BuiltTool[] = []; + // Only attach MCP clients when the module is enabled. Without the gate + // a previously-configured agent would still build live MCP connections + // after the operator removes the token from N8N_AGENTS_MODULES, which + // undermines the kill-switch. + const buildMcpClient = this.isMcpModuleEnabled() + ? async (server: AgentJsonMcpServerConfig) => + await buildMcpClientForServer(server, { + credentialProvider, + oauthService: this.oauthService, + projectId: agentEntity.projectId, + }) + : undefined; + const reconstructed = await buildFromJson(config, toolDescriptors, { toolExecutor, credentialProvider, @@ -2006,6 +2049,7 @@ export class AgentsService { }, skills: agentEntity.skills ?? {}, memoryFactory: this.getMemoryFactory(agentEntity.id), + buildMcpClient, }); await this.injectRuntimeDependencies({ diff --git a/packages/cli/src/modules/agents/builder/__tests__/agents-builder-model-recommendations.test.ts b/packages/cli/src/modules/agents/builder/__tests__/agents-builder-model-recommendations.test.ts index dabba395a7e..47139a8b985 100644 --- a/packages/cli/src/modules/agents/builder/__tests__/agents-builder-model-recommendations.test.ts +++ b/packages/cli/src/modules/agents/builder/__tests__/agents-builder-model-recommendations.test.ts @@ -91,6 +91,7 @@ function buildPrompt(modelRecommendationsSection: string | null) { toolList: '(none)', agentPreviewPath: '/projects/project-1/agents/agent-1/preview', modelRecommendationsSection, + enabledModules: [], }); } @@ -176,14 +177,14 @@ describe('builder model recommendations', () => { }); it('registers only optional builder runtime skills', () => { - expect(getBuilderRuntimeSkills().map((skill) => skill.id)).toEqual([ + expect(getBuilderRuntimeSkills({ enabledModules: [] }).map((skill) => skill.id)).toEqual([ 'agent-builder-integrations', 'agent-builder-target-skills', ]); }); it('does not tell the builder to prefer Slack OAuth credentials for chat integrations', () => { - const integrationsSkill = getBuilderRuntimeSkills().find( + const integrationsSkill = getBuilderRuntimeSkills({ enabledModules: [] }).find( (skill) => skill.id === 'agent-builder-integrations', ); diff --git a/packages/cli/src/modules/agents/builder/__tests__/agents-builder-prompts.test.ts b/packages/cli/src/modules/agents/builder/__tests__/agents-builder-prompts.test.ts new file mode 100644 index 00000000000..2864dd4f90a --- /dev/null +++ b/packages/cli/src/modules/agents/builder/__tests__/agents-builder-prompts.test.ts @@ -0,0 +1,52 @@ +import { getBuilderSkillRoutingSection } from '../agents-builder-prompts'; +import { getConfigMutationPrompt } from '../prompts/config-mutation.prompt'; +import { getBuilderRuntimeSkills } from '../skills'; + +describe('agents builder integrations prompt', () => { + it('does not tell the builder to prefer Slack OAuth credentials for chat integrations', () => { + const integrationsSkill = getBuilderRuntimeSkills({ enabledModules: [] }).find( + (skill) => skill.id === 'agent-builder-integrations', + ); + + expect(integrationsSkill?.instructions).not.toContain('slackOAuth2Api'); + expect(integrationsSkill?.instructions).not.toContain('prefer the OAuth variant'); + }); +}); + +describe('MCP skill gating', () => { + it('does not include the MCP skill when the "mcp" module is disabled', () => { + const skills = getBuilderRuntimeSkills({ + enabledModules: [], + }); + expect(skills.find((s) => s.id === 'agent-builder-mcp')).toBeUndefined(); + }); + + it('includes the MCP skill when the "mcp" module is enabled', () => { + const skills = getBuilderRuntimeSkills({ + enabledModules: ['mcp'], + }); + expect(skills.find((s) => s.id === 'agent-builder-mcp')).toBeDefined(); + }); + + it('omits the MCP skill from the routing section when the module is disabled', () => { + const section = getBuilderSkillRoutingSection([]); + expect(section).not.toContain('agent-builder-mcp'); + }); + + it('lists the MCP skill in the routing section when the module is enabled', () => { + const section = getBuilderSkillRoutingSection(['mcp']); + expect(section).toContain('agent-builder-mcp'); + }); +}); + +describe('Config mutation prompt gating', () => { + it('doesn\'t include the MCP servers section when the "mcp" module is disabled', () => { + const prompt = getConfigMutationPrompt([]); + expect(prompt).not.toContain('mcpServers'); + }); + + it('includes the MCP servers section when the "mcp" module is enabled', () => { + const prompt = getConfigMutationPrompt(['mcp']); + expect(prompt).toContain('mcpServers'); + }); +}); diff --git a/packages/cli/src/modules/agents/builder/__tests__/verify-mcp-server.tool.test.ts b/packages/cli/src/modules/agents/builder/__tests__/verify-mcp-server.tool.test.ts new file mode 100644 index 00000000000..214af0cf1ab --- /dev/null +++ b/packages/cli/src/modules/agents/builder/__tests__/verify-mcp-server.tool.test.ts @@ -0,0 +1,196 @@ +import type { CredentialProvider, McpClient } from '@n8n/agents'; +import { mock } from 'jest-mock-extended'; + +import type { OauthService } from '@/oauth/oauth.service'; + +import { buildVerifyMcpServerTool } from '../verify-mcp-server.tool'; + +// --------------------------------------------------------------------------- +// Module mocks +// --------------------------------------------------------------------------- + +const buildMcpClientForServerMock = jest.fn, [unknown, unknown]>(); + +jest.mock('../../json-config/mcp-client-factory', () => ({ + // eslint-disable-next-line @typescript-eslint/promise-function-async + buildMcpClientForServer: (arg0: unknown, arg1: unknown) => + buildMcpClientForServerMock(arg0, arg1), +})); + +// --------------------------------------------------------------------------- +// Helpers +// --------------------------------------------------------------------------- + +function makeDeps() { + return { + credentialProvider: mock(), + oauthService: mock(), + projectId: 'proj-1', + }; +} + +function makeMcpClient(overrides: Partial = {}): McpClient { + return { + listTools: jest.fn().mockResolvedValue([]), + close: jest.fn().mockResolvedValue(undefined), + ...overrides, + } as unknown as McpClient; +} + +// --------------------------------------------------------------------------- +// Tests +// --------------------------------------------------------------------------- + +describe('buildVerifyMcpServerTool', () => { + beforeEach(() => { + buildMcpClientForServerMock.mockReset(); + }); + + it('returns { ok: true, tools } with name and description on success', async () => { + const mcpClient = makeMcpClient({ + listTools: jest.fn().mockResolvedValue([ + { name: 'echo', description: 'Echo the input' }, + { name: 'add', description: 'Add two numbers' }, + ]), + }); + buildMcpClientForServerMock.mockResolvedValue(mcpClient); + + const tool = buildVerifyMcpServerTool(makeDeps()); + const result = await tool.handler!( + { name: 'my-server', url: 'https://example.test/mcp' }, + {} as never, + ); + + expect(result).toEqual({ + ok: true, + tools: [ + { name: 'echo', description: 'Echo the input' }, + { name: 'add', description: 'Add two numbers' }, + ], + }); + }); + + it('uses an empty string when a tool has no description', async () => { + const mcpClient = makeMcpClient({ + listTools: jest.fn().mockResolvedValue([{ name: 'silent-tool', description: undefined }]), + }); + buildMcpClientForServerMock.mockResolvedValue(mcpClient); + + const tool = buildVerifyMcpServerTool(makeDeps()); + const result = await tool.handler!( + { name: 'my-server', url: 'https://example.test/mcp' }, + {} as never, + ); + + expect(result).toEqual({ + ok: true, + tools: [{ name: 'silent-tool', description: '' }], + }); + }); + + it('returns { ok: false, error } when listTools throws', async () => { + const mcpClient = makeMcpClient({ + listTools: jest.fn().mockRejectedValue(new Error('connection timeout')), + }); + buildMcpClientForServerMock.mockResolvedValue(mcpClient); + + const tool = buildVerifyMcpServerTool(makeDeps()); + const result = await tool.handler!( + { name: 'my-server', url: 'https://example.test/mcp' }, + {} as never, + ); + + expect(result).toEqual({ ok: false, error: 'connection timeout' }); + }); + + it('returns { ok: false, error } with stringified non-Error rejections', async () => { + const mcpClient = makeMcpClient({ + listTools: jest.fn().mockRejectedValue('plain string error'), + }); + buildMcpClientForServerMock.mockResolvedValue(mcpClient); + + const tool = buildVerifyMcpServerTool(makeDeps()); + const result = await tool.handler!( + { name: 'my-server', url: 'https://example.test/mcp' }, + {} as never, + ); + + expect(result).toEqual({ ok: false, error: 'plain string error' }); + }); + + it('always closes the client — even when listTools fails', async () => { + const closeMock = jest.fn().mockResolvedValue(undefined); + const mcpClient = makeMcpClient({ + listTools: jest.fn().mockRejectedValue(new Error('boom')), + close: closeMock, + }); + buildMcpClientForServerMock.mockResolvedValue(mcpClient); + + const tool = buildVerifyMcpServerTool(makeDeps()); + await tool.handler!({ name: 'my-server', url: 'https://example.test/mcp' }, {} as never); + + expect(closeMock).toHaveBeenCalledTimes(1); + }); + + it('closes the client even when close() itself rejects', async () => { + const mcpClient = makeMcpClient({ + listTools: jest.fn().mockResolvedValue([]), + close: jest.fn().mockRejectedValue(new Error('close error')), + }); + buildMcpClientForServerMock.mockResolvedValue(mcpClient); + + const tool = buildVerifyMcpServerTool(makeDeps()); + // Should not throw even if close() rejects + await expect( + tool.handler!({ name: 'my-server', url: 'https://example.test/mcp' }, {} as never), + ).resolves.toEqual({ ok: true, tools: [] }); + }); + + it('forwards name, url, transport, authentication, credential, and connectionTimeoutMs to the factory', async () => { + const mcpClient = makeMcpClient(); + buildMcpClientForServerMock.mockResolvedValue(mcpClient); + + const deps = makeDeps(); + const tool = buildVerifyMcpServerTool(deps); + await tool.handler!( + { + name: 'my-server', + url: 'https://example.test/mcp', + transport: 'sse', + authentication: 'bearerAuth', + credential: 'cred-42', + connectionTimeoutMs: 10_000, + }, + {} as never, + ); + + expect(buildMcpClientForServerMock).toHaveBeenCalledWith( + expect.objectContaining({ + name: 'my-server', + url: 'https://example.test/mcp', + transport: 'sse', + authentication: 'bearerAuth', + credential: 'cred-42', + connectionTimeoutMs: 10_000, + }), + expect.objectContaining({ + credentialProvider: deps.credentialProvider, + oauthService: deps.oauthService, + projectId: deps.projectId, + }), + ); + }); + + it('omits connectionTimeoutMs from the factory call when not provided', async () => { + const mcpClient = makeMcpClient(); + buildMcpClientForServerMock.mockResolvedValue(mcpClient); + + const tool = buildVerifyMcpServerTool(makeDeps()); + await tool.handler!({ name: 'my-server', url: 'https://example.test/mcp' }, {} as never); + + const [serverArg] = buildMcpClientForServerMock.mock.calls[0] as unknown as [ + Record, + ]; + expect(serverArg).not.toHaveProperty('connectionTimeoutMs'); + }); +}); diff --git a/packages/cli/src/modules/agents/builder/agents-builder-prompts.ts b/packages/cli/src/modules/agents/builder/agents-builder-prompts.ts index 7fb189cf3c5..b665fedee9b 100644 --- a/packages/cli/src/modules/agents/builder/agents-builder-prompts.ts +++ b/packages/cli/src/modules/agents/builder/agents-builder-prompts.ts @@ -56,15 +56,32 @@ Never write empty, placeholder, or guessed \`instructions\`. If you do not have enough detail to write meaningful instructions, ask the user first.`; } -export const BUILDER_SKILL_ROUTING_SECTION = `\ -## Builder Runtime Skills +/** + * Build the routing section that tells the builder LLM which runtime skills + * exist and what they cover. Module-gated skills (like `agent-builder-mcp`) + * are only listed when their owning module is active so the LLM doesn't try + * to load a skill the runtime won't surface. + */ +export function getBuilderSkillRoutingSection(enabledModules?: ReadonlyArray): string { + const lines: string[] = [ + '- `agent-builder-integrations`: schedule and chat integrations.', + '- `agent-builder-target-skills`: creating skills for the target agent.', + ]; + + if (enabledModules?.includes('mcp')) { + lines.push( + '- `agent-builder-mcp`: adding, removing, or updating MCP (Model Context Protocol) servers.', + ); + } + + return `\ +## Builder runtime skills Additional specialized builder guidance is available through runtime skills. Before these specialized tasks, call \`load_skill\` with \`{ "skillId": "" }\` and follow the returned instructions. -- \`agent-builder-integrations\`: schedule and chat integrations. -- \`agent-builder-target-skills\`: creating skills for the target agent. +${lines.join('\n')} Requests for "web search", "Brave web search", or "SearXNG web search" are agent config changes, not node-tool tasks. Follow the Config schema reference: @@ -74,6 +91,7 @@ to add a Brave/SearXNG node tool or node integration. Do not use \`create_skill\` for your own builder guidance. \`create_skill\` creates a skill for the target agent only.`; +} export const INTERACTIVE_TOOLS_SECTION = `\ ## Interactive tools @@ -209,6 +227,7 @@ export interface BuilderPromptContext { toolList: string; agentPreviewPath: string; modelRecommendationsSection: string | null; + enabledModules: string[]; } export function buildBuilderPrompt(ctx: BuilderPromptContext): string { @@ -219,6 +238,7 @@ export function buildBuilderPrompt(ctx: BuilderPromptContext): string { toolList, agentPreviewPath, modelRecommendationsSection, + enabledModules, } = ctx; const sections = [ @@ -226,11 +246,11 @@ export function buildBuilderPrompt(ctx: BuilderPromptContext): string { TARGET_AGENT_SECTION, getAgentStateSection(configJson, configHash, configUpdatedAt, toolList), getConversationModeSection(agentPreviewPath), - getConfigMutationPrompt(), + getConfigMutationPrompt(enabledModules), getLlmSelectionPrompt(modelRecommendationsSection), MEMORY_PROMPT, TOOLS_PROMPT, - BUILDER_SKILL_ROUTING_SECTION, + getBuilderSkillRoutingSection(enabledModules), INTERACTIVE_TOOLS_SECTION, N8N_EXPRESSIONS_SECTION, READ_CONFIG_FRESHNESS_SECTION, diff --git a/packages/cli/src/modules/agents/builder/agents-builder-tools.service.ts b/packages/cli/src/modules/agents/builder/agents-builder-tools.service.ts index 3adf9102991..13c4fa6f216 100644 --- a/packages/cli/src/modules/agents/builder/agents-builder-tools.service.ts +++ b/packages/cli/src/modules/agents/builder/agents-builder-tools.service.ts @@ -33,6 +33,8 @@ import { } from './interactive'; import type { ModelLookup } from './interactive/resolve-llm.tool'; import { BUILDER_TOOLS } from './builder-tool-names'; +import { buildVerifyMcpServerTool } from './verify-mcp-server.tool'; +import { OauthService } from '@/oauth/oauth.service'; const EMPTY_INSTRUCTIONS_ERROR: ConfigValidationError = { path: '/instructions', @@ -176,6 +178,7 @@ export class AgentsBuilderToolsService { private readonly workflowRepository: WorkflowRepository, private readonly agentsToolsService: AgentsToolsService, private readonly builderModelLookupService: BuilderModelLookupService, + private readonly oauthService: OauthService, private readonly credentialTypes: CredentialTypes, ) {} @@ -184,9 +187,10 @@ export class AgentsBuilderToolsService { projectId: string, credentialProvider: CredentialProvider, user: User, + enabledModules?: ReadonlyArray, ): BuilderTools { return { - json: this.getJsonTools(agentId, projectId, credentialProvider, user), + json: this.getJsonTools(agentId, projectId, credentialProvider, user, enabledModules), shared: this.getSharedTools(agentId, projectId, credentialProvider), }; } @@ -196,6 +200,7 @@ export class AgentsBuilderToolsService { projectId: string, credentialProvider: CredentialProvider, user: User, + enabledModules?: ReadonlyArray, ): BuiltTool[] { const readConfigTool = new Tool(BUILDER_TOOLS.READ_CONFIG) .description( @@ -419,7 +424,7 @@ export class AgentsBuilderToolsService { await this.builderModelLookupService.list(user, credentialId, credentialType, lookup), }; - return [ + const tools: BuiltTool[] = [ readConfigTool, writeConfigTool, patchConfigTool, @@ -432,6 +437,18 @@ export class AgentsBuilderToolsService { buildAskLlmTool(), buildAskQuestionTool(), ]; + + if (enabledModules?.includes('mcp')) { + tools.push( + buildVerifyMcpServerTool({ + credentialProvider, + oauthService: this.oauthService, + projectId, + }), + ); + } + + return tools; } private getSharedTools( diff --git a/packages/cli/src/modules/agents/builder/agents-builder.service.ts b/packages/cli/src/modules/agents/builder/agents-builder.service.ts index 97e30a89036..00dd7755aa1 100644 --- a/packages/cli/src/modules/agents/builder/agents-builder.service.ts +++ b/packages/cli/src/modules/agents/builder/agents-builder.service.ts @@ -6,6 +6,7 @@ import type { Agent as RuntimeAgent, } from '@n8n/agents'; import { Logger } from '@n8n/backend-common'; +import { AgentsConfig } from '@n8n/config'; import type { User } from '@n8n/db'; import { Service } from '@n8n/di'; import { jsonParse, UserError } from 'n8n-workflow'; @@ -44,6 +45,7 @@ export class AgentsBuilderService { private readonly builderSettings: AgentsBuilderSettingsService, private readonly n8nCheckpointStorage: N8NCheckpointStorage, private readonly agentCheckpointRepository: AgentCheckpointRepository, + private readonly agentsConfig: AgentsConfig, ) {} // --------------------------------------------------------------------------- @@ -176,6 +178,7 @@ export class AgentsBuilderService { const configJson = currentConfig ? JSON.stringify(currentConfig, null, 2) : '(no config yet)'; const modelRecommendationsSection = await getModelRecommendationsSection(); + const enabledModules = this.agentsConfig.modules; const instructions = buildBuilderPrompt({ configJson, configHash: getAgentConfigHash(currentConfig), @@ -183,14 +186,18 @@ export class AgentsBuilderService { toolList, agentPreviewPath: buildAgentPreviewPath(projectId, agentId), modelRecommendationsSection, + enabledModules, + }); + const runtimeSkills = getBuilderRuntimeSkills({ + enabledModules, }); - const runtimeSkills = getBuilderRuntimeSkills(); const tools = this.agentsBuilderToolsService.getTools( agentId, projectId, credentialProvider, user, + enabledModules, ); const { Agent, Memory } = await import('@n8n/agents'); diff --git a/packages/cli/src/modules/agents/builder/builder-tool-names.ts b/packages/cli/src/modules/agents/builder/builder-tool-names.ts index 0b3e7f2a6c1..8b14160bb1c 100644 --- a/packages/cli/src/modules/agents/builder/builder-tool-names.ts +++ b/packages/cli/src/modules/agents/builder/builder-tool-names.ts @@ -10,6 +10,7 @@ export const BUILDER_TOOLS = { CREATE_SKILL: 'create_skill', LIST_INTEGRATION_TYPES: 'list_integration_types', RESOLVE_LLM: 'resolve_llm', + VERIFY_MCP_SERVER: 'verify_mcp_server', } as const; export type BuilderToolName = (typeof BUILDER_TOOLS)[keyof typeof BUILDER_TOOLS]; diff --git a/packages/cli/src/modules/agents/builder/prompts/config-mutation.prompt.ts b/packages/cli/src/modules/agents/builder/prompts/config-mutation.prompt.ts index 5f646f25777..79587d7a3ae 100644 --- a/packages/cli/src/modules/agents/builder/prompts/config-mutation.prompt.ts +++ b/packages/cli/src/modules/agents/builder/prompts/config-mutation.prompt.ts @@ -1,6 +1,6 @@ import { getConfigRulesSection, getSchemaReferenceSection } from './config-rules.prompt'; -export function getConfigMutationPrompt(): string { +export function getConfigMutationPrompt(enabledModules: string[]): string { return `\ ## Config Mutation Guidance @@ -24,7 +24,7 @@ Use this after deciding a config change is needed and before calling ${getConfigRulesSection()} -${getSchemaReferenceSection()} +${getSchemaReferenceSection(enabledModules)} - Follow the Config schema reference exactly; do not invent top-level fields. - Keep each feature in the schema path where it belongs. diff --git a/packages/cli/src/modules/agents/builder/prompts/config-rules.prompt.ts b/packages/cli/src/modules/agents/builder/prompts/config-rules.prompt.ts index a5c97f6ccf8..abc386757e3 100644 --- a/packages/cli/src/modules/agents/builder/prompts/config-rules.prompt.ts +++ b/packages/cli/src/modules/agents/builder/prompts/config-rules.prompt.ts @@ -1,4 +1,5 @@ import type { JSONSchema7 } from 'json-schema'; +import type { ZodObject, ZodRawShape } from 'zod'; import { z } from 'zod'; import { zodToJsonSchema } from 'zod-to-json-schema'; @@ -74,10 +75,15 @@ export function getConfigRulesSection(): string { is written.`; } -export function getSchemaReferenceSection(): string { - const jsonSchemaText = jsonSchemaToCompactText( - zodToJsonSchema(BuilderPromptAgentJsonConfigSchema) as JSONSchema7, - ); +export function getSchemaReferenceSection(enabledModules: string[]): string { + let zodSchema: ZodObject = BuilderPromptAgentJsonConfigSchema; + // don't let agent know about MCP servers if it's not enabled + if (!enabledModules.includes('mcp')) { + zodSchema = zodSchema.omit({ + mcpServers: true, + }); + } + const jsonSchemaText = jsonSchemaToCompactText(zodToJsonSchema(zodSchema) as JSONSchema7); return `\ #### Config Schema Reference diff --git a/packages/cli/src/modules/agents/builder/skills/index.ts b/packages/cli/src/modules/agents/builder/skills/index.ts index c1e5480adc0..25f6ced0029 100644 --- a/packages/cli/src/modules/agents/builder/skills/index.ts +++ b/packages/cli/src/modules/agents/builder/skills/index.ts @@ -1,10 +1,13 @@ import type { RuntimeSkill } from '@n8n/agents'; import { integrationsSkill } from './integrations.skill'; +import { mcpSkill } from './mcp.skill'; import { targetSkillsSkill } from './target-skills.skill'; -export function getBuilderRuntimeSkills(): RuntimeSkill[] { - return [ +export function getBuilderRuntimeSkills(options: { + enabledModules?: readonly string[]; +}): RuntimeSkill[] { + const skills: RuntimeSkill[] = [ integrationsSkill(), targetSkillsSkill(), // FIXME: Research is disabled until the builder has a supported research tool. @@ -12,4 +15,10 @@ export function getBuilderRuntimeSkills(): RuntimeSkill[] { // instead of merely loading instructions that tell it to research. // researchSkill(), ]; + + if (options.enabledModules?.includes('mcp')) { + skills.push(mcpSkill()); + } + + return skills; } diff --git a/packages/cli/src/modules/agents/builder/skills/mcp.skill.ts b/packages/cli/src/modules/agents/builder/skills/mcp.skill.ts new file mode 100644 index 00000000000..bd3757480ec --- /dev/null +++ b/packages/cli/src/modules/agents/builder/skills/mcp.skill.ts @@ -0,0 +1,92 @@ +import type { RuntimeSkill } from '@n8n/agents'; +import { ASK_QUESTION_TOOL_NAME } from '@n8n/api-types'; + +export function mcpSkill(): RuntimeSkill { + return { + id: 'agent-builder-mcp', + name: 'Agent builder MCP servers', + description: + 'Use when adding, removing, or updating MCP (Model Context Protocol) servers on the target agent.', + instructions: `\ +## Purpose + +Use this to manage external MCP server connections in the target agent config. +MCP servers expose external tool catalogs to the target agent over HTTP. They +live on the top-level \`mcpServers\` array, and each entry maps 1:1 to a +connected MCP server. + +## Boundaries + +- Prefer existing n8n workflow or node tools when the integration is already + available, since they execute inside the n8n runtime with full credential + handling. +- Use MCP when the user names a specific MCP server (for example "GitHub MCP", + "Slack MCP", or "Notion MCP"), or when no equivalent workflow or node tool + exists. + +## Workflow + +Each \`mcpServers[]\` entry supports: + +- \`name\` (required, unique within \`mcpServers\`, 1-64 chars, /^[A-Za-z0-9_-]+$/) +- \`url\` (required) +- \`transport\`: \`"sse"\` | \`"streamableHttp"\` (default \`"streamableHttp"\`) +- \`authentication\`: \`"none"\` | \`"bearerAuth"\` | \`"headerAuth"\` | + \`"multipleHeadersAuth"\` | \`"mcpOAuth2Api"\` | + string ending in \`"McpOAuth2Api"\` (default \`"none"\`) +- \`credential\`: required when authentication !== \`"none"\` (must be the id + returned by \`ask_credential\`) +- \`toolFilter\` (optional): \`{ mode: "allow" | "exclude", tools: string[] }\`, + matched against original (un-prefixed) tool names +- \`approval\` (optional): \`{ mode: "global" }\` for all tools, or + \`{ mode: "selected", tools: [...] }\` for specific tools (must be non-empty) +- \`connectionTimeoutMs\` (optional): 1-120000 +- \`metadata\` (optional): optional server-generated metadata. Do not use this + field unless explicitly instructed to do so by instructions + +### Credential flow + +- For \`bearerAuth\`, call \`ask_credential\` with + \`credentialType: "httpBearerAuth"\`. +- For \`headerAuth\`, call \`ask_credential\` with + \`credentialType: "httpHeaderAuth"\`. +- For \`multipleHeadersAuth\`, call \`ask_credential\` with + \`credentialType: "httpMultipleHeadersAuth"\`. +- For \`mcpOAuth2Api\`, call \`ask_credential\` with + \`credentialType: "mcpOAuth2Api"\`. +- Never invent credential IDs. If the user declines, omit the server entirely + rather than persisting a stub. + +### Testing the connection + +Before writing to config, call \`verify_mcp_server\` with server \`name\`, +\`url\`, \`transport\`, and (if applicable) the credential id from +\`ask_credential\`. + +- Success returns \`{ ok: true, tools: [{ name, description }] }\`. +- Use the returned tool list to populate \`toolFilter.tools\` or + \`approval.tools\` so the user does not need to type tool names manually. +- Failure returns \`{ ok: false, error: "..." }\`. +- If verification fails, explain the error and ask the user to check the URL + or credentials before proceeding. + +### Selecting credentials + +When connection testing without credentials fails and you do not know which +credential type to use, ask the user which credential type to use: OAuth2, +Bearer Token, Header Auth, Multiple Headers Auth, or None. Use +\`${ASK_QUESTION_TOOL_NAME}\` to ask. Based on the response, call +\`ask_credential\` with the appropriate credential type. + +### Patch pattern + +1. Initialize the array if missing: + \`{ "op": "add", "path": "/mcpServers", "value": [] }\` +2. Append each server: + \`{ "op": "add", "path": "/mcpServers/-", "value": { ... } }\` + +## Gotchas + +- Server \`name\` must be unique across \`mcpServers\` within an agent.`, + }; +} diff --git a/packages/cli/src/modules/agents/builder/verify-mcp-server.tool.ts b/packages/cli/src/modules/agents/builder/verify-mcp-server.tool.ts new file mode 100644 index 00000000000..5603ee2bbcd --- /dev/null +++ b/packages/cli/src/modules/agents/builder/verify-mcp-server.tool.ts @@ -0,0 +1,102 @@ +import { Tool } from '@n8n/agents/tool'; +import type { BuiltTool, CredentialProvider, McpClient } from '@n8n/agents'; +import { z } from 'zod'; + +import type { OauthService } from '@/oauth/oauth.service'; + +import { buildMcpClientForServer } from '../json-config/mcp-client-factory'; +import { BUILDER_TOOLS } from './builder-tool-names'; + +export interface VerifyMcpServerDeps { + credentialProvider: CredentialProvider; + oauthService: OauthService; + projectId: string; +} + +/** + * Input schema mirrors the required subset of `McpServerConfigSchema` that the + * builder can have in hand before writing the config. The credential field is + * optional here so the tool can also be used to test unauthenticated servers. + * + * `toolFilter` and `approval` are intentionally excluded — they have no bearing + * on whether the underlying transport connects, and simplifying the input keeps + * the LLM context small. + */ +const verifyMcpServerInputSchema = z.object({ + name: z + .string() + .min(1) + .max(64) + .regex(/^[a-zA-Z0-9_-]+$/) + .describe('The server name (used as the tool-name prefix)'), + url: z.string().min(1).describe('The MCP server endpoint URL'), + transport: z + .enum(['sse', 'streamableHttp']) + .default('streamableHttp') + .describe('Transport type. Defaults to streamableHttp'), + authentication: z + .enum(['none', 'bearerAuth', 'headerAuth', 'multipleHeadersAuth', 'mcpOAuth2Api']) + .default('none') + .describe('Authentication scheme'), + credential: z + .string() + .optional() + .describe( + 'Credential id returned by ask_credential. Required when authentication is not "none"', + ), + connectionTimeoutMs: z + .number() + .int() + .min(1) + .max(120_000) + .optional() + .describe('Connection timeout in milliseconds'), +}); + +type VerifyMcpServerInput = z.infer; + +export function buildVerifyMcpServerTool(deps: VerifyMcpServerDeps): BuiltTool { + return new Tool(BUILDER_TOOLS.VERIFY_MCP_SERVER) + .description( + 'Test connectivity to an MCP server before adding it to the agent config. ' + + 'Establishes a temporary connection, lists the available tools, then closes the connection. ' + + 'Returns { ok: true, tools: [{ name, description }] } on success, or ' + + '{ ok: false, error: string } on failure. ' + + 'Call this after ask_credential (when authentication is not "none") and before patch_config.', + ) + .input(verifyMcpServerInputSchema) + .handler(async (input: VerifyMcpServerInput) => { + let client: McpClient | undefined; + try { + client = await buildMcpClientForServer( + { + name: input.name, + url: input.url, + transport: input.transport, + authentication: input.authentication, + credential: input.credential, + ...(input.connectionTimeoutMs !== undefined && { + connectionTimeoutMs: input.connectionTimeoutMs, + }), + }, + deps, + ); + const tools = await client.listTools(); + return { + ok: true, + tools: tools.map((t) => ({ + name: t.name, + description: t.description ?? '', + })), + }; + } catch (error) { + return { + ok: false, + error: error instanceof Error ? error.message : String(error), + }; + } finally { + await client?.close().catch(() => {}); + } + }) + .build(); +} diff --git a/packages/cli/src/modules/agents/json-config/__tests__/mcp-client-factory.test.ts b/packages/cli/src/modules/agents/json-config/__tests__/mcp-client-factory.test.ts new file mode 100644 index 00000000000..afbbafb693c --- /dev/null +++ b/packages/cli/src/modules/agents/json-config/__tests__/mcp-client-factory.test.ts @@ -0,0 +1,505 @@ +import type { CredentialProvider } from '@n8n/agents'; +import type { AgentJsonMcpServerConfig } from '@n8n/api-types'; +import { mock } from 'jest-mock-extended'; + +import type { OauthService } from '@/oauth/oauth.service'; + +import { buildMcpClientForServer, createAuthFetch, mapApprovalToSdk } from '../mcp-client-factory'; + +// --------------------------------------------------------------------------- +// Module mocks +// --------------------------------------------------------------------------- + +const mcpClientCtor = jest.fn(); +jest.mock('@n8n/agents', () => ({ + McpClient: jest.fn(function (configs: unknown) { + mcpClientCtor(configs); + return { configs, close: jest.fn() }; + }), +})); + +const proxyFetchMock = jest.fn(); +jest.mock('@n8n/ai-utilities', () => ({ + proxyFetch: (...args: unknown[]) => proxyFetchMock(...args), +})); + +// --------------------------------------------------------------------------- +// Helpers +// --------------------------------------------------------------------------- + +function makeServer(overrides: Partial = {}): AgentJsonMcpServerConfig { + return { + name: 'srv', + url: 'https://example.test/mcp', + transport: 'streamableHttp', + authentication: 'none', + ...overrides, + } as AgentJsonMcpServerConfig; +} + +function makeOk(): Response { + return new Response('ok', { status: 200 }); +} + +function make401(): Response { + return new Response('unauthorized', { status: 401 }); +} + +// --------------------------------------------------------------------------- +// mapApprovalToSdk +// --------------------------------------------------------------------------- + +describe('mapApprovalToSdk', () => { + it('returns undefined when approval is absent', () => { + expect(mapApprovalToSdk(undefined)).toBeUndefined(); + }); + + it('maps mode "global" to literal true (all tools require approval)', () => { + expect(mapApprovalToSdk({ mode: 'global' })).toBe(true); + }); + + it('maps mode "selected" to the literal tools list', () => { + expect(mapApprovalToSdk({ mode: 'selected', tools: ['a', 'b'] })).toEqual(['a', 'b']); + }); +}); + +// --------------------------------------------------------------------------- +// createAuthFetch +// --------------------------------------------------------------------------- + +describe('createAuthFetch', () => { + beforeEach(() => { + proxyFetchMock.mockReset(); + }); + + it('routes through proxyFetch and injects the initial headers', async () => { + proxyFetchMock.mockResolvedValueOnce(makeOk()); + + const fetchFn = createAuthFetch({ initialHeaders: { Authorization: 'Bearer A' } }); + const res = await fetchFn('https://example.test/mcp'); + + expect(res.status).toBe(200); + expect(proxyFetchMock).toHaveBeenCalledTimes(1); + const [, init] = proxyFetchMock.mock.calls[0] as [unknown, RequestInit]; + expect(init.headers).toMatchObject({ Authorization: 'Bearer A' }); + }); + + it('returns 401 unchanged when no onUnauthorized handler is configured', async () => { + proxyFetchMock.mockResolvedValueOnce(make401()); + + const fetchFn = createAuthFetch({ initialHeaders: { Authorization: 'Bearer A' } }); + const res = await fetchFn('https://example.test/mcp'); + + expect(res.status).toBe(401); + expect(proxyFetchMock).toHaveBeenCalledTimes(1); + }); + + it('returns the original 401 when onUnauthorized returns null', async () => { + proxyFetchMock.mockResolvedValueOnce(make401()); + + const onUnauthorized = jest.fn().mockResolvedValue(null); + const fetchFn = createAuthFetch({ + initialHeaders: { Authorization: 'Bearer A' }, + onUnauthorized, + }); + const res = await fetchFn('https://example.test/mcp'); + + expect(res.status).toBe(401); + expect(onUnauthorized).toHaveBeenCalledTimes(1); + expect(proxyFetchMock).toHaveBeenCalledTimes(1); + }); + + it('retries once with refreshed headers when onUnauthorized returns new headers', async () => { + proxyFetchMock.mockResolvedValueOnce(make401()).mockResolvedValueOnce(makeOk()); + + const onUnauthorized = jest.fn().mockResolvedValue({ Authorization: 'Bearer B' }); + const fetchFn = createAuthFetch({ + initialHeaders: { Authorization: 'Bearer A' }, + onUnauthorized, + }); + const res = await fetchFn('https://example.test/mcp'); + + expect(res.status).toBe(200); + expect(proxyFetchMock).toHaveBeenCalledTimes(2); + const [, init2] = proxyFetchMock.mock.calls[1] as [unknown, RequestInit]; + expect(init2.headers).toMatchObject({ Authorization: 'Bearer B' }); + }); +}); + +// --------------------------------------------------------------------------- +// buildMcpClientForServer — header derivation per auth type +// --------------------------------------------------------------------------- + +describe('buildMcpClientForServer — header derivation', () => { + beforeEach(() => { + mcpClientCtor.mockReset(); + proxyFetchMock.mockReset(); + proxyFetchMock.mockResolvedValue(makeOk()); + }); + + async function captureInitialHeaders(server: AgentJsonMcpServerConfig, resolved: unknown) { + const credentialProvider = mock(); + credentialProvider.resolve.mockResolvedValue(resolved as never); + const oauthService = mock(); + + await buildMcpClientForServer(server, { + credentialProvider, + oauthService, + projectId: 'proj-1', + }); + + const [configs] = mcpClientCtor.mock.calls[0] as [Array<{ fetch: typeof fetch }>]; + const fetchFn = configs[0].fetch; + await fetchFn('https://example.test/mcp'); + const [, init] = proxyFetchMock.mock.calls[0] as [unknown, RequestInit]; + return (init.headers ?? {}) as Record; + } + + it('sends no auth headers for authentication: "none"', async () => { + const headers = await captureInitialHeaders(makeServer({ authentication: 'none' }), null); + expect(headers.Authorization).toBeUndefined(); + }); + + it('sends a Bearer header for bearerAuth', async () => { + const headers = await captureInitialHeaders( + makeServer({ authentication: 'bearerAuth', credential: 'cred-1' }), + { token: 'tok123' }, + ); + expect(headers.Authorization).toBe('Bearer tok123'); + }); + + it('sends the configured name/value pair for headerAuth', async () => { + const headers = await captureInitialHeaders( + makeServer({ authentication: 'headerAuth', credential: 'cred-1' }), + { name: 'X-Api-Key', value: 'secret' }, + ); + expect(headers['X-Api-Key']).toBe('secret'); + }); + + it('flattens the values array for multipleHeadersAuth', async () => { + const headers = await captureInitialHeaders( + makeServer({ authentication: 'multipleHeadersAuth', credential: 'cred-1' }), + { + headers: { + values: [ + { name: 'X-One', value: 'one' }, + { name: 'X-Two', value: 'two' }, + ], + }, + }, + ); + expect(headers['X-One']).toBe('one'); + expect(headers['X-Two']).toBe('two'); + }); + + it('uses oauthTokenData.access_token for mcpOAuth2Api', async () => { + const headers = await captureInitialHeaders( + makeServer({ authentication: 'mcpOAuth2Api', credential: 'cred-1' }), + { oauthTokenData: { access_token: 'oauth-token' } }, + ); + expect(headers.Authorization).toBe('Bearer oauth-token'); + }); +}); + +// --------------------------------------------------------------------------- +// buildMcpClientForServer — OAuth2 refresh path +// --------------------------------------------------------------------------- + +describe('buildMcpClientForServer — OAuth2 refresh on 401', () => { + beforeEach(() => { + mcpClientCtor.mockReset(); + proxyFetchMock.mockReset(); + }); + + it('invokes oauthService.refreshOAuth2CredentialById once on 401 and retries with the refreshed header', async () => { + proxyFetchMock.mockResolvedValueOnce(make401()).mockResolvedValueOnce(makeOk()); + + const credentialProvider = mock(); + credentialProvider.resolve.mockResolvedValue({ + oauthTokenData: { access_token: 'stale-token' }, + } as never); + + const oauthService = mock(); + oauthService.refreshOAuth2CredentialById.mockResolvedValue({ + Authorization: 'Bearer fresh-token', + }); + + await buildMcpClientForServer( + makeServer({ authentication: 'mcpOAuth2Api', credential: 'cred-1' }), + { credentialProvider, oauthService, projectId: 'proj-1' }, + ); + + const [configs] = mcpClientCtor.mock.calls[0] as [Array<{ fetch: typeof fetch }>]; + const fetchFn = configs[0].fetch; + const res = await fetchFn('https://example.test/mcp'); + + expect(res.status).toBe(200); + expect(oauthService.refreshOAuth2CredentialById).toHaveBeenCalledWith('cred-1', 'proj-1'); + // First call uses the stale header, second uses the refreshed one. + const [, firstInit] = proxyFetchMock.mock.calls[0] as [unknown, RequestInit]; + const [, secondInit] = proxyFetchMock.mock.calls[1] as [unknown, RequestInit]; + expect((firstInit.headers as Record).Authorization).toBe('Bearer stale-token'); + expect((secondInit.headers as Record).Authorization).toBe('Bearer fresh-token'); + }); + + it('does NOT call refreshOAuth2CredentialById for non-OAuth2 auth schemes', async () => { + proxyFetchMock.mockResolvedValueOnce(make401()); + + const credentialProvider = mock(); + credentialProvider.resolve.mockResolvedValue({ token: 'static' } as never); + + const oauthService = mock(); + + await buildMcpClientForServer( + makeServer({ authentication: 'bearerAuth', credential: 'cred-1' }), + { credentialProvider, oauthService, projectId: 'proj-1' }, + ); + + const [configs] = mcpClientCtor.mock.calls[0] as [Array<{ fetch: typeof fetch }>]; + await configs[0].fetch('https://example.test/mcp'); + + expect(oauthService.refreshOAuth2CredentialById).not.toHaveBeenCalled(); + }); +}); + +// --------------------------------------------------------------------------- +// buildMcpClientForServer — SDK config mapping +// --------------------------------------------------------------------------- + +describe('buildMcpClientForServer — SDK config mapping', () => { + beforeEach(() => { + mcpClientCtor.mockReset(); + proxyFetchMock.mockReset(); + }); + + it('forwards toolFilter, approval, transport, and connectionTimeoutMs to the SDK config', async () => { + const credentialProvider = mock(); + const oauthService = mock(); + + await buildMcpClientForServer( + makeServer({ + transport: 'sse', + toolFilter: { mode: 'allow', tools: ['echo'] }, + approval: { mode: 'selected', tools: ['create'] }, + connectionTimeoutMs: 5_000, + }), + { credentialProvider, oauthService, projectId: 'proj-1' }, + ); + + const [configs] = mcpClientCtor.mock.calls[0] as [Array>]; + expect(configs).toHaveLength(1); + expect(configs[0]).toMatchObject({ + name: 'srv', + url: 'https://example.test/mcp', + transport: 'sse', + toolFilter: { mode: 'allow', tools: ['echo'] }, + requireApproval: ['create'], + connectionTimeoutMs: 5_000, + }); + expect(typeof configs[0].fetch).toBe('function'); + }); + + it('omits connectionTimeoutMs from the SDK config when not provided', async () => { + const credentialProvider = mock(); + const oauthService = mock(); + + await buildMcpClientForServer(makeServer(), { + credentialProvider, + oauthService, + projectId: 'proj-1', + }); + + const [configs] = mcpClientCtor.mock.calls[0] as [Array>]; + expect(configs[0]).not.toHaveProperty('connectionTimeoutMs'); + }); +}); + +// --------------------------------------------------------------------------- +// createAuthFetch — header merging and stateful refresh +// --------------------------------------------------------------------------- + +describe('createAuthFetch — header merging', () => { + beforeEach(() => { + proxyFetchMock.mockReset(); + proxyFetchMock.mockResolvedValue(new Response('ok', { status: 200 })); + }); + + it('merges caller-supplied init.headers with auth headers (auth takes precedence)', async () => { + const fetchFn = createAuthFetch({ initialHeaders: { Authorization: 'Bearer A' } }); + await fetchFn('https://example.test/mcp', { headers: { 'X-Custom': 'value' } }); + + const [, init] = proxyFetchMock.mock.calls[0] as [unknown, RequestInit]; + expect(init.headers).toMatchObject({ + 'X-Custom': 'value', + Authorization: 'Bearer A', + }); + }); + + it('uses the refreshed headers on the second call after a successful 401 refresh', async () => { + proxyFetchMock + .mockResolvedValueOnce(new Response('unauthorized', { status: 401 })) + .mockResolvedValueOnce(new Response('ok', { status: 200 })) + .mockResolvedValueOnce(new Response('ok', { status: 200 })); + + let callCount = 0; + const onUnauthorized = jest.fn().mockImplementation(async () => { + callCount++; + return { Authorization: `Bearer refreshed-${callCount}` }; + }); + + const fetchFn = createAuthFetch({ + initialHeaders: { Authorization: 'Bearer stale' }, + onUnauthorized, + }); + + // First call triggers a 401 → refresh → retry + await fetchFn('https://example.test/mcp'); + + // Second call should use the refreshed headers without triggering another refresh + await fetchFn('https://example.test/mcp'); + + expect(onUnauthorized).toHaveBeenCalledTimes(1); + const [, thirdInit] = proxyFetchMock.mock.calls[2] as [unknown, RequestInit]; + expect((thirdInit.headers as Record).Authorization).toBe('Bearer refreshed-1'); + }); +}); + +// --------------------------------------------------------------------------- +// buildMcpClientForServer — auth header edge cases +// --------------------------------------------------------------------------- + +describe('buildMcpClientForServer — auth header edge cases', () => { + beforeEach(() => { + mcpClientCtor.mockReset(); + proxyFetchMock.mockReset(); + proxyFetchMock.mockResolvedValue(new Response('ok', { status: 200 })); + }); + + async function captureInitialHeaders(server: AgentJsonMcpServerConfig, resolved: unknown) { + const credentialProvider = mock(); + credentialProvider.resolve.mockResolvedValue(resolved as never); + const oauthService = mock(); + + await buildMcpClientForServer(server, { + credentialProvider, + oauthService, + projectId: 'proj-1', + }); + + const [configs] = mcpClientCtor.mock.calls[0] as [Array<{ fetch: typeof fetch }>]; + await configs[0].fetch('https://example.test/mcp'); + const [, init] = proxyFetchMock.mock.calls[0] as [unknown, RequestInit]; + return (init.headers ?? {}) as Record; + } + + it('sends no Authorization header for bearerAuth when the resolved token is an empty string', async () => { + const headers = await captureInitialHeaders( + makeServer({ authentication: 'bearerAuth', credential: 'cred-1' }), + { token: '' }, + ); + expect(headers.Authorization).toBeUndefined(); + }); + + it('sends no header for headerAuth when the name is missing from the resolved credential', async () => { + const headers = await captureInitialHeaders( + makeServer({ authentication: 'headerAuth', credential: 'cred-1' }), + { name: '', value: 'secret' }, + ); + expect(Object.keys(headers)).not.toContain(''); + expect(headers.Authorization).toBeUndefined(); + }); + + it('skips entries with non-string values in multipleHeadersAuth', async () => { + const headers = await captureInitialHeaders( + makeServer({ authentication: 'multipleHeadersAuth', credential: 'cred-1' }), + { + headers: { + values: [ + { name: 'X-Valid', value: 'yes' }, + { name: 42, value: 'ignored' }, + { name: 'X-Also-Ignored', value: null }, + ], + }, + }, + ); + expect(headers['X-Valid']).toBe('yes'); + expect(Object.keys(headers)).not.toContain('42'); + expect(headers['X-Also-Ignored']).toBeUndefined(); + }); + + it('sends no headers for multipleHeadersAuth when the values array is absent', async () => { + const headers = await captureInitialHeaders( + makeServer({ authentication: 'multipleHeadersAuth', credential: 'cred-1' }), + { headers: { values: undefined } }, + ); + expect(headers).toEqual({}); + }); + + it('uses the OAuth2 path for service-specific McpOAuth2Api variants (e.g. notionMcpOAuth2Api)', async () => { + const headers = await captureInitialHeaders( + makeServer({ authentication: 'notionMcpOAuth2Api' as never, credential: 'cred-1' }), + { oauthTokenData: { access_token: 'notion-oauth-token' } }, + ); + expect(headers.Authorization).toBe('Bearer notion-oauth-token'); + }); +}); + +// --------------------------------------------------------------------------- +// buildMcpClientForServer — OAuth2 variant wires refresh handler +// --------------------------------------------------------------------------- + +describe('buildMcpClientForServer — service-specific McpOAuth2Api refresh', () => { + beforeEach(() => { + mcpClientCtor.mockReset(); + proxyFetchMock.mockReset(); + }); + + it('wires the onUnauthorized refresh handler for non-canonical McpOAuth2Api variants', async () => { + proxyFetchMock + .mockResolvedValueOnce(new Response('unauthorized', { status: 401 })) + .mockResolvedValueOnce(new Response('ok', { status: 200 })); + + const credentialProvider = mock(); + credentialProvider.resolve.mockResolvedValue({ + oauthTokenData: { access_token: 'stale' }, + } as never); + + const oauthService = mock(); + oauthService.refreshOAuth2CredentialById.mockResolvedValue({ + Authorization: 'Bearer fresh', + }); + + await buildMcpClientForServer( + makeServer({ authentication: 'notionMcpOAuth2Api' as never, credential: 'cred-1' }), + { credentialProvider, oauthService, projectId: 'proj-1' }, + ); + + const [configs] = mcpClientCtor.mock.calls[0] as [Array<{ fetch: typeof fetch }>]; + const res = await configs[0].fetch('https://example.test/mcp'); + + expect(res.status).toBe(200); + expect(oauthService.refreshOAuth2CredentialById).toHaveBeenCalledWith('cred-1', 'proj-1'); + }); + + it('does NOT wire an onUnauthorized handler when credential is absent for mcpOAuth2Api', async () => { + proxyFetchMock.mockResolvedValueOnce(new Response('unauthorized', { status: 401 })); + + const credentialProvider = mock(); + credentialProvider.resolve.mockResolvedValue({} as never); + + const oauthService = mock(); + + // credential field intentionally absent + await buildMcpClientForServer(makeServer({ authentication: 'mcpOAuth2Api' }), { + credentialProvider, + oauthService, + projectId: 'proj-1', + }); + + const [configs] = mcpClientCtor.mock.calls[0] as [Array<{ fetch: typeof fetch }>]; + await configs[0].fetch('https://example.test/mcp'); + + // No refresh should have been attempted + expect(oauthService.refreshOAuth2CredentialById).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/cli/src/modules/agents/json-config/from-json-config.ts b/packages/cli/src/modules/agents/json-config/from-json-config.ts index 71ab6ed6a2c..9f4fae7f1d3 100644 --- a/packages/cli/src/modules/agents/json-config/from-json-config.ts +++ b/packages/cli/src/modules/agents/json-config/from-json-config.ts @@ -3,6 +3,7 @@ import type { BuiltMemory, BuiltTool, CredentialProvider, + McpClient, ModelConfig, ToolDescriptor, JSONObject, @@ -13,6 +14,7 @@ import { wrapToolForApproval } from '@n8n/agents/tool'; import type { AgentSkill, AgentJsonConfig, + AgentJsonMcpServerConfig, AgentJsonMemoryConfig, AgentJsonToolConfig, AgentJsonSkillConfig, @@ -47,6 +49,14 @@ export interface ToolExecutor { /** Factory function that reconstructs a BuiltMemory backend from serialized params. */ export type MemoryFactory = (params: AgentJsonMemoryConfig) => BuiltMemory | Promise; +/** + * Build an SDK `McpClient` from a single JSON-config MCP server entry. The + * platform layer owns this factory because it needs access to the credential + * store and OAuth2 refresh infrastructure, both of which are out of scope for + * `buildFromJson`. + */ +export type McpClientBuilder = (server: AgentJsonMcpServerConfig) => Promise; + type MemoryWorkerModelConfig = { model: string; credential: string; @@ -62,6 +72,14 @@ export interface BuildFromJsonOptions { skills?: Record; /** Memory backend factories keyed by storage preset name. */ memoryFactory: MemoryFactory; + /** + * When provided, each entry in `config.mcpServers` is built into an + * `McpClient` and attached to the agent via `agent.mcp(client)`. The + * platform layer is responsible for tracking returned clients for + * teardown when the runtime is evicted. + * + */ + buildMcpClient?: McpClientBuilder; } /** @@ -94,6 +112,14 @@ export async function buildFromJson( } } } + + if (config.mcpServers?.length && options.buildMcpClient) { + for (const server of config.mcpServers) { + const client = await options.buildMcpClient(server); + agent.mcp(client); + } + } + agent.skills(configuredSkills); // Provider tools diff --git a/packages/cli/src/modules/agents/json-config/mcp-client-factory.ts b/packages/cli/src/modules/agents/json-config/mcp-client-factory.ts new file mode 100644 index 00000000000..bc34d91eb73 --- /dev/null +++ b/packages/cli/src/modules/agents/json-config/mcp-client-factory.ts @@ -0,0 +1,201 @@ +import { proxyFetch } from '@n8n/ai-utilities'; +import type { CredentialProvider, McpClient, McpServerConfig } from '@n8n/agents'; +import type { AgentJsonMcpServerConfig } from '@n8n/api-types'; +import { isMcpOAuth2Authentication } from 'n8n-workflow'; + +import type { OauthService } from '@/oauth/oauth.service'; + +/** + * Convert the JSON-config `approval` shape into the SDK's `requireApproval` + * field. The two representations carry the same semantics: + * + * - `undefined` -> `undefined` (no per-server approval) + * - `{ mode: 'global' }` -> `true` (every tool requires approval) + * - `{ mode: 'selected' }` -> `string[]` (only listed tools require approval) + */ +export function mapApprovalToSdk( + approval: AgentJsonMcpServerConfig['approval'], +): McpServerConfig['requireApproval'] { + if (!approval) return undefined; + if (approval.mode === 'global') return true; + return approval.tools; +} + +function isTokenData(tokenData: unknown): tokenData is { access_token: string } { + return ( + typeof tokenData === 'object' && + tokenData !== null && + 'access_token' in tokenData && + typeof tokenData.access_token === 'string' + ); +} + +/** + * Derive static (non-OAuth2) auth headers from a credential resolved through + * the agents `CredentialProvider`. Mirrors the shape of `getAuthHeaders` in + * the langchain MCP node — kept inline here so the agents module does not + * have to depend on `@n8n/nodes-langchain`. + * + * For any `*McpOAuth2Api` credential type, the Bearer header is computed from + * the already-stored `oauthTokenData.access_token`. Refresh-on-401 is handled + * by `createAuthFetch` below; this function only computes the initial set. + */ +async function deriveAuthHeaders( + server: AgentJsonMcpServerConfig, + credentialProvider: CredentialProvider, +): Promise> { + if (server.authentication === 'none' || !server.credential) return {}; + + const resolved = await credentialProvider.resolve(server.credential).catch(() => null); + if (!resolved) return {}; + + if (isMcpOAuth2Authentication(server.authentication)) { + const tokenData = resolved.oauthTokenData as { access_token: string } | null | undefined; + if (!isTokenData(tokenData)) return {}; + return { + Authorization: `Bearer ${tokenData.access_token}`, + }; + } + + switch (server.authentication) { + case 'bearerAuth': { + const token = typeof resolved.token === 'string' ? resolved.token : ''; + return token ? { Authorization: `Bearer ${token}` } : {}; + } + case 'headerAuth': { + const name = typeof resolved.name === 'string' ? resolved.name : ''; + const value = typeof resolved.value === 'string' ? resolved.value : ''; + return name && value ? { [name]: value } : {}; + } + case 'multipleHeadersAuth': { + const headers = resolved.headers; + if ( + !headers || + typeof headers !== 'object' || + !('values' in headers) || + !Array.isArray((headers as { values: unknown }).values) + ) { + return {}; + } + const values = (headers as { values: Array<{ name?: unknown; value?: unknown }> }).values; + const out: Record = {}; + for (const entry of values) { + if (typeof entry.name === 'string' && typeof entry.value === 'string') { + out[entry.name] = entry.value; + } + } + return out; + } + default: + return {}; + } +} + +interface CreateAuthFetchOptions { + initialHeaders: Record; + /** + * Called on a 401 response. Should return a fresh set of auth headers, or + * `null` if the refresh failed. The returned headers replace the cached + * set used by subsequent requests. + */ + onUnauthorized?: () => Promise | null>; +} + +function headersToRecord(headers: HeadersInit | undefined): Record { + if (!headers) return {}; + if (headers instanceof Headers) return Object.fromEntries(headers.entries()); + if (Array.isArray(headers)) return Object.fromEntries(headers); + return headers; +} + +/** + * Build a fetch wrapper that: + * 1. routes through n8n's `proxyFetch` (so corporate HTTP_PROXY settings + * apply uniformly), + * 2. injects the latest auth headers on every request, + * 3. on a single 401, calls `onUnauthorized` to refresh the token and + * retries the request once with the new headers. + * + * This mirrors the langchain MCP node's `createAuthFetch` so an agent's MCP + * connection behaves identically to one configured via the workflow editor. + */ +export function createAuthFetch({ + initialHeaders, + onUnauthorized, +}: CreateAuthFetchOptions): typeof fetch { + let headers = initialHeaders; + + return async (input: RequestInfo | URL, init?: RequestInit): Promise => { + const response = await proxyFetch(input, { + ...init, + headers: { ...headersToRecord(init?.headers), ...headers }, + }); + + if (response.status !== 401 || !onUnauthorized) return response; + + const refreshed = await onUnauthorized(); + if (!refreshed) return response; + + headers = refreshed; + return await proxyFetch(input, { + ...init, + headers: { ...headersToRecord(init?.headers), ...headers }, + }); + }; +} + +export interface BuildMcpClientDeps { + credentialProvider: CredentialProvider; + /** + * Used to refresh OAuth2 tokens on a 401 response without an + * `IExecuteFunctions` workflow context. Only invoked when + * `server.authentication` is any `*McpOAuth2Api` credential type. + */ + oauthService: OauthService; + projectId: string; +} + +/** + * Build a connected-but-lazy SDK `McpClient` for a single JSON-config MCP + * server entry. The returned client opens its transport on first use + * (`agent.mcp(client)` → `client.listTools()` during agent run). + * + * Callers are responsible for keeping the returned client referenced for the + * lifetime of the runtime and calling `.close()` when the runtime is evicted. + */ +export async function buildMcpClientForServer( + server: AgentJsonMcpServerConfig, + deps: BuildMcpClientDeps, +): Promise { + const { credentialProvider, oauthService, projectId } = deps; + const { McpClient } = await import('@n8n/agents'); + + const initialHeaders = await deriveAuthHeaders(server, credentialProvider); + + const onUnauthorized = + isMcpOAuth2Authentication(server.authentication) && server.credential + ? async () => { + const credentialId = server.credential; + if (!credentialId) return null; + return await oauthService + .refreshOAuth2CredentialById(credentialId, projectId) + .catch(() => null); + } + : undefined; + + const authFetch = createAuthFetch({ initialHeaders, onUnauthorized }); + + const sdkServerConfig: McpServerConfig = { + name: server.name, + url: server.url, + transport: server.transport, + fetch: authFetch, + toolFilter: server.toolFilter, + requireApproval: mapApprovalToSdk(server.approval), + ...(server.connectionTimeoutMs !== undefined && { + connectionTimeoutMs: server.connectionTimeoutMs, + }), + }; + + return new McpClient([sdkServerConfig]); +} diff --git a/packages/cli/src/oauth/__tests__/oauth.service.test.ts b/packages/cli/src/oauth/__tests__/oauth.service.test.ts index efbbce108f7..53dd45b4497 100644 --- a/packages/cli/src/oauth/__tests__/oauth.service.test.ts +++ b/packages/cli/src/oauth/__tests__/oauth.service.test.ts @@ -3214,4 +3214,240 @@ describe('OauthService', () => { ).toBe('direct@example.com'); }); }); + + describe('refreshOAuth2CredentialById', () => { + const credentialId = 'cred-123'; + const projectId = 'proj-456'; + + function makeCredential( + overrides: Partial = {}, + ): ICredentialsDb & { isGlobal: boolean } { + return { + id: credentialId, + isGlobal: false, + shared: [], + ...overrides, + } as unknown as ICredentialsDb & { isGlobal: boolean }; + } + + it('returns null when the credential is not found', async () => { + credentialsRepository.findOne.mockResolvedValue(null); + + const result = await service.refreshOAuth2CredentialById(credentialId, projectId); + + expect(result).toBeNull(); + }); + + it('returns null when the credential is not accessible to the given project', async () => { + const credential = makeCredential({ + isGlobal: false, + shared: [{ projectId: 'other-project' }] as never, + }); + credentialsRepository.findOne.mockResolvedValue(credential as never); + + const result = await service.refreshOAuth2CredentialById(credentialId, projectId); + + expect(result).toBeNull(); + }); + + it('grants access when the credential is global regardless of project', async () => { + const credential = makeCredential({ isGlobal: true, shared: [] }); + credentialsRepository.findOne.mockResolvedValue(credential as never); + // Returns null because there's no oauthTokenData — but the access check passed + jest.spyOn(service, 'getOAuthCredentials').mockResolvedValue({ + clientId: 'id', + clientSecret: 'secret', + accessTokenUrl: 'https://example.com/token', + grantType: 'authorizationCode', + authentication: 'header', + } as unknown as OAuth2CredentialData); + + const result = await service.refreshOAuth2CredentialById(credentialId, projectId); + + // Null because oauthTokenData is missing — not because of an access denial + expect(result).toBeNull(); + expect(service.getOAuthCredentials).toHaveBeenCalled(); + }); + + it('grants access when the project is a shared member', async () => { + const credential = makeCredential({ + isGlobal: false, + shared: [{ projectId }] as never, + }); + credentialsRepository.findOne.mockResolvedValue(credential as never); + jest.spyOn(service, 'getOAuthCredentials').mockResolvedValue({ + clientId: 'id', + clientSecret: 'secret', + accessTokenUrl: 'https://example.com/token', + grantType: 'authorizationCode', + authentication: 'header', + } as unknown as OAuth2CredentialData); + + const result = await service.refreshOAuth2CredentialById(credentialId, projectId); + + // Access was granted — null because there's no oauthTokenData + expect(result).toBeNull(); + expect(service.getOAuthCredentials).toHaveBeenCalled(); + }); + + it('returns null when the credential has no stored oauthTokenData', async () => { + credentialsRepository.findOne.mockResolvedValue(makeCredential({ isGlobal: true }) as never); + jest.spyOn(service, 'getOAuthCredentials').mockResolvedValue({ + clientId: 'id', + clientSecret: 'secret', + accessTokenUrl: 'https://example.com/token', + grantType: 'authorizationCode', + authentication: 'header', + // oauthTokenData intentionally absent + } as unknown as OAuth2CredentialData); + + const result = await service.refreshOAuth2CredentialById(credentialId, projectId); + + expect(result).toBeNull(); + }); + + it('refreshes the token with token.refresh() for authorizationCode grant and returns a Bearer header', async () => { + const { ClientOAuth2 } = await import('@n8n/client-oauth2'); + const refreshed = { + data: { access_token: 'new-token', token_type: 'bearer' }, + accessToken: 'new-token', + }; + const mockToken = { refresh: jest.fn().mockResolvedValue(refreshed), client: {} }; + jest + .mocked(ClientOAuth2) + .mockImplementation(() => ({ createToken: jest.fn().mockReturnValue(mockToken) }) as never); + + credentialsRepository.findOne.mockResolvedValue(makeCredential({ isGlobal: true }) as never); + jest.spyOn(service, 'getOAuthCredentials').mockResolvedValue({ + clientId: 'id', + clientSecret: 'secret', + accessTokenUrl: 'https://example.com/token', + grantType: 'authorizationCode', + authentication: 'header', + oauthTokenData: { + access_token: 'stale', + refresh_token: 'refresh-tok', + token_type: 'bearer', + }, + } as unknown as OAuth2CredentialData); + jest.spyOn(service, 'encryptAndSaveData').mockResolvedValue(undefined); + + const result = await service.refreshOAuth2CredentialById(credentialId, projectId); + + expect(result).toEqual({ Authorization: 'Bearer new-token' }); + expect(mockToken.refresh).toHaveBeenCalledTimes(1); + }); + + it('persists the refreshed token data after a successful refresh', async () => { + const { ClientOAuth2 } = await import('@n8n/client-oauth2'); + const refreshedData = { access_token: 'new-token', token_type: 'bearer' }; + const refreshed = { data: refreshedData, accessToken: 'new-token' }; + const mockToken = { refresh: jest.fn().mockResolvedValue(refreshed), client: {} }; + const credential = makeCredential({ isGlobal: true }); + jest + .mocked(ClientOAuth2) + .mockImplementation(() => ({ createToken: jest.fn().mockReturnValue(mockToken) }) as never); + + credentialsRepository.findOne.mockResolvedValue(credential as never); + jest.spyOn(service, 'getOAuthCredentials').mockResolvedValue({ + clientId: 'id', + clientSecret: 'secret', + accessTokenUrl: 'https://example.com/token', + grantType: 'authorizationCode', + authentication: 'header', + oauthTokenData: { access_token: 'stale', refresh_token: 'refresh-tok' }, + } as unknown as OAuth2CredentialData); + jest.spyOn(service, 'encryptAndSaveData').mockResolvedValue(undefined); + + await service.refreshOAuth2CredentialById(credentialId, projectId); + + expect(service.encryptAndSaveData).toHaveBeenCalledWith(credential, { + oauthTokenData: refreshedData, + }); + }); + + it('uses credentials.getToken() for clientCredentials grant type', async () => { + const { ClientOAuth2 } = await import('@n8n/client-oauth2'); + const refreshed = { data: { access_token: 'cc-token' }, accessToken: 'cc-token' }; + const getToken = jest.fn().mockResolvedValue(refreshed); + const mockToken = { refresh: jest.fn(), client: { credentials: { getToken } } }; + jest + .mocked(ClientOAuth2) + .mockImplementation(() => ({ createToken: jest.fn().mockReturnValue(mockToken) }) as never); + + credentialsRepository.findOne.mockResolvedValue(makeCredential({ isGlobal: true }) as never); + jest.spyOn(service, 'getOAuthCredentials').mockResolvedValue({ + clientId: 'id', + clientSecret: 'secret', + accessTokenUrl: 'https://example.com/token', + grantType: 'clientCredentials', + authentication: 'header', + oauthTokenData: { access_token: 'stale' }, + } as unknown as OAuth2CredentialData); + jest.spyOn(service, 'encryptAndSaveData').mockResolvedValue(undefined); + + const result = await service.refreshOAuth2CredentialById(credentialId, projectId); + + expect(result).toEqual({ Authorization: 'Bearer cc-token' }); + expect(getToken).toHaveBeenCalledTimes(1); + expect(mockToken.refresh).not.toHaveBeenCalled(); + }); + + it('returns null and logs a warning when the refresh call throws', async () => { + const { ClientOAuth2 } = await import('@n8n/client-oauth2'); + const mockToken = { + refresh: jest.fn().mockRejectedValue(new Error('network timeout')), + client: {}, + }; + jest + .mocked(ClientOAuth2) + .mockImplementation(() => ({ createToken: jest.fn().mockReturnValue(mockToken) }) as never); + + credentialsRepository.findOne.mockResolvedValue(makeCredential({ isGlobal: true }) as never); + jest.spyOn(service, 'getOAuthCredentials').mockResolvedValue({ + clientId: 'id', + clientSecret: 'secret', + accessTokenUrl: 'https://example.com/token', + grantType: 'authorizationCode', + authentication: 'header', + oauthTokenData: { access_token: 'stale', refresh_token: 'refresh-tok' }, + } as unknown as OAuth2CredentialData); + + const result = await service.refreshOAuth2CredentialById(credentialId, projectId); + + expect(result).toBeNull(); + expect(logger.warn).toHaveBeenCalledWith( + 'Failed to refresh OAuth2 token for credential', + expect.objectContaining({ credentialId, error: 'network timeout' }), + ); + }); + + it('still returns the auth header even when persisting the new token data fails', async () => { + const { ClientOAuth2 } = await import('@n8n/client-oauth2'); + const refreshed = { data: { access_token: 'new-token' }, accessToken: 'new-token' }; + const mockToken = { refresh: jest.fn().mockResolvedValue(refreshed), client: {} }; + jest + .mocked(ClientOAuth2) + .mockImplementation(() => ({ createToken: jest.fn().mockReturnValue(mockToken) }) as never); + + credentialsRepository.findOne.mockResolvedValue(makeCredential({ isGlobal: true }) as never); + jest.spyOn(service, 'getOAuthCredentials').mockResolvedValue({ + clientId: 'id', + clientSecret: 'secret', + accessTokenUrl: 'https://example.com/token', + grantType: 'authorizationCode', + authentication: 'header', + oauthTokenData: { access_token: 'stale', refresh_token: 'refresh-tok' }, + } as unknown as OAuth2CredentialData); + jest.spyOn(service, 'encryptAndSaveData').mockRejectedValue(new Error('db write error')); + + const result = await service.refreshOAuth2CredentialById(credentialId, projectId); + + expect(result).toEqual({ Authorization: 'Bearer new-token' }); + expect(logger.warn).toHaveBeenCalledWith( + 'Refreshed OAuth2 token but failed to persist new token data', + expect.objectContaining({ credentialId }), + ); + }); + }); }); diff --git a/packages/cli/src/oauth/oauth.service.ts b/packages/cli/src/oauth/oauth.service.ts index 66b7e3b8704..3e1adcc195f 100644 --- a/packages/cli/src/oauth/oauth.service.ts +++ b/packages/cli/src/oauth/oauth.service.ts @@ -26,6 +26,7 @@ import * as WorkflowExecuteAdditionalData from '@/workflow-execute-additional-da import { ClientOAuth2, type ClientOAuth2Options, + type ClientOAuth2TokenData, type OAuth2AuthenticationMethod, type OAuth2CredentialData, type OAuth2GrantType, @@ -417,6 +418,77 @@ export class OauthService { return oauthCredentials; } + /** + * Refresh the OAuth2 token stored on a credential by id, persist the refreshed token data, + * and return the new auth headers to inject into outbound requests. + */ + async refreshOAuth2CredentialById( + credentialId: string, + projectId: string, + ): Promise | null> { + const credential = await this.credentialsRepository.findOne({ + where: { id: credentialId }, + relations: { shared: true }, + }); + if (!credential) return null; + + const isAccessible = + credential.isGlobal || (credential.shared ?? []).some((s) => s.projectId === projectId); + if (!isAccessible) return null; + + const oauthCredentials = await this.getOAuthCredentials(credential); + const oauthTokenData = oauthCredentials.oauthTokenData as ClientOAuth2TokenData | undefined; + if (!oauthTokenData) return null; + + const scopes = oauthCredentials.scope + ?.split(' ') + .map((s) => s.trim()) + .filter(Boolean); + + const oAuthClient = new ClientOAuth2({ + clientId: oauthCredentials.clientId, + clientSecret: oauthCredentials.clientSecret, + accessTokenUri: oauthCredentials.accessTokenUrl, + scopes: scopes?.length ? scopes : undefined, + ignoreSSLIssues: oauthCredentials.ignoreSSLIssues, + authentication: oauthCredentials.authentication ?? 'header', + }); + + const token = oAuthClient.createToken( + { + ...oauthTokenData, + ...(oauthTokenData.access_token ? { access_token: oauthTokenData.access_token } : {}), + ...(oauthTokenData.refresh_token ? { refresh_token: oauthTokenData.refresh_token } : {}), + }, + oauthTokenData.token_type, + ); + + let refreshed; + try { + refreshed = + oauthCredentials.grantType === 'clientCredentials' + ? await token.client.credentials.getToken() + : await token.refresh(); + } catch (error) { + this.logger.warn('Failed to refresh OAuth2 token for credential', { + credentialId, + error: error instanceof Error ? error.message : String(error), + }); + return null; + } + + try { + await this.encryptAndSaveData(credential, { oauthTokenData: refreshed.data }); + } catch (error) { + this.logger.warn('Refreshed OAuth2 token but failed to persist new token data', { + credentialId, + error: error instanceof Error ? error.message : String(error), + }); + } + + return { Authorization: `Bearer ${refreshed.accessToken}` }; + } + /** * Checks whether the credential type (after merging inherited properties) exposes * a user-editable `scope` property. A property is considered editable when it is diff --git a/packages/workflow/src/index.ts b/packages/workflow/src/index.ts index a4ce1858235..0bb52ece3c4 100644 --- a/packages/workflow/src/index.ts +++ b/packages/workflow/src/index.ts @@ -24,6 +24,7 @@ export * from './from-ai-parse-utils'; export * from './node-helpers'; export * from './node-validation'; export * from './node-grouping-validation'; +export * from './mcp-helpers'; export * from './tool-helpers'; export * from './node-reference-parser-utils'; export * from './metadata-utils'; diff --git a/packages/workflow/src/mcp-helpers.ts b/packages/workflow/src/mcp-helpers.ts new file mode 100644 index 00000000000..c89c740cfbb --- /dev/null +++ b/packages/workflow/src/mcp-helpers.ts @@ -0,0 +1,12 @@ +/** Covers `mcpOAuth2Api` and registry-specific variants like `notionMcpOAuth2Api`. */ +export type McpOAuth2CredentialType = 'mcpOAuth2Api' | `${string}McpOAuth2Api`; + +/** + * Returns `true` for `mcpOAuth2Api` and any credential type ending in + * `McpOAuth2Api` (e.g. `notionMcpOAuth2Api`, `githubMcpOAuth2Api`). + */ +export function isMcpOAuth2Authentication( + authentication: string, +): authentication is McpOAuth2CredentialType { + return authentication === 'mcpOAuth2Api' || authentication.endsWith('McpOAuth2Api'); +} diff --git a/packages/workflow/test/mcp-helpers.test.ts b/packages/workflow/test/mcp-helpers.test.ts new file mode 100644 index 00000000000..e389716441e --- /dev/null +++ b/packages/workflow/test/mcp-helpers.test.ts @@ -0,0 +1,24 @@ +import { isMcpOAuth2Authentication } from '../src/mcp-helpers'; + +describe('isMcpOAuth2Authentication', () => { + it('returns true for the canonical "mcpOAuth2Api" type', () => { + expect(isMcpOAuth2Authentication('mcpOAuth2Api')).toBe(true); + }); + + it('returns true for service-specific variants ending in "McpOAuth2Api"', () => { + expect(isMcpOAuth2Authentication('notionMcpOAuth2Api')).toBe(true); + expect(isMcpOAuth2Authentication('githubMcpOAuth2Api')).toBe(true); + expect(isMcpOAuth2Authentication('slackMcpOAuth2Api')).toBe(true); + }); + + it('returns false for static auth types', () => { + expect(isMcpOAuth2Authentication('bearerAuth')).toBe(false); + expect(isMcpOAuth2Authentication('headerAuth')).toBe(false); + expect(isMcpOAuth2Authentication('multipleHeadersAuth')).toBe(false); + expect(isMcpOAuth2Authentication('none')).toBe(false); + }); + + it('returns false for an empty string', () => { + expect(isMcpOAuth2Authentication('')).toBe(false); + }); +});