From 8af6414e137b218a147124dd67995a8d33e21cbb Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Thu, 30 Jul 2026 13:22:11 -0400 Subject: [PATCH] =?UTF-8?q?=F0=9F=AA=9F=20fix:=20Surface=20MCP=20Initializ?= =?UTF-8?q?ation=20Errors=20(#14529)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- api/server/routes/__tests__/mcp.spec.js | 31 +++++++++ api/server/routes/mcp.js | 12 +++- api/server/services/Tools/mcp.js | 27 ++++++-- api/server/services/Tools/mcp.spec.js | 11 +++- client/src/hooks/MCP/__tests__/errors.spec.ts | 66 +++++++++++++++++++ client/src/hooks/MCP/errors.ts | 21 ++++++ client/src/hooks/MCP/useMCPServerManager.ts | 3 +- client/src/locales/en/translation.json | 2 + packages/data-provider/src/data-service.ts | 2 +- .../src/react-query/react-query-service.ts | 12 +--- .../data-provider/src/types/mcpServers.ts | 19 ++++++ 11 files changed, 187 insertions(+), 19 deletions(-) create mode 100644 client/src/hooks/MCP/__tests__/errors.spec.ts create mode 100644 client/src/hooks/MCP/errors.ts diff --git a/api/server/routes/__tests__/mcp.spec.js b/api/server/routes/__tests__/mcp.spec.js index 5323fa0d0e..6109346446 100644 --- a/api/server/routes/__tests__/mcp.spec.js +++ b/api/server/routes/__tests__/mcp.spec.js @@ -1885,6 +1885,37 @@ describe('MCP Routes', () => { }); }); + it('should return structured reinitialization failure details', async () => { + const mockMcpManager = { + disconnectUserConnection: jest.fn().mockResolvedValue(), + }; + + mockRegistryInstance.getServerConfig.mockResolvedValue({}); + require('~/config').getMCPManager.mockReturnValue(mockMcpManager); + require('~/server/services/Tools/mcp').reinitMCPServer.mockResolvedValue({ + success: false, + message: "MCP server 'test-server' requires user-provided variables", + serverName: 'test-server', + oauthRequired: false, + oauthUrl: null, + failureReason: 'missing_custom_user_vars', + missingUserVars: ['API_KEY'], + }); + + const response = await request(app).post('/api/mcp/test-server/reinitialize'); + + expect(response.status).toBe(200); + expect(response.body).toEqual({ + success: false, + message: "MCP server 'test-server' requires user-provided variables", + serverName: 'test-server', + oauthRequired: false, + oauthUrl: null, + failureReason: 'missing_custom_user_vars', + missingUserVars: ['API_KEY'], + }); + }); + it('should return 500 when reinitialize fails with non-OAuth error', async () => { const mockMcpManager = { disconnectUserConnection: jest.fn().mockResolvedValue(), diff --git a/api/server/routes/mcp.js b/api/server/routes/mcp.js index cf213821f8..bcd3392c7d 100644 --- a/api/server/routes/mcp.js +++ b/api/server/routes/mcp.js @@ -718,7 +718,15 @@ router.post( return res.status(500).json({ error: 'Failed to reinitialize MCP server for user' }); } - const { success, message, oauthRequired, oauthUrl, connectionDeferred } = result; + const { + success, + message, + oauthRequired, + oauthUrl, + failureReason, + missingUserVars, + connectionDeferred, + } = result; if (oauthRequired) { const flowId = getOAuthFlowId(user.id, serverName); @@ -731,6 +739,8 @@ router.post( oauthUrl, serverName, oauthRequired, + failureReason, + missingUserVars, connectionDeferred, }); } catch (error) { diff --git a/api/server/services/Tools/mcp.js b/api/server/services/Tools/mcp.js index eab23fb7f9..de75c8437c 100644 --- a/api/server/services/Tools/mcp.js +++ b/api/server/services/Tools/mcp.js @@ -13,6 +13,13 @@ const { createOboTrustChecker } = require('~/server/services/OboPolicyService'); const { updateMCPServerTools } = require('~/server/services/Config'); const { getLogStores } = require('~/cache'); +const MCP_REINITIALIZE_FAILURE_REASONS = { + UNREACHABLE: 'unreachable', + MISSING_CUSTOM_USER_VARS: 'missing_custom_user_vars', + OAUTH_REQUIRED: 'oauth_required', + INITIALIZATION_FAILED: 'initialization_failed', +}; + /** * Reinitializes an MCP server connection and discovers available tools. * When OAuth is required, uses discovery mode to list tools without full authentication @@ -72,6 +79,7 @@ async function reinitMCPServer({ availableTools: null, success: false, message: `MCP server '${serverName}' is still unreachable`, + failureReason: MCP_REINITIALIZE_FAILURE_REASONS.UNREACHABLE, oauthRequired: false, serverName, oauthUrl: null, @@ -94,6 +102,7 @@ async function reinitMCPServer({ availableTools: null, success: false, message: `MCP server '${serverName}' is still unreachable`, + failureReason: MCP_REINITIALIZE_FAILURE_REASONS.UNREACHABLE, oauthRequired: false, serverName, oauthUrl: null, @@ -118,6 +127,8 @@ async function reinitMCPServer({ message: `MCP server '${serverName}' requires user-provided variable(s) [${missingUserVars.join( ', ', )}] which are not set`, + failureReason: MCP_REINITIALIZE_FAILURE_REASONS.MISSING_CUSTOM_USER_VARS, + missingUserVars, oauthRequired: false, serverName, oauthUrl: null, @@ -270,14 +281,20 @@ async function reinitMCPServer({ return `Failed to reinitialize MCP server '${serverName}'`; }; + const success = Boolean( + (connection && !oauthRequired) || (oauthRequired && oauthUrl) || (tools && tools.length > 0), + ); + let failureReason; + if (!success) { + failureReason = oauthRequired + ? MCP_REINITIALIZE_FAILURE_REASONS.OAUTH_REQUIRED + : MCP_REINITIALIZE_FAILURE_REASONS.INITIALIZATION_FAILED; + } const result = { availableTools, - success: Boolean( - (connection && !oauthRequired) || - (oauthRequired && oauthUrl) || - (tools && tools.length > 0), - ), + success, message: getResponseMessage(), + failureReason, oauthRequired, serverName, oauthUrl, diff --git a/api/server/services/Tools/mcp.spec.js b/api/server/services/Tools/mcp.spec.js index 730d9cffb8..8e69ba9a78 100644 --- a/api/server/services/Tools/mcp.spec.js +++ b/api/server/services/Tools/mcp.spec.js @@ -60,6 +60,8 @@ describe('reinitMCPServer — customUserVars gating (issue #10969)', () => { availableTools: null, success: false, tools: null, + failureReason: 'missing_custom_user_vars', + missingUserVars: ['THINGY_TOKEN'], oauthRequired: false, serverName, }); @@ -125,7 +127,7 @@ describe('reinitMCPServer — customUserVars gating (issue #10969)', () => { mockDiscoverServerTools.mockResolvedValue({ tools: [], oauthRequired: true, oauthUrl: null }); const requestBody = { conversationId: 'conv-456', messageId: 'msg-456' }; - await reinitMCPServer({ + const result = await reinitMCPServer({ user, serverName, serverConfig: { type: 'streamable-http', url: 'https://thingy.example.com/mcp' }, @@ -133,6 +135,12 @@ describe('reinitMCPServer — customUserVars gating (issue #10969)', () => { userMCPAuthMap: undefined, }); + expect(result).toMatchObject({ + success: false, + failureReason: 'oauth_required', + oauthRequired: true, + oauthUrl: null, + }); expect(mockDiscoverServerTools).toHaveBeenCalledWith( expect.objectContaining({ requestBody, @@ -280,6 +288,7 @@ describe('reinitMCPServer — runtime BODY placeholder pre-check (issue #14074)' expect(mockDiscoverServerTools).not.toHaveBeenCalled(); expect(result.success).toBe(false); + expect(result.failureReason).toBe('initialization_failed'); expect(result.message).toBe(`Failed to reinitialize MCP server '${serverName}'`); }); }); diff --git a/client/src/hooks/MCP/__tests__/errors.spec.ts b/client/src/hooks/MCP/__tests__/errors.spec.ts new file mode 100644 index 0000000000..282dc9fba6 --- /dev/null +++ b/client/src/hooks/MCP/__tests__/errors.spec.ts @@ -0,0 +1,66 @@ +import type { MCPReinitializeResponse } from 'librechat-data-provider'; +import type { LocalizeFunction } from '~/common'; +import { getMCPReinitializeErrorMessage } from '../errors'; + +const localize = jest.fn((key: string) => key) as unknown as jest.MockedFunction; + +const createResponse = ( + overrides: Partial = {}, +): MCPReinitializeResponse => ({ + success: false, + message: 'Raw backend message that must not be displayed', + serverName: 'ClickHouse', + ...overrides, +}); + +describe('getMCPReinitializeErrorMessage', () => { + beforeEach(() => { + localize.mockClear(); + }); + + it('localizes unreachable servers with the existing connection guidance', () => { + const message = getMCPReinitializeErrorMessage( + createResponse({ failureReason: 'unreachable' }), + localize, + ); + + expect(message).toBe('com_ui_mcp_server_connection_failed'); + expect(localize).toHaveBeenCalledWith('com_ui_mcp_server_connection_failed'); + }); + + it('localizes missing variables with the server and variable names', () => { + const message = getMCPReinitializeErrorMessage( + createResponse({ + failureReason: 'missing_custom_user_vars', + missingUserVars: ['API_KEY', 'ACCOUNT_ID'], + }), + localize, + ); + + expect(message).toBe('com_ui_mcp_missing_custom_user_vars'); + expect(localize).toHaveBeenCalledWith('com_ui_mcp_missing_custom_user_vars', { + 0: 'ClickHouse', + 1: 'API_KEY, ACCOUNT_ID', + }); + }); + + it('localizes an OAuth failure that requires reauthentication', () => { + const message = getMCPReinitializeErrorMessage( + createResponse({ failureReason: 'oauth_required' }), + localize, + ); + + expect(message).toBe('com_ui_mcp_reauthentication_required'); + expect(localize).toHaveBeenCalledWith('com_ui_mcp_reauthentication_required', { + 0: 'ClickHouse', + }); + }); + + it('uses the localized fallback instead of exposing unknown backend messages', () => { + const message = getMCPReinitializeErrorMessage(createResponse(), localize); + + expect(message).toBe('com_ui_mcp_init_failed'); + expect(message).not.toContain('Raw backend message'); + expect(localize).toHaveBeenCalledWith('com_ui_mcp_init_failed'); + }); +}); diff --git a/client/src/hooks/MCP/errors.ts b/client/src/hooks/MCP/errors.ts new file mode 100644 index 0000000000..f284e39cb5 --- /dev/null +++ b/client/src/hooks/MCP/errors.ts @@ -0,0 +1,21 @@ +import type { MCPReinitializeResponse } from 'librechat-data-provider'; +import type { LocalizeFunction } from '~/common'; + +export function getMCPReinitializeErrorMessage( + response: MCPReinitializeResponse, + localize: LocalizeFunction, +): string { + switch (response.failureReason) { + case 'unreachable': + return localize('com_ui_mcp_server_connection_failed'); + case 'missing_custom_user_vars': + return localize('com_ui_mcp_missing_custom_user_vars', { + 0: response.serverName, + 1: response.missingUserVars?.join(', ') ?? '', + }); + case 'oauth_required': + return localize('com_ui_mcp_reauthentication_required', { 0: response.serverName }); + default: + return localize('com_ui_mcp_init_failed'); + } +} diff --git a/client/src/hooks/MCP/useMCPServerManager.ts b/client/src/hooks/MCP/useMCPServerManager.ts index c5bc7ba022..ead897c439 100644 --- a/client/src/hooks/MCP/useMCPServerManager.ts +++ b/client/src/hooks/MCP/useMCPServerManager.ts @@ -27,6 +27,7 @@ import type { ConfigFieldDetail } from '~/common'; import { useLocalize, useHasAccess, useMCPSelect, useMCPConnectionStatus } from '~/hooks'; import { useGetStartupConfig, useMCPServersQuery } from '~/data-provider'; import { mcpServerInitStatesAtom, getServerInitState } from '~/store/mcp'; +import { getMCPReinitializeErrorMessage } from './errors'; export interface MCPServerDefinition { serverName: string; @@ -358,7 +359,7 @@ export function useMCPServerManager({ }); if (!response.success) { showToast({ - message: localize('com_ui_mcp_init_failed', { 0: serverName }), + message: getMCPReinitializeErrorMessage(response, localize), status: 'error', }); cleanupServerState(serverName); diff --git a/client/src/locales/en/translation.json b/client/src/locales/en/translation.json index 5ca95632c6..d0bc71ab91 100644 --- a/client/src/locales/en/translation.json +++ b/client/src/locales/en/translation.json @@ -1351,6 +1351,7 @@ "com_ui_mcp_initialize": "Initialize", "com_ui_mcp_initialized_success": "MCP server '{{0}}' initialized successfully", "com_ui_mcp_invalid_url": "Please enter a valid URL", + "com_ui_mcp_missing_custom_user_vars": "MCP server '{{0}}' requires variables [{{1}}] which are not set", "com_ui_mcp_no_description": "No description available", "com_ui_mcp_oauth_cancelled": "OAuth login cancelled for {{0}}", "com_ui_mcp_oauth_description": "Continue to authenticate, or copy the link to open it on another device.", @@ -1359,6 +1360,7 @@ "com_ui_mcp_oauth_timeout": "OAuth login timed out for {{0}}", "com_ui_mcp_programmatic": "Programmatic", "com_ui_mcp_programmatic_all": "Mark all as programmatic", + "com_ui_mcp_reauthentication_required": "MCP server '{{0}}' needs authentication. Reconnect to continue; if that fails, revoke its OAuth access and try again.", "com_ui_mcp_server": "MCP Server", "com_ui_mcp_server_connection_failed": "Connection attempt to the provided MCP server failed. Please make sure the URL, the server type, and any authentication configuration are correct, then try again. Also ensure the URL is reachable.", "com_ui_mcp_server_created": "MCP server created successfully", diff --git a/packages/data-provider/src/data-service.ts b/packages/data-provider/src/data-service.ts index d1ab0c0388..4db596a867 100644 --- a/packages/data-provider/src/data-service.ts +++ b/packages/data-provider/src/data-service.ts @@ -221,7 +221,7 @@ export const updateUserPlugins = (payload: t.TUpdateUserPlugins) => { return request.post(endpoints.userPlugins(), payload); }; -export const reinitializeMCPServer = (serverName: string) => { +export const reinitializeMCPServer = (serverName: string): Promise => { return request.post(endpoints.mcpReinitialize(serverName)); }; diff --git a/packages/data-provider/src/react-query/react-query-service.ts b/packages/data-provider/src/react-query/react-query-service.ts index cd85462ecb..4c74903261 100644 --- a/packages/data-provider/src/react-query/react-query-service.ts +++ b/packages/data-provider/src/react-query/react-query-service.ts @@ -4,6 +4,7 @@ import type { UseMutationResult, QueryObserverResult, } from '@tanstack/react-query'; +import type { MCPReinitializeResponse } from '../types/mcpServers'; import { MCPServerConnectionStatusResponse } from '../types/queries'; import { Constants, initialModelsConfig } from '../config'; import { defaultOrderQuery } from '../types/assistants'; @@ -334,16 +335,7 @@ export const useUpdateUserPluginsMutation = ( }; export const useReinitializeMCPServerMutation = (): UseMutationResult< - { - success: boolean; - message: string; - serverName: string; - oauthRequired?: boolean; - oauthUrl?: string; - /** True when the server uses request-scoped placeholders and the connection - * was deferred to the next chat turn (tools are not enumerable up front). */ - connectionDeferred?: boolean; - }, + MCPReinitializeResponse, unknown, string, unknown diff --git a/packages/data-provider/src/types/mcpServers.ts b/packages/data-provider/src/types/mcpServers.ts index 27520a11d5..5aa45ab242 100644 --- a/packages/data-provider/src/types/mcpServers.ts +++ b/packages/data-provider/src/types/mcpServers.ts @@ -47,3 +47,22 @@ export type MCPServerDBObjectResponse = { } & MCPOptions; export type MCPServersListResponse = Record; + +export type MCPReinitializeFailureReason = + | 'unreachable' + | 'missing_custom_user_vars' + | 'oauth_required' + | 'initialization_failed'; + +export interface MCPReinitializeResponse { + success: boolean; + message: string; + serverName: string; + oauthRequired?: boolean; + oauthUrl?: string | null; + failureReason?: MCPReinitializeFailureReason; + missingUserVars?: string[]; + /** True when the server uses request-scoped placeholders and the connection + * was deferred to the next chat turn (tools are not enumerable up front). */ + connectionDeferred?: boolean; +}