diff --git a/packages/cli/src/modules/instance-ai/mcp/__tests__/instance-ai-mcp-connection.controller.test.ts b/packages/cli/src/modules/instance-ai/mcp/__tests__/instance-ai-mcp-connection.controller.test.ts index a7a6a9e5076..916db407afa 100644 --- a/packages/cli/src/modules/instance-ai/mcp/__tests__/instance-ai-mcp-connection.controller.test.ts +++ b/packages/cli/src/modules/instance-ai/mcp/__tests__/instance-ai-mcp-connection.controller.test.ts @@ -168,6 +168,22 @@ describe('InstanceAiMcpConnectionController', () => { }); }); + describe('listAllTools', () => { + it('returns tool statuses for the authenticated user’s connections', async () => { + const { controller, service } = createController(); + const tools = [ + { id: 'conn-1', status: 'connected' as const, tools: [{ name: 'search' }] }, + { id: 'conn-2', status: 'disconnected' as const, tools: [] }, + ]; + service.listAllConnectionTools.mockResolvedValue(tools); + + const result = await controller.listAllTools(authedRequest()); + + expect(service.listAllConnectionTools).toHaveBeenCalledWith(user); + expect(result).toEqual(tools); + }); + }); + describe('update', () => { it('delegates update to service and returns enriched response', async () => { const { controller, service, credentialsFinderService, mcpRegistryService } = diff --git a/packages/cli/src/modules/instance-ai/mcp/__tests__/instance-ai-mcp-registry.service.test.ts b/packages/cli/src/modules/instance-ai/mcp/__tests__/instance-ai-mcp-registry.service.test.ts index e5b2e1249e0..433388d37e2 100644 --- a/packages/cli/src/modules/instance-ai/mcp/__tests__/instance-ai-mcp-registry.service.test.ts +++ b/packages/cli/src/modules/instance-ai/mcp/__tests__/instance-ai-mcp-registry.service.test.ts @@ -24,9 +24,15 @@ import type { InstanceAiMcpRegistryConnection } from '../../entities/instance-ai import type { InstanceAiMcpRegistryConnectionRepository } from '../../repositories/instance-ai-mcp-registry-connection.repository'; import { InstanceAiMcpRegistryService } from '../instance-ai-mcp-registry.service'; -const { mcpClientCloseMock, mcpClientConstructorMock, mcpClientListToolsMock } = vi.hoisted(() => ({ +const { + mcpClientCloseMock, + mcpClientConstructorMock, + mcpClientGetConnectionFailuresMock, + mcpClientListToolsMock, +} = vi.hoisted(() => ({ mcpClientCloseMock: vi.fn<() => Promise>(), mcpClientConstructorMock: vi.fn<(configs: unknown) => void>(), + mcpClientGetConnectionFailuresMock: vi.fn(), mcpClientListToolsMock: vi.fn<() => Promise>(), })); @@ -35,6 +41,7 @@ vi.mock('@n8n/agents', () => ({ mcpClientConstructorMock(configs); return { close: mcpClientCloseMock, + getConnectionFailures: mcpClientGetConnectionFailuresMock, listTools: mcpClientListToolsMock, }; }), @@ -131,6 +138,8 @@ describe('InstanceAiMcpRegistryService', () => { mcpClientCloseMock.mockReset(); mcpClientCloseMock.mockResolvedValue(undefined); mcpClientConstructorMock.mockReset(); + mcpClientGetConnectionFailuresMock.mockReset(); + mcpClientGetConnectionFailuresMock.mockReturnValue([]); mcpClientListToolsMock.mockReset(); mcpClientListToolsMock.mockResolvedValue([]); proxyFetchMock.mockReset(); @@ -488,7 +497,7 @@ describe('InstanceAiMcpRegistryService', () => { }); }); - describe('listConnectionTools', () => { + describe('connection tools', () => { function makeConnection( overrides: Partial = {}, ): InstanceAiMcpRegistryConnection { @@ -553,11 +562,15 @@ describe('InstanceAiMcpRegistryService', () => { connectionTimeoutMs: 10_000, }), ]); - expect(result).toEqual([ - { name: 'search', description: 'Search Linear issues' }, - { name: 'create_issue', description: 'Create a Linear issue' }, - { name: 'no_description' }, - ]); + expect(result).toEqual({ + id: 'conn-1', + status: 'connected', + tools: [ + { name: 'search', description: 'Search Linear issues' }, + { name: 'create_issue', description: 'Create a Linear issue' }, + { name: 'no_description' }, + ], + }); expect(mcpClientCloseMock).toHaveBeenCalledTimes(1); }); @@ -577,7 +590,20 @@ describe('InstanceAiMcpRegistryService', () => { const result = await service.listConnectionTools(user, 'conn-1'); - expect(result).toEqual([{ name: 'read file', description: 'Read a file' }]); + expect(result).toEqual({ + id: 'conn-1', + status: 'connected', + tools: [{ name: 'read file', description: 'Read a file' }], + }); + }); + + it('treats a successful empty tool list as connected', async () => { + const deps = createService(); + arrangeResolvableConnection(deps); + + const result = await deps.service.listConnectionTools(user, 'conn-1'); + + expect(result).toEqual({ id: 'conn-1', status: 'connected', tools: [] }); }); it('throws NotFoundError when the connection does not belong to the user', async () => { @@ -609,7 +635,7 @@ describe('InstanceAiMcpRegistryService', () => { expect(mcpClientConstructorMock).not.toHaveBeenCalled(); }); - it('returns an empty list when the server has no supported remote transport', async () => { + it('returns disconnected when the server has no supported remote transport', async () => { const { service, connectionRepository, @@ -622,7 +648,7 @@ describe('InstanceAiMcpRegistryService', () => { const result = await service.listConnectionTools(user, 'conn-1'); - expect(result).toEqual([]); + expect(result).toEqual({ id: 'conn-1', status: 'disconnected', tools: [] }); expect(credentialsFinderService.findCredentialForUser).not.toHaveBeenCalled(); expect(mcpClientConstructorMock).not.toHaveBeenCalled(); expect(logger.warn).toHaveBeenCalledWith( @@ -631,7 +657,7 @@ describe('InstanceAiMcpRegistryService', () => { ); }); - it('returns an empty list when OAuth credential data is unavailable', async () => { + it('returns disconnected when OAuth credential data is unavailable', async () => { const { service, connectionRepository, @@ -645,7 +671,7 @@ describe('InstanceAiMcpRegistryService', () => { const result = await service.listConnectionTools(user, 'conn-1'); - expect(result).toEqual([]); + expect(result).toEqual({ id: 'conn-1', status: 'disconnected', tools: [] }); expect(mcpClientConstructorMock).not.toHaveBeenCalled(); expect(logger.warn).toHaveBeenCalledWith( 'Skipping MCP registry connection with inaccessible credential', @@ -658,6 +684,45 @@ describe('InstanceAiMcpRegistryService', () => { ); }); + it('returns disconnected when the MCP client records a connection failure', async () => { + const deps = createService(); + arrangeResolvableConnection(deps); + mcpClientGetConnectionFailuresMock.mockReturnValue([ + { server: 'mcp_linear', error: 'Connection timed out' }, + ]); + + const result = await deps.service.listConnectionTools(user, 'conn-1'); + + expect(result).toEqual({ id: 'conn-1', status: 'disconnected', tools: [] }); + }); + + it('returns statuses for all connections when one check throws', async () => { + const deps = createService(); + const { connectionRepository, credentialsFinderService, credentialsService, logger } = deps; + connectionRepository.findBy.mockResolvedValue([ + makeConnection(), + makeConnection({ id: 'conn-2', serverSlug: 'notion' }), + ]); + deps.mcpRegistryService.get.mockImplementation(async (slug) => { + if (slug === 'notion') throw new Error('Registry unavailable'); + return makeRegistryServer(slug); + }); + credentialsFinderService.findCredentialForUser.mockResolvedValue(credential); + credentialsService.decrypt.mockResolvedValue(oauthCredentialData); + + const result = await deps.service.listAllConnectionTools(user); + + expect(result).toEqual([ + { id: 'conn-1', status: 'connected', tools: [] }, + { id: 'conn-2', status: 'disconnected', tools: [] }, + ]); + expect(logger.warn).toHaveBeenCalledWith('Failed to check MCP connection', { + connectionId: 'conn-2', + serverSlug: 'notion', + error: expect.any(Error), + }); + }); + it('closes the MCP client when listing tools fails', async () => { const deps = createService(); const { service } = deps; diff --git a/packages/frontend/editor-ui/src/features/ai/instanceAi/__tests__/instanceAiMcp.store.test.ts b/packages/frontend/editor-ui/src/features/ai/instanceAi/__tests__/instanceAiMcp.store.test.ts index 40b1c787492..f8157b9eccf 100644 --- a/packages/frontend/editor-ui/src/features/ai/instanceAi/__tests__/instanceAiMcp.store.test.ts +++ b/packages/frontend/editor-ui/src/features/ai/instanceAi/__tests__/instanceAiMcp.store.test.ts @@ -1,6 +1,10 @@ import { setActivePinia, createPinia } from 'pinia'; import { beforeEach, describe, expect, it, vi } from 'vitest'; -import type { InstanceAiMcpConnectionResponse, McpRegistryServerResponse } from '@n8n/api-types'; +import type { + InstanceAiMcpConnectionResponse, + InstanceAiMcpConnectionToolsResponse, + McpRegistryServerResponse, +} from '@n8n/api-types'; vi.mock('@n8n/stores/useRootStore', () => ({ useRootStore: vi.fn().mockReturnValue({ @@ -42,11 +46,13 @@ const mockFetchMcpConnections = vi.fn(); const mockCreateMcpConnection = vi.fn(); const mockUpdateMcpConnection = vi.fn(); const mockDeleteMcpConnection = vi.fn(); +const mockFetchAllMcpConnectionTools = vi.fn(); const mockFetchMcpConnectionTools = vi.fn(); vi.mock('../instanceAi.mcp.api', () => ({ fetchMcpRegistryServers: (...args: unknown[]) => mockFetchMcpRegistryServers(...args), fetchMcpConnections: (...args: unknown[]) => mockFetchMcpConnections(...args), + fetchAllMcpConnectionTools: (...args: unknown[]) => mockFetchAllMcpConnectionTools(...args), fetchMcpConnectionTools: (...args: unknown[]) => mockFetchMcpConnectionTools(...args), createMcpConnection: (...args: unknown[]) => mockCreateMcpConnection(...args), updateMcpConnection: (...args: unknown[]) => mockUpdateMcpConnection(...args), @@ -98,19 +104,72 @@ describe('useInstanceAiMcpStore', () => { beforeEach(() => { vi.clearAllMocks(); + mockFetchMcpRegistryServers.mockReset(); + mockFetchMcpConnections.mockReset(); + mockFetchAllMcpConnectionTools.mockReset(); + mockFetchAllMcpConnectionTools.mockResolvedValue([]); + mockFetchMcpConnectionTools.mockReset(); + mockFetchMcpConnectionTools.mockResolvedValue({ + id: 'conn-1', + status: 'connected', + tools: [], + } satisfies InstanceAiMcpConnectionToolsResponse); + mockCreateMcpConnection.mockReset(); + mockUpdateMcpConnection.mockReset(); + mockDeleteMcpConnection.mockReset(); deletionListeners.clear(); setActivePinia(createPinia()); store = useInstanceAiMcpStore(); }); describe('fetchConnections', () => { - it('loads connections into state', async () => { + it('marks connections as connecting until bulk tool loading completes', async () => { + const bulkRequest = createDeferred(); mockFetchMcpConnections.mockResolvedValue([makeConnection()]); + mockFetchAllMcpConnectionTools.mockReturnValue(bulkRequest.promise); await store.fetchConnections(); - expect(store.connections).toHaveLength(1); - expect(store.connections[0].id).toBe('conn-1'); + expect(store.connections[0]).toMatchObject({ id: 'conn-1', status: 'connecting' }); + + bulkRequest.resolve([{ id: 'conn-1', status: 'connected', tools: [] }]); + await vi.waitFor(() => expect(store.connections[0].status).toBe('connected')); + }); + + it('applies bulk statuses and caches tools for connected servers', async () => { + mockFetchMcpConnections.mockResolvedValue([ + makeConnection({ id: 'conn-1' }), + makeConnection({ id: 'conn-2', serverSlug: 'notion' }), + ]); + mockFetchAllMcpConnectionTools.mockResolvedValue([ + { id: 'conn-1', status: 'connected', tools: [{ name: 'search' }] }, + { id: 'conn-2', status: 'disconnected', tools: [] }, + ] satisfies InstanceAiMcpConnectionToolsResponse[]); + + await store.fetchConnections(); + + await vi.waitFor(() => { + expect(store.connections.map(({ id, status }) => ({ id, status }))).toEqual([ + { id: 'conn-1', status: 'connected' }, + { id: 'conn-2', status: 'disconnected' }, + ]); + }); + expect(store.connectionToolsById.get('conn-1')).toEqual([{ name: 'search' }]); + expect(store.connectionToolsById.has('conn-2')).toBe(false); + }); + + it('loads connections and tools only once', async () => { + const connectionsRequest = createDeferred(); + mockFetchMcpConnections.mockReturnValue(connectionsRequest.promise); + + const firstFetch = store.fetchConnections(); + await store.fetchConnections(); + connectionsRequest.resolve([makeConnection()]); + await firstFetch; + await store.fetchConnections(); + + expect(mockFetchMcpConnections).toHaveBeenCalledTimes(1); + expect(mockFetchAllMcpConnectionTools).toHaveBeenCalledTimes(1); }); it('surfaces errors via toast', async () => { @@ -137,11 +196,40 @@ describe('useInstanceAiMcpStore', () => { }); describe('fetchConnectionToolsLazy', () => { + it('retries a disconnected connection and caches its tools', async () => { + mockFetchMcpConnections.mockResolvedValue([makeConnection()]); + mockFetchAllMcpConnectionTools.mockResolvedValue([ + { id: 'conn-1', status: 'disconnected', tools: [] }, + ]); + await store.fetchConnections(); + await vi.waitFor(() => expect(store.connections[0].status).toBe('disconnected')); + + const toolsRequest = createDeferred(); + mockFetchMcpConnectionTools.mockReturnValue(toolsRequest.promise); + const retry = store.fetchConnectionToolsLazy('conn-1'); + + expect(store.connections[0].status).toBe('connecting'); + toolsRequest.resolve({ + id: 'conn-1', + status: 'connected', + tools: [{ name: 'search' }], + }); + await retry; + + expect(store.connections[0].status).toBe('connected'); + expect(store.connectionToolsById.get('conn-1')).toEqual([{ name: 'search' }]); + }); + it('refreshes tools after credential changes and ignores the stale response', async () => { - const staleTools = [{ name: 'old_tool' }]; - const freshTools = [{ name: 'fresh_tool' }]; - const staleToolsRequest = createDeferred(); - const freshToolsRequest = createDeferred(); + mockFetchMcpConnections.mockResolvedValue([makeConnection()]); + mockFetchAllMcpConnectionTools.mockResolvedValue([ + { id: 'conn-1', status: 'disconnected', tools: [] }, + ]); + await store.fetchConnections(); + await vi.waitFor(() => expect(store.connections[0].status).toBe('disconnected')); + + const staleToolsRequest = createDeferred(); + const freshToolsRequest = createDeferred(); mockFetchMcpConnectionTools .mockReturnValueOnce(staleToolsRequest.promise) .mockReturnValueOnce(freshToolsRequest.promise); @@ -155,13 +243,21 @@ describe('useInstanceAiMcpStore', () => { await store.updateConnection('conn-1', { credentialId: 'cred-2' }); expect(mockFetchMcpConnectionTools).toHaveBeenCalledTimes(2); - staleToolsRequest.resolve(staleTools); + staleToolsRequest.resolve({ + id: 'conn-1', + status: 'connected', + tools: [{ name: 'old_tool' }], + }); await staleFetch; expect(store.connectionToolsById.get('conn-1')).toBeUndefined(); - freshToolsRequest.resolve(freshTools); + freshToolsRequest.resolve({ + id: 'conn-1', + status: 'connected', + tools: [{ name: 'fresh_tool' }], + }); await vi.waitFor(() => { - expect(store.connectionToolsById.get('conn-1')).toEqual(freshTools); + expect(store.connectionToolsById.get('conn-1')).toEqual([{ name: 'fresh_tool' }]); }); }); }); @@ -169,12 +265,16 @@ describe('useInstanceAiMcpStore', () => { describe('connect', () => { it('appends the new connection on success', async () => { const created = makeConnection({ id: 'conn-new' }); + const toolsRequest = createDeferred(); mockCreateMcpConnection.mockResolvedValue(created); + mockFetchMcpConnectionTools.mockReturnValue(toolsRequest.promise); const result = await store.connect({ serverSlug: 'linear', credentialId: 'cred-1' }); - expect(result).toEqual(created); - expect(store.connections).toContainEqual(created); + expect(result).toEqual({ ...created, status: 'connecting' }); + expect(store.connections).toContainEqual({ ...created, status: 'connecting' }); + toolsRequest.resolve({ id: 'conn-new', status: 'connected', tools: [] }); + await vi.waitFor(() => expect(store.connections[0].status).toBe('connected')); }); it('returns null and reports an error on failure', async () => { @@ -198,7 +298,7 @@ describe('useInstanceAiMcpStore', () => { const result = await store.updateConnection('conn-1', { inclusionMode: 'except' }); - expect(result).toEqual(updated); + expect(result).toEqual({ ...updated, status: 'connecting' }); expect(store.connections[0].credentialName).toBe('Renamed'); }); }); diff --git a/packages/frontend/editor-ui/src/features/ai/instanceAi/components/__tests__/ConnectionsCard.test.ts b/packages/frontend/editor-ui/src/features/ai/instanceAi/components/__tests__/ConnectionsCard.test.ts index bb1934437d4..6e36f457a39 100644 --- a/packages/frontend/editor-ui/src/features/ai/instanceAi/components/__tests__/ConnectionsCard.test.ts +++ b/packages/frontend/editor-ui/src/features/ai/instanceAi/components/__tests__/ConnectionsCard.test.ts @@ -109,9 +109,13 @@ const renderComponent = createComponentRenderer(ConnectionsCard, { global: { stubs: { ConnectionRow: { - props: ['name', 'actions'], + props: ['name', 'actions', 'status'], template: ` -
+
{{ name }}