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 index 481756beaf3..43dde950728 100644 --- 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 @@ -35,6 +35,7 @@ function makeDeps(overrides: Partial function makeMcpClient(overrides: Partial = {}): McpClient { return { listTools: vi.fn().mockResolvedValue([]), + getConnectionFailures: vi.fn().mockReturnValue([]), close: vi.fn().mockResolvedValue(undefined), ...overrides, } as unknown as McpClient; @@ -142,6 +143,59 @@ describe('buildVerifyMcpServerTool', () => { }); }); + it('returns { ok: false, error } when the connection fails with no tools', async () => { + const mcpClient = makeMcpClient({ + listTools: vi.fn().mockResolvedValue([]), + getConnectionFailures: vi + .fn() + .mockReturnValue([{ server: 'my-server', error: 'fetch failed' }]), + }); + 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: 'MCP server "my-server" connection failed: fetch failed', + }); + }); + + it('does not apply the credential when the connection fails with no tools', async () => { + const applyCredentialToMcpServer = vi.fn(); + const mcpClient = makeMcpClient({ + listTools: vi.fn().mockResolvedValue([]), + getConnectionFailures: vi + .fn() + .mockReturnValue([{ server: 'my-server', error: 'fetch failed' }]), + }); + buildMcpClientForServerMock.mockResolvedValue(mcpClient); + + const tool = buildVerifyMcpServerTool( + makeDeps({ + agentId: 'agent-1', + applyCredentialToMcpServer, + }), + ); + const result = await tool.handler!( + { + name: 'my-server', + url: 'https://example.test/mcp', + credential: 'cred-42', + }, + {} as never, + ); + + expect(result).toEqual({ + ok: false, + error: 'MCP server "my-server" connection failed: fetch failed', + }); + expect(applyCredentialToMcpServer).not.toHaveBeenCalled(); + }); + it('returns { ok: false, error } when listTools throws', async () => { const mcpClient = makeMcpClient({ listTools: vi.fn().mockRejectedValue(new Error('connection timeout')), 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 index e2f259e8809..35f0086f768 100644 --- a/packages/cli/src/modules/agents/builder/verify-mcp-server.tool.ts +++ b/packages/cli/src/modules/agents/builder/verify-mcp-server.tool.ts @@ -154,6 +154,17 @@ export function buildVerifyMcpServerTool(deps: VerifyMcpServerDeps): BuiltTool { deps, ); const tools = await listToolsWithinDeadline(client, timeoutMs, ctx.abortSignal); + const failures = client.getConnectionFailures(); + if (failures.length > 0) { + return { + ok: false, + error: failures + .map( + (failure) => `MCP server "${failure.server}" connection failed: ${failure.error}`, + ) + .join('; '), + }; + } const mappedTools = tools.map((t) => ({ name: t.mcpToolName ?? t.name, description: t.description ?? '', 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 index 7177ae5a416..c1560257d27 100644 --- 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 @@ -2,7 +2,7 @@ import type { CredentialProvider } from '@n8n/agents'; import type { AgentJsonMcpServerConfig } from '@n8n/api-types'; import type { CustomFetch } from '@n8n/backend-network'; import { mock } from 'vitest-mock-extended'; -import { UserError } from 'n8n-workflow'; +import { OperationalError, UserError } from 'n8n-workflow'; import type { OauthService } from '@/oauth/oauth.service'; @@ -19,10 +19,16 @@ import { const mcpClientCtor = vi.fn(); const listToolsMock = vi.fn(); const closeMock = vi.fn(); +const getConnectionFailuresMock = vi.fn(); vi.mock('@n8n/agents', () => ({ McpClient: vi.fn(function (configs: unknown) { mcpClientCtor(configs); - return { configs, close: closeMock, listTools: listToolsMock }; + return { + configs, + close: closeMock, + listTools: listToolsMock, + getConnectionFailures: getConnectionFailuresMock, + }; }), })); @@ -800,6 +806,8 @@ describe('listMcpServerTools', () => { listToolsMock.mockReset(); closeMock.mockReset(); closeMock.mockResolvedValue(undefined); + getConnectionFailuresMock.mockReset(); + getConnectionFailuresMock.mockReturnValue([]); }); it('returns name/description pairs (empty description fallback) and closes the client', async () => { @@ -824,6 +832,17 @@ describe('listMcpServerTools', () => { expect(closeMock).toHaveBeenCalledTimes(1); }); + it('rejects a recorded connection failure and still closes the client', async () => { + listToolsMock.mockResolvedValue([]); + getConnectionFailuresMock.mockReturnValue([{ server: 'srv', error: 'fetch failed' }]); + + const pending = listMcpServerTools(makeServer(), deps()); + + await expect(pending).rejects.toBeInstanceOf(OperationalError); + await expect(pending).rejects.toThrow('MCP server "srv" connection failed: fetch failed'); + expect(closeMock).toHaveBeenCalledTimes(1); + }); + it('swallows close errors', async () => { listToolsMock.mockResolvedValue([{ name: 'echo', description: 'd' }]); closeMock.mockRejectedValue(new Error('already closed')); 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 index 56860584c25..d2ca10b31bc 100644 --- a/packages/cli/src/modules/agents/json-config/mcp-client-factory.ts +++ b/packages/cli/src/modules/agents/json-config/mcp-client-factory.ts @@ -306,6 +306,14 @@ export async function listMcpServerTools( try { client = await buildMcpClientForServer(server, deps); const tools = await client.listTools(); + const failures = client.getConnectionFailures(); + if (failures.length > 0) { + throw new OperationalError( + failures + .map((failure) => `MCP server "${failure.server}" connection failed: ${failure.error}`) + .join('; '), + ); + } return tools.map((tool) => ({ name: tool.name, description: tool.description ?? '' })); } finally { await client?.close().catch(() => {}); diff --git a/packages/cli/src/modules/mcp/__tests__/agent-tools.service.test.ts b/packages/cli/src/modules/mcp/__tests__/agent-tools.service.test.ts index e7aefccb191..0157b859298 100644 --- a/packages/cli/src/modules/mcp/__tests__/agent-tools.service.test.ts +++ b/packages/cli/src/modules/mcp/__tests__/agent-tools.service.test.ts @@ -9,6 +9,7 @@ import { Container } from '@n8n/di'; import { TELEMETRY_EVENT } from '@n8n/telemetry'; import type { Mock } from 'vitest'; import { mock } from 'vitest-mock-extended'; +import { OperationalError } from 'n8n-workflow'; vi.mock('@/permissions.ee/check-access', () => ({ userHasScopes: vi.fn(), @@ -1840,6 +1841,21 @@ describe('McpAgentToolsService', () => { authentication: 'none', }; + it('returns ok: false when the MCP connection fails', async () => { + outboundHttp.transport.mockReturnValue({ asCustomFetch: () => vi.fn() } as never); + listMcpServerToolsMock.mockRejectedValue( + new OperationalError('MCP server "srv" connection failed: fetch failed'), + ); + + const result = await callTool('verify_agent_mcp_server', input); + + expect(result.structuredContent).toEqual({ + ok: false, + error: 'MCP server "srv" connection failed: fetch failed', + }); + expect(result.isError).toBe(true); + }); + it('connects and returns the server tool list', async () => { outboundHttp.transport.mockReturnValue({ asCustomFetch: () => vi.fn() } as never); listMcpServerToolsMock.mockResolvedValue([{ name: 'echo', description: 'Echo' }]); diff --git a/packages/cli/src/modules/mcp/tools/agents/agent-tools.service.ts b/packages/cli/src/modules/mcp/tools/agents/agent-tools.service.ts index fa52a6cae9b..c7051850ed0 100644 --- a/packages/cli/src/modules/mcp/tools/agents/agent-tools.service.ts +++ b/packages/cli/src/modules/mcp/tools/agents/agent-tools.service.ts @@ -24,7 +24,7 @@ import type { User } from '@n8n/db'; import { Service } from '@n8n/di'; import type { Scope } from '@n8n/permissions'; import { isRecord } from '@n8n/utils/is-record'; -import { UserError } from 'n8n-workflow'; +import { OperationalError, UserError } from 'n8n-workflow'; import { z } from 'zod'; import { CredentialsService } from '@/credentials/credentials.service'; @@ -1734,28 +1734,35 @@ export class McpAgentToolsService { await this.requireAccessibleCredential(credentialProvider, input.credential); } - const tools = await listMcpServerTools( - { - name: input.name, - url: input.url, - transport: input.transport, - authentication: input.authentication, - credential: input.credential, - metadata: input.metadata, - ...(input.connectionTimeoutMs !== undefined - ? { connectionTimeoutMs: input.connectionTimeoutMs } - : {}), - }, - { - credentialProvider, - oauthService: this.oauthService, - projectId: input.projectId, - proxyFetch: createAiMcpFetch(this.outboundHttp), - resolveRegistryConnection: async (nodeTypeName) => - await this.mcpRegistryService.getConnection(nodeTypeName), - }, - ); - return { ok: true, tools }; + try { + const tools = await listMcpServerTools( + { + name: input.name, + url: input.url, + transport: input.transport, + authentication: input.authentication, + credential: input.credential, + metadata: input.metadata, + ...(input.connectionTimeoutMs !== undefined + ? { connectionTimeoutMs: input.connectionTimeoutMs } + : {}), + }, + { + credentialProvider, + oauthService: this.oauthService, + projectId: input.projectId, + proxyFetch: createAiMcpFetch(this.outboundHttp), + resolveRegistryConnection: async (nodeTypeName) => + await this.mcpRegistryService.getConnection(nodeTypeName), + }, + ); + return { ok: true, tools }; + } catch (error) { + if (error instanceof OperationalError) { + return { ok: false, error: error.message }; + } + throw error; + } } private async updateIntegration(user: User, input: UpdateIntegrationInput) {