diff --git a/api/server/controllers/__tests__/mcp.servers.spec.js b/api/server/controllers/__tests__/mcp.servers.spec.js index c46cc07615..7ff8de938d 100644 --- a/api/server/controllers/__tests__/mcp.servers.spec.js +++ b/api/server/controllers/__tests__/mcp.servers.spec.js @@ -183,6 +183,24 @@ describe('getMCPServersList', () => { expect(res.json).toHaveBeenCalledWith({}); }); + it('exposes safe request-scoped metadata while redacting placeholder-bearing fields', async () => { + const reqUser = await createUser(); + mockResolveAllMcpConfigs.mockResolvedValue({ + runtimeServer: { + ...yamlConfig, + headers: { 'X-Conversation': '{{LIBRECHAT_BODY_CONVERSATIONID}}' }, + }, + }); + + const res = createRes(); + await getMCPServersList({ user: reqUser }, res); + + const payload = res.json.mock.calls[0][0]; + expect(payload.runtimeServer.requestScoped).toBe(true); + expect(payload.runtimeServer.url).toBeUndefined(); + expect(payload.runtimeServer.headers).toBeUndefined(); + }); + it('applies the capability bypass to all servers when a DB-backed server is present', async () => { await seedManageMcpGrant(); const reqUser = await createUser(SystemRoles.ADMIN); diff --git a/api/server/routes/__tests__/mcp.spec.js b/api/server/routes/__tests__/mcp.spec.js index 89192c3d66..44dcfe2628 100644 --- a/api/server/routes/__tests__/mcp.spec.js +++ b/api/server/routes/__tests__/mcp.spec.js @@ -936,7 +936,7 @@ describe('MCP Routes', () => { expect(response.headers.location).toContain(`${basePath}/oauth/success`); }); - it('should forward the merged server config so the tool cache gate sees request-scoped servers', async () => { + it('should use the merged server config to defer request-scoped post-OAuth reconnect', async () => { const flowId = 'test-user-id:test-server'; const mockFlowManager = { getFlowState: jest.fn().mockResolvedValue({ @@ -959,8 +959,6 @@ describe('MCP Routes', () => { url: 'https://override.example.com/{{LIBRECHAT_BODY_CONVERSATIONID}}/mcp', source: 'config', }; - const fetchedTools = [{ name: 'search', inputSchema: { type: 'object' } }]; - getLogStores.mockReturnValue({}); require('~/config').getFlowStateManager.mockReturnValue(mockFlowManager); MCPOAuthHandler.getFlowState.mockResolvedValue(mockFlowState); @@ -973,12 +971,15 @@ describe('MCP Routes', () => { const fetchOrderedToolsSnapshot = jest .fn() - .mockResolvedValue({ tools: fetchedTools, complete: true }); + .mockResolvedValue({ tools: [{ name: 'search' }], complete: true }); const mockMcpManager = createLeasedMcpManager({ fetchOrderedToolsSnapshot }); require('~/config').getMCPManager.mockReturnValue(mockMcpManager); - require('~/config').getOAuthReconnectionManager.mockReturnValue({ + const mockOAuthReconnectionManager = { clearReconnection: jest.fn(), - }); + }; + require('~/config').getOAuthReconnectionManager.mockReturnValue( + mockOAuthReconnectionManager, + ); const { updateMCPServerTools } = require('~/server/services/Config/mcp'); updateMCPServerTools.mockResolvedValue(); @@ -988,17 +989,13 @@ describe('MCP Routes', () => { expect(response.status).toBe(302); expect(mockResolveAllMcpConfigs).toHaveBeenCalledWith('test-user-id'); - expect(fetchOrderedToolsSnapshot).toHaveBeenCalledTimes(1); - expect(mockMcpManager.withUserConnectionLease).toHaveBeenCalledWith( - expect.objectContaining({ serverConfig: mergedServerConfig }), - expect.any(Function), + expect(fetchOrderedToolsSnapshot).not.toHaveBeenCalled(); + expect(mockMcpManager.withUserConnectionLease).not.toHaveBeenCalled(); + expect(mockOAuthReconnectionManager.clearReconnection).toHaveBeenCalledWith( + 'test-user-id', + 'test-server', ); - expect(updateMCPServerTools).toHaveBeenCalledWith({ - userId: 'test-user-id', - serverName: 'test-server', - tools: fetchedTools, - serverConfig: mergedServerConfig, - }); + expect(updateMCPServerTools).not.toHaveBeenCalled(); }); it('should resolve and forward customUserVars so header templates are substituted on first post-callback connection', async () => { @@ -1659,6 +1656,81 @@ describe('MCP Routes', () => { expect(mockFlowManager.deleteFlow).toHaveBeenCalledWith(flowId, 'mcp_get_tokens'); }); + it('defers request-scoped post-OAuth reconnect while completing the waiting tool flow', async () => { + const { logger } = require('@librechat/data-schemas'); + const { updateMCPServerTools } = require('~/server/services/Config/mcp'); + const mockFlowManager = { + getFlowState: jest.fn().mockResolvedValue({ status: 'PENDING', createdAt: Date.now() }), + completeFlow: jest.fn().mockResolvedValue(true), + deleteFlow: jest.fn().mockResolvedValue(true), + }; + const mockFlowState = { + state: 'test-user-id:test-server', + serverName: 'test-server', + userId: 'test-user-id', + metadata: { toolFlowId: 'tool-flow-request-scoped' }, + clientInfo: {}, + codeVerifier: 'test-verifier', + }; + const mockTokens = { + access_token: 'test-access-token', + refresh_token: 'test-refresh-token', + }; + const mockMcpManager = { + withUserConnectionLease: jest.fn(), + }; + const mockOAuthReconnectionManager = { + clearReconnection: jest.fn(), + }; + + MCPOAuthHandler.getFlowState.mockResolvedValue(mockFlowState); + mockOAuthCompletion(mockTokens); + MCPTokenStorage.storeTokens.mockResolvedValue(mockTokens); + mockResolveAllMcpConfigs.mockResolvedValue({ + 'test-server': { + type: 'streamable-http', + url: 'https://mcp.example.com/mcp', + source: 'yaml', + headers: { + 'X-Conversation-ID': '{{LIBRECHAT_BODY_CONVERSATIONID}}', + }, + requiresOAuth: true, + }, + }); + getLogStores.mockReturnValue({}); + require('~/config').getFlowStateManager.mockReturnValue(mockFlowManager); + require('~/config').getMCPManager.mockReturnValue(mockMcpManager); + require('~/config').getOAuthReconnectionManager.mockReturnValue(mockOAuthReconnectionManager); + + const flowId = 'test-user-id:test-server'; + const csrfToken = generateTestCsrfToken(flowId); + const response = await request(app) + .get('/api/mcp/test-server/oauth/callback') + .set('Cookie', [`oauth_csrf=${csrfToken}`]) + .query({ code: 'test-auth-code', state: flowId }); + + expect(response.status).toBe(302); + expect(response.headers.location).toBe( + `${getBasePath()}/oauth/success?serverName=test-server`, + ); + expect(MCPTokenStorage.storeTokens).toHaveBeenCalled(); + expect(mockMcpManager.withUserConnectionLease).not.toHaveBeenCalled(); + expect(updateMCPServerTools).not.toHaveBeenCalled(); + expect(mockOAuthReconnectionManager.clearReconnection).toHaveBeenCalledWith( + 'test-user-id', + 'test-server', + ); + expect(mockFlowManager.completeFlow).toHaveBeenCalledWith( + 'tool-flow-request-scoped', + 'mcp_oauth', + mockTokens, + ); + expect(logger.warn).not.toHaveBeenCalledWith( + expect.stringContaining('Failed to reconnect test-server after OAuth'), + expect.anything(), + ); + }); + it('should redirect to error page if token storage fails', async () => { // mockRegistryInstance is defined at the top of the file const mockFlowManager = { diff --git a/api/server/routes/mcp.js b/api/server/routes/mcp.js index 99174ce59d..554d110405 100644 --- a/api/server/routes/mcp.js +++ b/api/server/routes/mcp.js @@ -23,6 +23,7 @@ const { OAUTH_SESSION_COOKIE, mcpConfig: mcpSettings, getServerCustomUserVars, + requiresEphemeralUserConnection, } = require('@librechat/api'); const { createMCPServerController, @@ -476,9 +477,6 @@ router.get('/:serverName/oauth/callback', async (req, res) => { logger.info('[MCP OAuth] OAuth flow completed, tokens received in callback route'); try { - const mcpManager = getMCPManager(flowState.userId); - logger.debug(`[MCP OAuth] Attempting to reconnect ${serverName} with new OAuth tokens`); - if (flowState.userId !== 'system') { const user = { id: flowState.userId }; @@ -496,77 +494,90 @@ router.get('/:serverName/oauth/callback', async (req, res) => { ); } - /** - * Without this, getUserConnection resolves `headers`/`oauth_headers` - * customUserVars templates (e.g. `{{MY_VAR}}`) with no substitution - * data, so the literal placeholder is sent on this first post-callback - * connection attempt even though the user's value is already saved - - * surfaces upstream as a generic auth rejection from the MCP server. - * The other reconnect path (oauth/reinitialize route below) already - * resolves this the same way; this one was missing it. - */ - let userMCPAuthMap; - if (serverConfig?.customUserVars && typeof serverConfig.customUserVars === 'object') { - try { - userMCPAuthMap = await getUserMCPAuthMap({ + const requestScoped = serverConfig + ? requiresEphemeralUserConnection(serverConfig) + : false; + if (requestScoped) { + logger.info( + `[MCP OAuth] Deferring post-OAuth connection for request-scoped server ${serverName} until its first chat use`, + ); + getOAuthReconnectionManager().clearReconnection(flowState.userId, serverName); + } else { + const mcpManager = getMCPManager(flowState.userId); + logger.debug(`[MCP OAuth] Attempting to reconnect ${serverName} with new OAuth tokens`); + + /** + * Without this, getUserConnection resolves `headers`/`oauth_headers` + * customUserVars templates (e.g. `{{MY_VAR}}`) with no substitution + * data, so the literal placeholder is sent on this first post-callback + * connection attempt even though the user's value is already saved - + * surfaces upstream as a generic auth rejection from the MCP server. + * The other reconnect path (oauth/reinitialize route below) already + * resolves this the same way; this one was missing it. + */ + let userMCPAuthMap; + if (serverConfig?.customUserVars && typeof serverConfig.customUserVars === 'object') { + try { + userMCPAuthMap = await getUserMCPAuthMap({ + userId: flowState.userId, + servers: [serverName], + findPluginAuthsByKeys: db.findPluginAuthsByKeys, + }); + } catch (error) { + logger.warn( + `[MCP OAuth] Could not resolve customUserVars for ${serverName} before reconnecting:`, + error, + ); + } + } + const customUserVars = getServerCustomUserVars(userMCPAuthMap, serverName); + + const { snapshot, publicationGeneration } = await mcpManager.withUserConnectionLease( + { + user, + serverName, + flowManager, + serverConfig, + customUserVars, + tokenMethods: { + findToken: db.findToken, + updateToken: db.updateToken, + createToken: db.createToken, + deleteTokens: db.deleteTokens, + }, + }, + async (userConnection) => { + logger.info( + `[MCP OAuth] Successfully reconnected ${serverName} for user ${flowState.userId}`, + ); + + const oauthReconnectionManager = getOAuthReconnectionManager(); + oauthReconnectionManager.clearReconnection(flowState.userId, serverName); + + const snapshot = + typeof userConnection.fetchOrderedToolsSnapshot === 'function' + ? await userConnection.fetchOrderedToolsSnapshot() + : await userConnection.fetchToolsSnapshot(); + return { + snapshot, + publicationGeneration: mcpManager.getToolPublicationGeneration?.(userConnection), + }; + }, + ); + if (snapshot.complete) { + await updateMCPServerTools({ userId: flowState.userId, - servers: [serverName], - findPluginAuthsByKeys: db.findPluginAuthsByKeys, + serverName, + tools: snapshot.tools, + serverConfig, + publicationGeneration, }); - } catch (error) { + } else { logger.warn( - `[MCP OAuth] Could not resolve customUserVars for ${serverName} before reconnecting:`, - error, + `[MCP OAuth] Preserving cached tools for ${serverName} because tools/list returned an incomplete snapshot`, ); } } - const customUserVars = getServerCustomUserVars(userMCPAuthMap, serverName); - - const { snapshot, publicationGeneration } = await mcpManager.withUserConnectionLease( - { - user, - serverName, - flowManager, - serverConfig, - customUserVars, - tokenMethods: { - findToken: db.findToken, - updateToken: db.updateToken, - createToken: db.createToken, - deleteTokens: db.deleteTokens, - }, - }, - async (userConnection) => { - logger.info( - `[MCP OAuth] Successfully reconnected ${serverName} for user ${flowState.userId}`, - ); - - const oauthReconnectionManager = getOAuthReconnectionManager(); - oauthReconnectionManager.clearReconnection(flowState.userId, serverName); - - const snapshot = - typeof userConnection.fetchOrderedToolsSnapshot === 'function' - ? await userConnection.fetchOrderedToolsSnapshot() - : await userConnection.fetchToolsSnapshot(); - return { - snapshot, - publicationGeneration: mcpManager.getToolPublicationGeneration?.(userConnection), - }; - }, - ); - if (snapshot.complete) { - await updateMCPServerTools({ - userId: flowState.userId, - serverName, - tools: snapshot.tools, - serverConfig, - publicationGeneration, - }); - } else { - logger.warn( - `[MCP OAuth] Preserving cached tools for ${serverName} because tools/list returned an incomplete snapshot`, - ); - } } else { logger.debug(`[MCP OAuth] System-level OAuth completed for ${serverName}`); } diff --git a/api/server/services/MCP.spec.js b/api/server/services/MCP.spec.js index 66bfe592ed..c03d0e8902 100644 --- a/api/server/services/MCP.spec.js +++ b/api/server/services/MCP.spec.js @@ -847,7 +847,7 @@ describe('tests for the new helper functions used by the MCP connection status e const config = { ...mockConfig, source: 'yaml', - url: 'https://mcp.example.com/{{LIBRECHAT_BODY_TENANT}}/mcp', + url: 'https://mcp.example.com/{{LIBRECHAT_BODY_CONVERSATIONID}}/mcp', }; mockGetOAuthReconnectionManager.mockReturnValue({ isReconnecting: jest.fn(() => false) }); mockGetFlowStateManager.mockReturnValue({ diff --git a/api/server/services/ToolService.js b/api/server/services/ToolService.js index db790803a5..c6654c0daf 100644 --- a/api/server/services/ToolService.js +++ b/api/server/services/ToolService.js @@ -1075,7 +1075,7 @@ async function loadToolDefinitionsWrapper({ connectionTimeout: Time.TWO_MINUTES, }); - if (result?.availableTools) { + if (result?.availableTools && Object.keys(result.availableTools).length > 0) { rememberMCPAvailableTools(serverName, result.availableTools); logger.info(`[Tool Definitions] OAuth completed for ${serverName}, tools available`); return { serverName, success: true }; diff --git a/api/server/services/__tests__/ToolService.spec.js b/api/server/services/__tests__/ToolService.spec.js index 0d0b60b71e..67a25dc7bf 100644 --- a/api/server/services/__tests__/ToolService.spec.js +++ b/api/server/services/__tests__/ToolService.spec.js @@ -635,6 +635,49 @@ describe('ToolService - Action Capability Gating', () => { ]); }); + it('does not count an empty post-OAuth catalog as tools available or reload definitions', async () => { + const req = createMockReq([AgentCapabilities.tools]); + const res = { writableEnded: false }; + const serverName = 'Empty-Catalog'; + const mcpTool = `search${Constants.mcp_delimiter}${serverName}`; + mockGetEndpointsConfig.mockResolvedValue(createEndpointsConfig([AgentCapabilities.tools])); + mockResolveConfigServers.mockResolvedValue({ + [serverName]: { + type: 'streamable-http', + url: 'https://mcp.example.com/empty', + requiresOAuth: true, + }, + }); + mockGetMCPServerTools.mockResolvedValue(null); + mockFlowManager.getFlowState.mockResolvedValue(null); + mockLoadToolDefinitions.mockImplementationOnce(async (params, deps) => { + await deps.getOrFetchMCPServerTools(params.userId, serverName); + return { + toolDefinitions: [], + toolRegistry: new Map(), + hasDeferredTools: false, + mcpResolution: { resolvedToolCount: 1 }, + }; + }); + reinitMCPServer + .mockImplementationOnce(async ({ oauthStart }) => { + await oauthStart(`https://auth.example.com/${serverName}`); + return { availableTools: null }; + }) + .mockResolvedValueOnce({ availableTools: {} }); + + const result = await loadAgentTools({ + req, + res, + agent: { id: 'agent_123', tools: [mcpTool] }, + definitionsOnly: true, + }); + + expect(result.toolDefinitions).toEqual([]); + expect(result.mcpAvailableTools).toEqual({}); + expect(mockLoadToolDefinitions).toHaveBeenCalledTimes(1); + }); + it('fences resumable MCP OAuth definition events to the owning job epoch', async () => { const req = createMockReq([AgentCapabilities.tools]); const res = { writableEnded: false }; diff --git a/client/src/Providers/AgentPanelContext.tsx b/client/src/Providers/AgentPanelContext.tsx index f465165e28..493ef03a93 100644 --- a/client/src/Providers/AgentPanelContext.tsx +++ b/client/src/Providers/AgentPanelContext.tsx @@ -99,6 +99,7 @@ export function AgentPanelProvider({ children }: { children: React.ReactNode }) tools, isConfigured: configuredServers.has(serverName), isConnected: connectionStatus?.[serverName]?.connectionState === 'connected', + requestScoped: serverConfig?.requestScoped, metadata, consumeOnly: serverConfig?.consumeOnly, }); @@ -130,6 +131,7 @@ export function AgentPanelProvider({ children }: { children: React.ReactNode }) isConfigured: true, serverName: mcpServerName, isConnected: connectionStatus?.[mcpServerName]?.connectionState === 'connected', + requestScoped: serverConfig?.requestScoped, consumeOnly: serverConfig?.consumeOnly, }); } diff --git a/client/src/common/types.ts b/client/src/common/types.ts index 20499a3206..07d7544de9 100644 --- a/client/src/common/types.ts +++ b/client/src/common/types.ts @@ -211,6 +211,8 @@ export interface MCPServerInfo { tools: t.AgentToolType[]; isConfigured: boolean; isConnected: boolean; + /** True when tools can only be discovered with live chat request fields. */ + requestScoped?: boolean; consumeOnly?: boolean; metadata: t.TPlugin; } diff --git a/client/src/components/SidePanel/Agents/Tools/ItemDialog/__tests__/McpSection.spec.tsx b/client/src/components/SidePanel/Agents/Tools/ItemDialog/__tests__/McpSection.spec.tsx index 6ff1be2099..0a15331a12 100644 --- a/client/src/components/SidePanel/Agents/Tools/ItemDialog/__tests__/McpSection.spec.tsx +++ b/client/src/components/SidePanel/Agents/Tools/ItemDialog/__tests__/McpSection.spec.tsx @@ -132,6 +132,7 @@ jest.mock('@librechat/client', () => { 'aria-label': ariaLabel, onChange: (e: { target: { checked: boolean } }) => onCheckedChange(e.target.checked), }), + Skeleton: ({ className }: { className?: string }) => React.createElement('div', { className }), }; }); @@ -186,6 +187,19 @@ describe('McpSection', () => { ); }); + test('selecting a current tool replaces stale catalog ids for the same server', () => { + mockGetValues.mockReturnValue(['removed_mcp_srv', 'dalle']); + + render(); + fireEvent.click(screen.getByTestId('tool-mcp:srv:a')); + + expect(mockSetValue).toHaveBeenCalledWith( + 'tools', + ['dalle', 'sys__server__sys_mcp_srv', 'mcp:srv:a'], + { shouldDirty: true }, + ); + }); + test('select-all writes every tool id', () => { render(); fireEvent.click(screen.getByLabelText('com_ui_tools_mcp_select_all')); @@ -249,6 +263,76 @@ describe('McpSection', () => { expect(screen.getByText('com_ui_tools_mcp_no_tools')).toBeInTheDocument(); }); + test('lets an already-connected request-scoped server attach its runtime tools', () => { + const runtimeItem: McpItem = { + ...item, + server: { + ...item.server, + tools: [], + isConnected: true, + requestScoped: true, + } as never, + toolCount: 0, + }; + + render(); + + expect(screen.getByText('com_ui_tools_mcp_runtime_tools_available')).toBeInTheDocument(); + expect(mockSetValue).not.toHaveBeenCalled(); + + fireEvent.click(screen.getByLabelText('com_ui_tools_mcp_select_all')); + expect(mockSetValue).toHaveBeenCalledWith( + 'tools', + ['sys__server__sys_mcp_srv', 'sys__all__sys_mcp_srv'], + { shouldDirty: true }, + ); + }); + + test('detaches every token for a request-scoped server while preserving unrelated tools', () => { + mockGetValues.mockReturnValue([ + 'sys__server__sys_mcp_srv', + 'sys__all__sys_mcp_srv', + 'search_mcp_srv', + 'dalle', + ]); + const runtimeItem: McpItem = { + ...item, + server: { + ...item.server, + tools: [], + isConnected: true, + requestScoped: true, + } as never, + toolCount: 0, + }; + + render(); + + expect(screen.getByText('com_ui_tools_mcp_runtime_tools')).toBeInTheDocument(); + fireEvent.click(screen.getByLabelText('com_ui_tools_mcp_deselect_all')); + expect(mockSetValue).toHaveBeenCalledWith('tools', ['dalle'], { shouldDirty: true }); + }); + + test('does not offer runtime attachment before a request-scoped server is connected', () => { + const disconnectedRuntimeItem: McpItem = { + ...item, + server: { + ...item.server, + tools: [], + isConnected: false, + requestScoped: true, + } as never, + toolCount: 0, + }; + + render(); + + expect(screen.queryByLabelText('com_ui_tools_mcp_select_all')).not.toBeInTheDocument(); + expect(screen.queryByText('com_ui_tools_mcp_runtime_tools_available')).not.toBeInTheDocument(); + expect(screen.getByText('com_ui_tools_mcp_no_tools')).toBeInTheDocument(); + expect(mockSetValue).not.toHaveBeenCalled(); + }); + test('deferred connect attaches the whole server via the mcp_all wildcard', async () => { // Request-scoped servers (runtime {{LIBRECHAT_BODY_*}} placeholders) defer // their connection to the next chat turn, so no tool list arrives here — diff --git a/client/src/components/SidePanel/Agents/Tools/ItemDialog/sections/McpSection.tsx b/client/src/components/SidePanel/Agents/Tools/ItemDialog/sections/McpSection.tsx index d4e0e5a5d5..ee792c06f7 100644 --- a/client/src/components/SidePanel/Agents/Tools/ItemDialog/sections/McpSection.tsx +++ b/client/src/components/SidePanel/Agents/Tools/ItemDialog/sections/McpSection.tsx @@ -18,8 +18,8 @@ import { useMCPServerManager, useMCPToolOptions, } from '~/hooks'; +import { matchesMcpServer, mcpAllToken, mcpServerToken } from '../../items/selectors'; import MCPServerStatusIcon from '~/components/MCP/MCPServerStatusIcon'; -import { mcpAllToken, mcpServerToken } from '../../items/selectors'; import MCPConfigDialog from '~/components/MCP/MCPConfigDialog'; import McpOAuthDialog from '~/components/MCP/McpOAuthDialog'; import { useAgentPanelContext } from '~/Providers'; @@ -167,6 +167,17 @@ export default function McpSection({ item }: Props) { [serverName, serverToken, serverAllToken, mcpServersMap], ); + const isServerSelection = useCallback( + (token: string): boolean => { + const allServerNames = Array.from(new Set([...mcpServersMap.keys(), serverName])); + return ( + matchesMcpServer(token, serverName, allServerNames) || + tools.some((tool) => tool.tool_id === toCurrentToolId(token)) + ); + }, + [mcpServersMap, serverName, tools, toCurrentToolId], + ); + /** * Migrates legacy raw-keyed `tool_options` for THIS server to the current * normalized ids the option toggles (defer / programmatic / background / @@ -221,21 +232,30 @@ export default function McpSection({ item }: Props) { * wildcard is also stripped unless explicitly re-passed in `next`, so a * per-tool selection always supersedes a stale wildcard (e.g. after a server * stops being request-scoped and its tools become enumerable). Legacy - * raw-keyed entries count as this server's (via `toCurrentToolId`), so a - * selection update REPLACES them instead of letting a deselected legacy - * tool survive every rewrite. */ + * raw-keyed and removed-tool entries count as this server's via boundary-safe + * server matching, so a selection update REPLACES them instead of letting an + * invisible stale tool survive every rewrite. */ const updateFormTools = useCallback( (next: string[]) => { const current = (getValues('tools') ?? []) as string[]; - const otherTools = current.filter( - (t) => - t !== serverToken && - t !== serverAllToken && - !tools.some((st) => st.tool_id === toCurrentToolId(t)), - ); + const otherTools = current.filter((tool) => !isServerSelection(tool)); setValue('tools', [...otherTools, serverToken, ...next], { shouldDirty: true }); }, - [getValues, setValue, serverToken, serverAllToken, tools, toCurrentToolId], + [getValues, isServerSelection, serverToken, setValue], + ); + + /** Request-scoped servers have no per-tool catalog outside a chat turn. Their + * sole meaningful selection is the runtime wildcard, so clearing it detaches + * the whole server instead of leaving behind an unusable server-only pin. */ + const toggleRuntimeTools = useCallback( + (checked: boolean) => { + const current = (getValues('tools') ?? []) as string[]; + const otherTools = current.filter((tool) => !isServerSelection(tool)); + setValue('tools', checked ? [...otherTools, serverToken, serverAllToken] : otherTools, { + shouldDirty: true, + }); + }, + [getValues, isServerSelection, serverAllToken, serverToken, setValue], ); const toggleToolSelect = (toolId: string) => { @@ -270,7 +290,7 @@ export default function McpSection({ item }: Props) { * both cases instead of a misleading "no tools" message. */ const toolsLoading = !hasTools && (mcpToolsLoading || isInitializing || connectionState === 'connecting'); - const isConnected = connectionState === 'connected'; + const isConnected = connectionState === 'connected' || liveServer.isConnected === true; const isBusy = isInitializing || connectionState === 'connecting'; /** Close + clear the OAuth dialog once the server connects, and don't let it @@ -296,15 +316,21 @@ export default function McpSection({ item }: Props) { * manager's init state (not the awaited response) also covers connects that * happen behind the customUserVars config dialog, which this component does * not await. */ - const serverDeferred = isConnectionDeferred(serverName); + const initConnectionDeferred = isConnectionDeferred(serverName); + const requestScoped = liveServer.requestScoped === true; + const runtimeToolsAvailable = + !hasTools && !toolsLoading && (isWildcardAttached || (requestScoped && isConnected)); + const runtimeToolsMessage = isWildcardAttached + ? 'com_ui_tools_mcp_runtime_tools' + : 'com_ui_tools_mcp_runtime_tools_available'; useEffect(() => { if (!autoSelectPending) { return; } - if (serverDeferred && !hasTools) { + if (initConnectionDeferred && !hasTools) { setAutoSelectPending(false); if (!isWildcardAttached) { - updateFormTools([serverAllToken]); + toggleRuntimeTools(true); } return; } @@ -315,13 +341,13 @@ export default function McpSection({ item }: Props) { updateFormTools(tools.map((t) => t.tool_id)); }, [ autoSelectPending, - serverDeferred, + initConnectionDeferred, isConnected, hasTools, tools, updateFormTools, + toggleRuntimeTools, isWildcardAttached, - serverAllToken, ]); /** Connect inline from this first dialog. Servers with custom user variables are @@ -407,9 +433,9 @@ export default function McpSection({ item }: Props) { {localize('com_ui_tools_mcp_tools_section')} - {hasTools && ( + {(hasTools || runtimeToolsAvailable) && (
- {deferredToolsEnabled && ( + {hasTools && deferredToolsEnabled && ( toggleDeferAll(tools)} /> )} - {programmaticToolsEnabled && ( + {hasTools && programmaticToolsEnabled && ( toggleProgrammaticAll(tools)} /> )} - {backgroundToolsEnabled && ( + {hasTools && backgroundToolsEnabled && ( toggleBackgroundAll(tools)} /> )} - {toolIntentsEnabled && ( + {hasTools && toolIntentsEnabled && ( toggleIntentAll(intentEligibleTools)} /> )} - {(deferredToolsEnabled || - programmaticToolsEnabled || - backgroundToolsEnabled || - toolIntentsEnabled) && ( -
diff --git a/client/src/components/SidePanel/Agents/Tools/ToolsMarketplaceDialog.tsx b/client/src/components/SidePanel/Agents/Tools/ToolsMarketplaceDialog.tsx index 2089518637..9b95ec79e0 100644 --- a/client/src/components/SidePanel/Agents/Tools/ToolsMarketplaceDialog.tsx +++ b/client/src/components/SidePanel/Agents/Tools/ToolsMarketplaceDialog.tsx @@ -11,7 +11,13 @@ import { } from '@librechat/client'; import type { AgentItem, AgentItemKind, ItemFilter } from './items/types'; import type { AgentForm } from '~/common'; -import { itemKey, mcpServerToken, matchesMcpServer, mcpServerIds } from './items/selectors'; +import { + itemKey, + mcpAllToken, + mcpServerToken, + matchesMcpServer, + mcpServerIds, +} from './items/selectors'; import { useAgentItems, useUninstallToolCredentials } from './hooks'; import AddMcpServerDialog from './ItemDialog/AddMcpServerDialog'; import { computeToggleAction } from './items/mutations'; @@ -121,7 +127,9 @@ export default function ToolsMarketplaceDialog({ } case 'mcp-add': { if (item.kind !== 'mcp') break; - const toolIds = (item.server.tools ?? []).map((t) => t.tool_id); + const toolIds = item.server.requestScoped + ? [mcpAllToken(item.id)] + : (item.server.tools ?? []).map((t) => t.tool_id); const current = (getValues('tools') ?? []) as string[]; setValue( 'tools', @@ -160,13 +168,19 @@ export default function ToolsMarketplaceDialog({ setDetailItem(item); return; } - /** An MCP server with no exposed tools yet can't be enabled in place — open - * its dialog so it can be connected/configured first. */ - if (item.kind === 'mcp' && item.toolCount === 0) { + const wasSelected = selectedIds.has(itemKey(item)); + /** An unselected, toolless MCP server normally needs its setup dialog. + * A connected request-scoped server is already ready and attaches via + * its runtime wildcard; a selected toolless server must remain removable. */ + if ( + item.kind === 'mcp' && + item.toolCount === 0 && + !wasSelected && + !(item.server.requestScoped === true && item.server.isConnected === true) + ) { setDetailItem(item); return; } - const wasSelected = selectedIds.has(itemKey(item)); if (!wasSelected && item.status === 'needs_setup') { setDetailItem(item); return; diff --git a/client/src/components/SidePanel/Agents/Tools/ToolsSection.tsx b/client/src/components/SidePanel/Agents/Tools/ToolsSection.tsx index 3008e14059..bf106b9bb4 100644 --- a/client/src/components/SidePanel/Agents/Tools/ToolsSection.tsx +++ b/client/src/components/SidePanel/Agents/Tools/ToolsSection.tsx @@ -30,6 +30,7 @@ import { useLocalize, useHasAccess } from '~/hooks'; import { useAgentPanelContext } from '~/Providers'; import { isEphemeralAgent, ESide } from '~/common'; import ItemDialog from './ItemDialog/ItemDialog'; +import { mcpAllToken } from './items/selectors'; import { InfoTrigger } from '../Advanced/ui'; import { Collapse } from '~/components/ui'; import SkillsDialog from './SkillsDialog'; @@ -51,7 +52,8 @@ export default function ToolsSection({ agentId }: Props) { const { control, getValues, setValue } = useFormContext(); const { agentsConfig, regularTools, mcpServersMap } = useAgentPanelContext(); - const { removeTool: removeMCPTool } = useRemoveMCPTool(); + const mcpServerNames = useMemo(() => Array.from(mcpServersMap?.keys() ?? []), [mcpServersMap]); + const { removeTool: removeMCPTool } = useRemoveMCPTool({ serverNames: mcpServerNames }); const deleteAgentAction = useDeleteAgentAction({ onSuccess: () => { showToast({ @@ -253,7 +255,9 @@ export default function ToolsSection({ agentId }: Props) { item.kind === 'mcp' ? { ...item, - toolCount: (item.server.tools ?? []).filter((t) => enabled.has(t.tool_id)).length, + toolCount: enabled.has(mcpAllToken(item.id)) + ? (item.server.tools ?? []).length + : (item.server.tools ?? []).filter((t) => enabled.has(t.tool_id)).length, } : item, ); diff --git a/client/src/components/SidePanel/Agents/Tools/__tests__/ToolsMarketplaceDialog.spec.tsx b/client/src/components/SidePanel/Agents/Tools/__tests__/ToolsMarketplaceDialog.spec.tsx index 404963505e..946c422343 100644 --- a/client/src/components/SidePanel/Agents/Tools/__tests__/ToolsMarketplaceDialog.spec.tsx +++ b/client/src/components/SidePanel/Agents/Tools/__tests__/ToolsMarketplaceDialog.spec.tsx @@ -3,7 +3,9 @@ import { fireEvent, render, screen } from '@testing-library/react'; import ToolsMarketplaceDialog from '../ToolsMarketplaceDialog'; const mockSetValue = jest.fn(); -const mockGetValues = jest.fn(() => []); +const mockGetValues = jest.fn((): string[] => []); +let mockWatchedTools: string[] = []; +let mockMcpServersMap = new Map(); jest.mock('react-hook-form', () => ({ useFormContext: () => ({ @@ -13,7 +15,7 @@ jest.mock('react-hook-form', () => ({ }), useWatch: ({ name }: { name: string }) => { const map: Record = { - tools: [], + tools: mockWatchedTools, skills: [], execute_code: false, web_search: false, @@ -31,7 +33,7 @@ jest.mock('~/Providers', () => ({ useAgentPanelContext: () => ({ agentsConfig: { capabilities: ['execute_code', 'tools'] }, regularTools: [{ pluginKey: 'dalle', name: 'DALL-E', description: 'Images' }], - mcpServersMap: new Map(), + mcpServersMap: mockMcpServersMap, actions: [], }), })); @@ -153,6 +155,8 @@ describe('ToolsMarketplaceDialog', () => { mockSetValue.mockClear(); mockGetValues.mockClear(); mockGetValues.mockReturnValue([]); + mockWatchedTools = []; + mockMcpServersMap = new Map(); mockToggleFavorite.mockClear(); mockFavoriteKeys = new Set(); }); @@ -198,6 +202,94 @@ describe('ToolsMarketplaceDialog', () => { ); }); + test('clicking a connected request-scoped zero-tool server attaches its runtime wildcard', () => { + mockMcpServersMap = new Map([ + [ + 'runtime', + { + serverName: 'runtime', + tools: [], + isConfigured: true, + isConnected: true, + requestScoped: true, + metadata: { name: 'runtime', pluginKey: 'runtime', description: '' }, + }, + ], + ]); + + render(); + fireEvent.click(screen.getByRole('button', { name: /runtime/ })); + + expect(screen.queryByTestId('item-dialog')).not.toBeInTheDocument(); + expect(mockSetValue).toHaveBeenCalledWith( + 'tools', + ['sys__server__sys_mcp_runtime', 'sys__all__sys_mcp_runtime'], + { shouldDirty: true }, + ); + }); + + test('clicking a selected zero-tool server removes all of its tokens directly', () => { + const selectedTools = [ + 'sys__server__sys_mcp_runtime', + 'sys__all__sys_mcp_runtime', + 'search_mcp_runtime', + 'dalle', + ]; + mockWatchedTools = selectedTools; + mockGetValues.mockReturnValue(selectedTools); + mockMcpServersMap = new Map([ + [ + 'runtime', + { + serverName: 'runtime', + tools: [], + isConfigured: true, + isConnected: true, + requestScoped: true, + metadata: { name: 'runtime', pluginKey: 'runtime', description: '' }, + }, + ], + ]); + + render(); + fireEvent.click(screen.getByRole('button', { name: /runtime/ })); + + expect(screen.queryByTestId('item-dialog')).not.toBeInTheDocument(); + expect(mockSetValue).toHaveBeenCalledWith('tools', ['dalle'], { shouldDirty: true }); + }); + + test.each([ + ['an ordinary connected', false, true], + ['a disconnected request-scoped', true, false], + ])( + 'clicking %s zero-tool server opens setup without changing the form', + (_description, requestScoped, isConnected) => { + mockMcpServersMap = new Map([ + [ + 'setup-required', + { + serverName: 'setup-required', + tools: [], + isConfigured: true, + isConnected, + requestScoped, + metadata: { + name: 'setup-required', + pluginKey: 'setup-required', + description: '', + }, + }, + ], + ]); + + render(); + fireEvent.click(screen.getByRole('button', { name: /setup-required/ })); + + expect(screen.getByTestId('item-dialog')).toBeInTheDocument(); + expect(mockSetValue).not.toHaveBeenCalled(); + }, + ); + test('clicking a card star toggles the favorite without selecting the tool', () => { render(); fireEvent.click(screen.getAllByRole('button', { name: 'com_ui_favorite' })[0]); diff --git a/client/src/components/SidePanel/Agents/Tools/__tests__/ToolsSection.spec.tsx b/client/src/components/SidePanel/Agents/Tools/__tests__/ToolsSection.spec.tsx index b136b5b21c..c9959ff2fc 100644 --- a/client/src/components/SidePanel/Agents/Tools/__tests__/ToolsSection.spec.tsx +++ b/client/src/components/SidePanel/Agents/Tools/__tests__/ToolsSection.spec.tsx @@ -5,6 +5,7 @@ import type { AgentItem } from '../items/types'; import ToolsSection from '../ToolsSection'; let mockSelected: AgentItem[] = []; +let mockAgentTools: string[] = []; let mockFileEntries: { contextFiles: unknown[]; knowledgeFiles: unknown[]; @@ -47,7 +48,7 @@ jest.mock('~/hooks/MCP', () => ({ })); jest.mock('../hooks', () => ({ - useAgentItems: () => ({ catalog: [], selected: mockSelected, tools: [] }), + useAgentItems: () => ({ catalog: [], selected: mockSelected, tools: mockAgentTools }), useResolvedSkills: (skills?: unknown[]) => skills, useAgentFileEntries: () => mockFileEntries, useUninstallToolCredentials: () => jest.fn(), @@ -58,6 +59,7 @@ jest.mock('../ToolRow', () => ({ default: ({ item, onRemove }: { item: AgentItem; onRemove: (item: AgentItem) => void }) => ( ), })); @@ -123,6 +125,7 @@ const fileSearchItem: AgentItem = { beforeEach(() => { mockSelected = []; + mockAgentTools = []; mockFileEntries = { contextFiles: [], knowledgeFiles: [], codeFiles: [] }; mockFormValues = {}; mockSetValue.mockClear(); @@ -153,6 +156,31 @@ describe('ToolsSection', () => { expect(screen.getByText('com_ui_skills_empty')).toBeInTheDocument(); }); + test('counts every enumerable MCP tool when the server is attached by wildcard', () => { + mockAgentTools = ['sys__all__sys_mcp_runtime']; + mockSelected = [ + { + kind: 'mcp', + id: 'runtime', + name: 'runtime', + description: '', + iconKey: 'mcp', + toolCount: 0, + server: { + serverName: 'runtime', + tools: [{ tool_id: 'search_mcp_runtime' }, { tool_id: 'read_mcp_runtime' }], + isConfigured: true, + isConnected: true, + metadata: { name: 'runtime', pluginKey: 'runtime', description: '' }, + } as never, + }, + ]; + + render(); + + expect(screen.getByRole('button', { name: 'remove-runtime' })).toHaveTextContent('2'); + }); + test('opens the config dialog instead of toggling when a file-backed built-in holds files', () => { mockSelected = [fileSearchItem]; mockFileEntries = { contextFiles: [], knowledgeFiles: [['f1', {}]], codeFiles: [] }; diff --git a/client/src/components/SidePanel/Agents/Tools/items/__tests__/selectors.spec.ts b/client/src/components/SidePanel/Agents/Tools/items/__tests__/selectors.spec.ts index 940aef31b3..f5ef4a18b9 100644 --- a/client/src/components/SidePanel/Agents/Tools/items/__tests__/selectors.spec.ts +++ b/client/src/components/SidePanel/Agents/Tools/items/__tests__/selectors.spec.ts @@ -152,6 +152,34 @@ describe('deriveSelectedItems', () => { expect(result.find((i) => i.kind === 'mcp')?.id).toBe('srv'); }); + test('an exact normalized MCP token selects only its collision owner', () => { + const serverNames = ['foo mcp bar', 'foo_mcp_bar']; + const catalog: AgentItem[] = [ + ...sampleCatalog, + ...serverNames.map( + (serverName): AgentItem => ({ + kind: 'mcp', + id: serverName, + name: serverName, + description: '', + iconKey: 'mcp', + server: makeMcpServer({ serverName }), + toolCount: 0, + }), + ), + ]; + + const result = deriveSelectedItems( + { ...emptyFormState, tools: ['mcp_foo_mcp_bar'] }, + catalog, + [], + ); + + expect(result.filter((item) => item.kind === 'mcp').map((item) => item.id)).toEqual([ + 'foo_mcp_bar', + ]); + }); + test('deselect-all (empty tools) leaves no MCP server selected', () => { const catalog: AgentItem[] = [ ...sampleCatalog, @@ -236,6 +264,27 @@ describe('matchesMcpServer', () => { expect(matchesMcpServer('search_mcp_bar', 'foo mcp bar', allServers)).toBe(false); }); + test('exact MCP tokens belong only to the configured owner of a normalized name', () => { + /** `foo mcp bar` and the literal `foo_mcp_bar` normalize to the same + * model-facing name. An exact `mcp_foo_mcp_bar` token must follow the + * alias registry's identity-first ownership instead of selecting both. */ + const collidingServers = ['foo mcp bar', 'foo_mcp_bar']; + expect(matchesMcpServer('mcp_foo_mcp_bar', 'foo mcp bar', collidingServers)).toBe(false); + expect(matchesMcpServer('mcp_foo_mcp_bar', 'foo_mcp_bar', collidingServers)).toBe(true); + + /** Without an identity-name collision, the normalized exact token still + * resolves back to its special-character raw server as before. */ + expect(matchesMcpServer('mcp_foo_mcp_bar', 'foo mcp bar', ['foo mcp bar'])).toBe(true); + expect(matchesMcpServer('mcp_foo mcp bar', 'foo mcp bar', collidingServers)).toBe(true); + expect(matchesMcpServer('mcp_foo mcp bar', 'foo_mcp_bar', collidingServers)).toBe(false); + + /** If neither raw name is already normalized, the alias registry's + * deterministic first-configured owner is the only match. */ + const aliasCollision = ['foo!', 'foo?']; + expect(matchesMcpServer('mcp_foo', 'foo!', aliasCollision)).toBe(true); + expect(matchesMcpServer('mcp_foo', 'foo?', aliasCollision)).toBe(false); + }); + test('matches normalized-spelling tool ids for a special-character server', () => { /** Model-facing tool ids embed `normalizeServerName(server)`, while the * marketplace/server cards are keyed raw — both spellings must count as diff --git a/client/src/components/SidePanel/Agents/Tools/items/selectors.ts b/client/src/components/SidePanel/Agents/Tools/items/selectors.ts index 5023d8cab4..36bc9937c5 100644 --- a/client/src/components/SidePanel/Agents/Tools/items/selectors.ts +++ b/client/src/components/SidePanel/Agents/Tools/items/selectors.ts @@ -102,6 +102,22 @@ export function matchesMcpServer( ): boolean { const prefixed = `${MCP_PREFIX}${serverName}`; const normalized = normalizeServerName(serverName); + const aliases = allServerNames?.length ? buildServerNameAliases(allServerNames) : undefined; + if (aliases && allServerNames) { + /** Exact `mcp_` entries need the same single-owner resolution as + * tool-key suffixes. A literal configured name wins over another name + * that merely normalizes to it; otherwise the alias registry maps the + * normalized spelling back to its raw owner. Without this early global + * check, each colliding target could independently satisfy its own exact + * comparison and one token would select/remove both servers. */ + if (token.startsWith(MCP_PREFIX)) { + const exactName = token.slice(MCP_PREFIX.length); + const exactOwner = allServerNames.includes(exactName) ? exactName : aliases.get(exactName); + if (exactOwner != null) { + return exactOwner === serverName; + } + } + } if ( token === mcpServerToken(serverName) || token === serverName || @@ -110,13 +126,12 @@ export function matchesMcpServer( ) { return true; } - if (allServerNames?.length) { + if (aliases && allServerNames) { /** Boundary-exact: resolve the token ONCE against every configured * server (longest match, both spellings) — a normalized name that * itself contains the delimiter (`foo mcp bar` → `foo_mcp_bar`) must * not ALSO suffix-match a server named `bar`, or both cards select * together and removing one strips the other's tool. */ - const aliases = buildServerNameAliases(allServerNames); const [, parsed] = splitMCPToolKey(token, [...allServerNames, ...aliases.keys()]); if (parsed == null) { return false; diff --git a/client/src/hooks/MCP/__tests__/useRemoveMCPTool.spec.tsx b/client/src/hooks/MCP/__tests__/useRemoveMCPTool.spec.tsx index 296856b8c2..376d0ea091 100644 --- a/client/src/hooks/MCP/__tests__/useRemoveMCPTool.spec.tsx +++ b/client/src/hooks/MCP/__tests__/useRemoveMCPTool.spec.tsx @@ -58,6 +58,25 @@ describe('useRemoveMCPTool', () => { expect(mockShowToast).toHaveBeenCalledTimes(1); }); + test('does not remove a longer configured server whose tool key shares the suffix', () => { + const tools = [ + `search${Constants.mcp_delimiter}bar`, + `search${Constants.mcp_delimiter}foo_mcp_bar`, + ]; + let next: string[] = []; + const { result } = renderHook(() => useRemoveMCPTool({ serverNames: ['bar', 'foo mcp bar'] }), { + wrapper: makeWrapper(tools, (value) => { + next = value; + }), + }); + + act(() => { + result.current.removeTool('bar'); + }); + + expect(next).toEqual([`search${Constants.mcp_delimiter}foo_mcp_bar`]); + }); + test('ignores an empty server name', () => { let called = false; const { result } = renderHook(() => useRemoveMCPTool(), { diff --git a/client/src/hooks/MCP/useMCPServerManager.ts b/client/src/hooks/MCP/useMCPServerManager.ts index 6ebeed6632..910bba288b 100644 --- a/client/src/hooks/MCP/useMCPServerManager.ts +++ b/client/src/hooks/MCP/useMCPServerManager.ts @@ -45,6 +45,8 @@ export interface MCPServerDefinition { dbId?: string; // MongoDB ObjectId for database servers (used for permissions) effectivePermissions: number; // Permission bits (VIEW=1, EDIT=2, DELETE=4, SHARE=8) consumeOnly?: boolean; + /** True when chat request fields are required before the server can connect. */ + requestScoped?: boolean; } // Poll intervals are kept local since they're timer references that can't be serialized @@ -80,7 +82,7 @@ export function useMCPServerManager({ const definitions: MCPServerDefinition[] = []; if (loadedServers) { for (const [serverName, metadata] of Object.entries(loadedServers)) { - const { dbId, consumeOnly, ...config } = metadata; + const { dbId, consumeOnly, requestScoped, ...config } = metadata; // Get effective permissions from the permissions map using _id // Fall back to 1 (VIEW) for YAML-based servers without _id @@ -91,6 +93,7 @@ export function useMCPServerManager({ dbId, effectivePermissions, consumeOnly, + requestScoped, config, }); } diff --git a/client/src/hooks/MCP/useRemoveMCPTool.ts b/client/src/hooks/MCP/useRemoveMCPTool.ts index 696db01b94..e1ae279c88 100644 --- a/client/src/hooks/MCP/useRemoveMCPTool.ts +++ b/client/src/hooks/MCP/useRemoveMCPTool.ts @@ -9,11 +9,15 @@ import { useLocalize } from '~/hooks'; * Hook for removing an MCP server (and all of its tools) from the agent form. * Note: This only removes the tool from the form, it does not delete associated auth credentials */ -export function useRemoveMCPTool(options?: { showToast?: boolean }) { +export function useRemoveMCPTool(options?: { + showToast?: boolean; + serverNames?: readonly string[]; +}) { const localize = useLocalize(); const { showToast } = useToastContext(); const { getValues, setValue } = useFormContext(); const shouldShowToast = options?.showToast !== false; + const serverNames = options?.serverNames; const removeTool = useCallback( (serverName: string) => { @@ -22,11 +26,14 @@ export function useRemoveMCPTool(options?: { showToast?: boolean }) { } const currentTools = getValues('tools'); + const allServerNames = Array.from(new Set([...(serverNames ?? []), serverName])); /** Strip every token format the selection logic counts as this server — * removal lagging behind `matchesMcpServer` leaves the row permanently * selected with no way to clean it up. */ const remainingToolIds = - currentTools?.filter((currentToolId) => !matchesMcpServer(currentToolId, serverName)) || []; + currentTools?.filter( + (currentToolId) => !matchesMcpServer(currentToolId, serverName, allServerNames), + ) || []; setValue('tools', remainingToolIds, { shouldDirty: true }); if (shouldShowToast) { @@ -36,7 +43,7 @@ export function useRemoveMCPTool(options?: { showToast?: boolean }) { }); } }, - [getValues, setValue, showToast, localize, shouldShowToast], + [getValues, setValue, showToast, localize, shouldShowToast, serverNames], ); return { removeTool }; diff --git a/client/src/locales/en/translation.json b/client/src/locales/en/translation.json index 1685bfcb4b..0a47014d49 100644 --- a/client/src/locales/en/translation.json +++ b/client/src/locales/en/translation.json @@ -2103,6 +2103,7 @@ "com_ui_tools_marketplace_search": "Search tools…", "com_ui_tools_mcp_deselect_all": "Deselect all", "com_ui_tools_mcp_no_tools": "This server has not exposed any tools yet.", + "com_ui_tools_mcp_runtime_tools_available": "This server's tools are resolved at runtime, during chat.", "com_ui_tools_mcp_runtime_tools": "All of this server's tools are attached; they are resolved at runtime, during chat.", "com_ui_tools_mcp_select_all": "Select all", "com_ui_tools_mcp_status_unconfigured": "Needs configuration", diff --git a/packages/api/src/mcp/__tests__/utils.test.ts b/packages/api/src/mcp/__tests__/utils.test.ts index 120278f683..0f85636cc4 100644 --- a/packages/api/src/mcp/__tests__/utils.test.ts +++ b/packages/api/src/mcp/__tests__/utils.test.ts @@ -401,6 +401,38 @@ describe('redactServerSecrets', () => { expect(redacted.customUserVars).toEqual(config.customUserVars); }); + it('should expose request-scoped behavior without exposing placeholder-bearing fields', () => { + const config: ParsedServerConfig = { + type: 'streamable-http', + url: 'https://infra.internal/mcp', + source: 'yaml', + headers: { 'X-Conversation': '{{LIBRECHAT_BODY_CONVERSATIONID}}' }, + }; + + const redacted = redactServerSecrets(config); + + expect(redacted.requestScoped).toBe(true); + expect(redacted.url).toBeUndefined(); + expect((redacted as Record).headers).toBeUndefined(); + }); + + it('should omit request-scoped metadata for ordinary and unsupported BODY placeholders', () => { + expect( + redactServerSecrets({ + type: 'streamable-http', + url: 'https://example.com/mcp', + source: 'yaml', + }).requestScoped, + ).toBeUndefined(); + expect( + redactServerSecrets({ + type: 'streamable-http', + url: 'https://example.com/{{LIBRECHAT_BODY_TENANT}}/mcp', + source: 'yaml', + }).requestScoped, + ).toBeUndefined(); + }); + it('should pass URLs through unchanged when caller has edit authority', () => { const config: ParsedServerConfig = { type: 'sse', @@ -810,6 +842,39 @@ describe('hasRuntimeBodyPlaceholders', () => { }), ).toBe(false); }); + + it('ignores unsupported BODY placeholder names that the resolver leaves literal', () => { + const config = { + source: 'yaml' as const, + url: 'https://example.com/{{LIBRECHAT_BODY_TENANT}}/mcp', + }; + + expect(hasRuntimeContextPlaceholders(config)).toBe(false); + expect(hasRuntimeUrlPlaceholders(config)).toBe(false); + expect(hasRuntimeBodyPlaceholders(config)).toBe(false); + expect(getRuntimeBodyPlaceholderFields(config)).toEqual([]); + expect(getMissingRuntimeBodyPlaceholderFields(config)).toEqual([]); + expect(requiresEphemeralUserConnection(config)).toBe(false); + expect(requiresUserScopedConnection(config)).toBe(false); + }); + + it('ignores BODY and USER literals in plugin-sourced configs', () => { + const config = { + source: 'plugin' as const, + url: 'https://example.com/users/{{LIBRECHAT_USER_ID}}/mcp', + headers: { + 'X-Conversation': '{{LIBRECHAT_BODY_CONVERSATIONID}}', + }, + }; + + expect(hasRuntimeContextPlaceholders(config)).toBe(false); + expect(hasRuntimeUrlPlaceholders(config)).toBe(false); + expect(hasRuntimeBodyPlaceholders(config)).toBe(false); + expect(getRuntimeBodyPlaceholderFields(config)).toEqual([]); + expect(getMissingRuntimeBodyPlaceholderFields(config)).toEqual([]); + expect(requiresEphemeralUserConnection(config)).toBe(false); + expect(requiresUserScopedConnection(config)).toBe(false); + }); }); describe('getMissingRuntimeBodyPlaceholderFields', () => { diff --git a/packages/api/src/mcp/oauth/OAuthReconnectionManager.test.ts b/packages/api/src/mcp/oauth/OAuthReconnectionManager.test.ts index c4c0544b96..0a97b3518f 100644 --- a/packages/api/src/mcp/oauth/OAuthReconnectionManager.test.ts +++ b/packages/api/src/mcp/oauth/OAuthReconnectionManager.test.ts @@ -1,5 +1,6 @@ import { logger, TokenMethods } from '@librechat/data-schemas'; import type { IToken } from '@librechat/data-schemas'; +import type { ParsedServerConfig } from '../..'; import { OAuthReconnectionManager } from './OAuthReconnectionManager'; import { OAuthReconnectionTracker } from './OAuthReconnectionTracker'; import { FlowStateManager, MCPConnection, MCPOptions } from '../..'; @@ -203,6 +204,8 @@ describe('OAuthReconnectionManager', () => { await reconnectionManager.reconnectServers(userId); + expect(mockRegistryInstance.getOAuthServers).toHaveBeenCalledWith(userId); + // Verify server3 was marked as active expect(reconnectionTracker.isActive(userId, 'server3')).toBe(true); @@ -213,6 +216,7 @@ describe('OAuthReconnectionManager', () => { expect(mockMCPManager.getUserConnection).toHaveBeenCalledWith({ serverName: 'server3', user: { id: userId }, + serverConfig: { initTimeout: 5000 }, flowManager, tokenMethods, forceNew: false, @@ -452,6 +456,71 @@ describe('OAuthReconnectionManager', () => { expect(result).toBe(true); }); + it('should defer request-scoped reconnection without reporting success', async () => { + const userId = 'user-123'; + const serverName = 'request-scoped-server'; + + reconnectionTracker.setFailed(userId, serverName); + reconnectionTracker.setActive(userId, serverName); + (mockRegistryInstance.getServerConfig as jest.Mock).mockResolvedValue({ + type: 'streamable-http', + url: 'https://example.com/mcp', + source: 'yaml', + headers: { + 'X-Conversation-ID': '{{LIBRECHAT_BODY_CONVERSATIONID}}', + }, + } as unknown as MCPOptions); + + const result = await reconnectionManager.reconnectServer(userId, serverName); + + expect(result).toBe(false); + expect(reconnectionTracker.isFailed(userId, serverName)).toBe(false); + expect(reconnectionTracker.isActive(userId, serverName)).toBe(false); + expect(mockMCPManager.getUserConnection).not.toHaveBeenCalled(); + expect(mockMCPManager.disconnectUserConnection).not.toHaveBeenCalled(); + }); + + it('should classify request scope from the effective config-tier overlay', async () => { + const userId = 'user-123'; + const serverName = 'overlaid-server'; + const configServers: Record = { + [serverName]: { + type: 'streamable-http', + url: 'https://example.com/mcp', + source: 'config', + headers: { + 'X-Conversation-ID': '{{LIBRECHAT_BODY_CONVERSATIONID}}', + }, + }, + }; + const effectiveConfig = { + ...configServers[serverName], + source: 'yaml', + } as ParsedServerConfig; + + (mockRegistryInstance.getServerConfig as jest.Mock).mockImplementation( + async (_name, _userId, candidates) => + candidates === configServers + ? effectiveConfig + : ({ + type: 'streamable-http', + url: 'https://example.com/mcp', + source: 'yaml', + } as MCPOptions), + ); + const result = await reconnectionManager.reconnectServer(userId, serverName, configServers); + + expect(result).toBe(false); + expect(mockRegistryInstance.getServerConfig).toHaveBeenCalledWith( + serverName, + userId, + configServers, + ); + expect(mockMCPManager.getUserConnection).not.toHaveBeenCalled(); + expect(reconnectionTracker.isFailed(userId, serverName)).toBe(false); + expect(reconnectionTracker.isActive(userId, serverName)).toBe(false); + }); + it('should return false on failed reconnection', async () => { const userId = 'user-123'; const serverName = 'server1'; diff --git a/packages/api/src/mcp/oauth/OAuthReconnectionManager.ts b/packages/api/src/mcp/oauth/OAuthReconnectionManager.ts index 1adb9b4be0..0857462559 100644 --- a/packages/api/src/mcp/oauth/OAuthReconnectionManager.ts +++ b/packages/api/src/mcp/oauth/OAuthReconnectionManager.ts @@ -1,13 +1,16 @@ import { logger } from '@librechat/data-schemas'; import type { TokenMethods, IUser } from '@librechat/data-schemas'; +import type { ParsedServerConfig } from '~/mcp/types'; import type { MCPOAuthTokens } from './types'; import { MCPServersRegistry } from '~/mcp/registry/MCPServersRegistry'; import { OAuthReconnectionTracker } from './OAuthReconnectionTracker'; +import { requiresEphemeralUserConnection } from '~/mcp/utils'; import { FlowStateManager } from '~/flow/manager'; import { MCPManager } from '~/mcp/MCPManager'; const DEFAULT_CONNECTION_TIMEOUT_MS = 10_000; // ms const RECONNECT_STAGGER_MS = 500; // ms between each server reconnection +type ReconnectOutcome = 'connected' | 'deferred' | 'failed'; export class OAuthReconnectionManager { private static instance: OAuthReconnectionManager | null = null; @@ -62,7 +65,14 @@ export class OAuthReconnectionManager { return this.reconnectionsTracker.isStillReconnecting(userId, serverName); } - public async reconnectServers(userId: string): Promise { + /** + * Reconnects the user's eligible OAuth servers. + * @param configServers Tenant-scoped Config-tier candidates used to resolve effective overlays. + */ + public async reconnectServers( + userId: string, + configServers?: Record, + ): Promise { // Check if MCPManager is available if (this.mcpManager == null) { logger.warn( @@ -73,7 +83,7 @@ export class OAuthReconnectionManager { // 1. derive the servers to reconnect const serversToReconnect = []; - for (const serverName of await MCPServersRegistry.getInstance().getOAuthServers()) { + for (const serverName of await MCPServersRegistry.getInstance().getOAuthServers(userId)) { const canReconnect = await this.canReconnect(userId, serverName); if (canReconnect) { serversToReconnect.push(serverName); @@ -89,9 +99,12 @@ export class OAuthReconnectionManager { for (let i = 0; i < serversToReconnect.length; i++) { const serverName = serversToReconnect[i]; if (i === 0) { - this.safeTryReconnect(userId, serverName); + this.safeTryReconnect(userId, serverName, configServers); } else { - setTimeout(() => this.safeTryReconnect(userId, serverName), i * RECONNECT_STAGGER_MS); + setTimeout( + () => this.safeTryReconnect(userId, serverName, configServers), + i * RECONNECT_STAGGER_MS, + ); } } } @@ -104,8 +117,12 @@ export class OAuthReconnectionManager { * `RECONNECTION_TIMEOUT_MS` window if an error escapes * {@link tryReconnect}'s internal try/catch. */ - private safeTryReconnect(userId: string, serverName: string): void { - this.tryReconnect(userId, serverName).catch((error) => { + private safeTryReconnect( + userId: string, + serverName: string, + configServers?: Record, + ): void { + this.tryReconnect(userId, serverName, configServers).catch((error) => { logger.error( `[OAuthReconnectionManager][User: ${userId}][${serverName}] Unexpected reconnect error`, error, @@ -122,17 +139,21 @@ export class OAuthReconnectionManager { /** * Attempts to reconnect a single OAuth MCP server. + * @param configServers Tenant-scoped Config-tier candidates used to resolve the effective config. * @returns true if reconnection succeeded, false otherwise. */ - public async reconnectServer(userId: string, serverName: string): Promise { + public async reconnectServer( + userId: string, + serverName: string, + configServers?: Record, + ): Promise { if (this.mcpManager == null) { return false; } this.reconnectionsTracker.setActive(userId, serverName); try { - await this.tryReconnect(userId, serverName); - return !this.reconnectionsTracker.isFailed(userId, serverName); + return (await this.tryReconnect(userId, serverName, configServers)) === 'connected'; } catch { return false; } @@ -143,9 +164,13 @@ export class OAuthReconnectionManager { this.reconnectionsTracker.removeActive(userId, serverName); } - private async tryReconnect(userId: string, serverName: string) { + private async tryReconnect( + userId: string, + serverName: string, + configServers?: Record, + ): Promise { if (this.mcpManager == null) { - return; + return 'failed'; } const logPrefix = `[tryReconnectOAuthMCPServer][User: ${userId}][${serverName}]`; @@ -153,12 +178,22 @@ export class OAuthReconnectionManager { logger.info(`${logPrefix} Attempting reconnection`); try { - const config = await MCPServersRegistry.getInstance().getServerConfig(serverName, userId); + const config = await MCPServersRegistry.getInstance().getServerConfig( + serverName, + userId, + configServers, + ); + if (config && requiresEphemeralUserConnection(config)) { + logger.info(`${logPrefix} Deferring request-scoped connection until chat use`); + this.clearReconnection(userId, serverName); + return 'deferred'; + } // attempt to get connection (this will use existing tokens and refresh if needed) const connection = await this.mcpManager.getUserConnection({ serverName, user: { id: userId } as IUser, + serverConfig: config, flowManager: this.flowManager, tokenMethods: this.tokenMethods, // don't force new connection, let it reuse existing or create new as needed @@ -172,14 +207,17 @@ export class OAuthReconnectionManager { if (connection && (await connection.isConnected())) { logger.info(`${logPrefix} Successfully reconnected`); this.clearReconnection(userId, serverName); + return 'connected'; } else { logger.warn(`${logPrefix} Failed to reconnect`); await connection?.disconnect(); this.cleanupOnFailedReconnect(userId, serverName); + return 'failed'; } } catch (error) { logger.warn(`${logPrefix} Failed to reconnect: ${error}`); this.cleanupOnFailedReconnect(userId, serverName); + return 'failed'; } } diff --git a/packages/api/src/mcp/utils.ts b/packages/api/src/mcp/utils.ts index 07c296ffbd..5327365272 100644 --- a/packages/api/src/mcp/utils.ts +++ b/packages/api/src/mcp/utils.ts @@ -8,6 +8,7 @@ import { import type { AgentToolOptions } from 'librechat-data-provider'; import type { ParsedServerConfig } from '~/mcp/types'; import type { RequestBody } from '~/types'; +import { ALLOWED_BODY_FIELDS, isPluginSourced } from '~/utils/env'; export const mcpToolPattern: RegExp = new RegExp(`^.+${Constants.mcp_delimiter}.+$`); @@ -178,15 +179,18 @@ export function normalizeAgentToolKeys(params: { }; } -const RUNTIME_CONTEXT_PLACEHOLDER_PATTERN = /\{\{LIBRECHAT_(?:USER|OPENID|GRAPH|BODY)_[^}]+\}\}/; -const RUNTIME_BODY_PLACEHOLDER_PATTERN = /\{\{LIBRECHAT_BODY_[^}]+\}\}/; -const RUNTIME_BODY_PLACEHOLDER_CAPTURE_PATTERN = /\{\{LIBRECHAT_BODY_([^}]+)\}\}/g; - -const BODY_PLACEHOLDER_FIELDS: Record = { - CONVERSATIONID: 'conversationId', - PARENTMESSAGEID: 'parentMessageId', - MESSAGEID: 'messageId', -}; +const RUNTIME_CONTEXT_PLACEHOLDER_PATTERN = /\{\{LIBRECHAT_(?:USER|OPENID|GRAPH)_[^}]+\}\}/; +const BODY_PLACEHOLDER_FIELDS = Object.fromEntries( + ALLOWED_BODY_FIELDS.map((field) => [field.toUpperCase(), field]), +) as Record; +const RUNTIME_BODY_FIELD_NAMES = Object.keys(BODY_PLACEHOLDER_FIELDS).join('|'); +const RUNTIME_BODY_PLACEHOLDER_PATTERN = new RegExp( + `\\{\\{LIBRECHAT_BODY_(?:${RUNTIME_BODY_FIELD_NAMES})\\}\\}`, +); +const RUNTIME_BODY_PLACEHOLDER_CAPTURE_PATTERN = new RegExp( + `\\{\\{LIBRECHAT_BODY_(${RUNTIME_BODY_FIELD_NAMES})\\}\\}`, + 'g', +); type PlaceholderValue = | string @@ -255,7 +259,10 @@ export function hasCustomUserVars( } function hasRuntimeContextPlaceholder(value: PlaceholderValue): boolean { - return hasPlaceholder(value, RUNTIME_CONTEXT_PLACEHOLDER_PATTERN); + return ( + hasPlaceholder(value, RUNTIME_CONTEXT_PLACEHOLDER_PATTERN) || + hasPlaceholder(value, RUNTIME_BODY_PLACEHOLDER_PATTERN) + ); } function hasPlaceholder(value: PlaceholderValue, pattern: RegExp): boolean { @@ -277,8 +284,9 @@ function addRuntimeBodyPlaceholderFields(value: PlaceholderValue, fields: Set & { + /** True when this config needs chat request fields before it can connect. */ + requestScoped?: boolean; +}; + /** * Allowlist-based sanitization for API responses. Only explicitly listed fields are included; * new fields added to ParsedServerConfig are excluded by default until allowlisted here. @@ -443,8 +461,8 @@ export function isUserSourced(config: Pick { - const safe: Partial = { +): RedactedServerConfig { + const safe: RedactedServerConfig = { type: config.type, url: config.url, title: config.title, @@ -464,6 +482,9 @@ export function redactServerSecrets( inspectionFailed: config.inspectionFailed, customUserVars: config.customUserVars, serverInstructions: config.serverInstructions, + /** Safe derived metadata: it exposes no placeholder-bearing value, but lets + * clients attach tools that can only be discovered during a chat turn. */ + requestScoped: requiresEphemeralUserConnection(config) || undefined, }; if (config.apiKey) { @@ -498,15 +519,15 @@ export function redactServerSecrets( return Object.fromEntries( Object.entries(safe).filter(([, v]) => v !== undefined), - ) as Partial; + ) as RedactedServerConfig; } /** Applies allowlist-based sanitization to a map of server configs. */ export function redactAllServerSecrets( configs: Record, options?: { canEditByServer?: ReadonlyMap }, -): Record> { - const result: Record> = {}; +): Record { + const result: Record = {}; for (const [key, config] of Object.entries(configs)) { const canEdit = options?.canEditByServer?.get(key) ?? false; result[key] = redactServerSecrets(config, { canEdit }); diff --git a/packages/api/src/utils/env.ts b/packages/api/src/utils/env.ts index f18e8538ec..6c18825e72 100644 --- a/packages/api/src/utils/env.ts +++ b/packages/api/src/utils/env.ts @@ -142,7 +142,7 @@ export function createSafeUser( * List of allowed request body fields that can be used in header placeholders. * These are common fields from the request body that are safe to expose in headers. */ -const ALLOWED_BODY_FIELDS = ['conversationId', 'parentMessageId', 'messageId'] as const; +export const ALLOWED_BODY_FIELDS = ['conversationId', 'parentMessageId', 'messageId'] as const; /** * Matches every placeholder this module knows how to resolve: the enumerated diff --git a/packages/data-provider/src/types/mcpServers.ts b/packages/data-provider/src/types/mcpServers.ts index c2b1d3d833..65e5040a99 100644 --- a/packages/data-provider/src/types/mcpServers.ts +++ b/packages/data-provider/src/types/mcpServers.ts @@ -44,6 +44,8 @@ export type MCPServerDBObjectResponse = { serverName: string; /** True if access is only via agent (not directly shared with user) */ consumeOnly?: boolean; + /** True when chat request fields are required before the server can connect. */ + requestScoped?: boolean; } & MCPOptions; export type MCPServersListResponse = Record;