mirror of
https://github.com/danny-avila/LibreChat.git
synced 2025-12-17 08:50:15 +01:00
🐛 fix: MCP Name Normalization breaking User Provided Variables (#8644)
This commit is contained in:
parent
01470ef9fd
commit
1fe977e48f
4 changed files with 172 additions and 13 deletions
|
|
@ -3,7 +3,6 @@ const { isEnabled, getUserMCPAuthMap } = require('@librechat/api');
|
||||||
const { CacheKeys, EModelEndpoint } = require('librechat-data-provider');
|
const { CacheKeys, EModelEndpoint } = require('librechat-data-provider');
|
||||||
const { normalizeEndpointName } = require('~/server/utils');
|
const { normalizeEndpointName } = require('~/server/utils');
|
||||||
const loadCustomConfig = require('./loadCustomConfig');
|
const loadCustomConfig = require('./loadCustomConfig');
|
||||||
const { getCachedTools } = require('./getCachedTools');
|
|
||||||
const getLogStores = require('~/cache/getLogStores');
|
const getLogStores = require('~/cache/getLogStores');
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|
@ -66,13 +65,9 @@ async function getMCPAuthMap({ userId, tools, findPluginAuthsByKeys }) {
|
||||||
if (!tools || tools.length === 0) {
|
if (!tools || tools.length === 0) {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
const appTools = await getCachedTools({
|
|
||||||
userId,
|
|
||||||
});
|
|
||||||
return await getUserMCPAuthMap({
|
return await getUserMCPAuthMap({
|
||||||
tools,
|
tools,
|
||||||
userId,
|
userId,
|
||||||
appTools,
|
|
||||||
findPluginAuthsByKeys,
|
findPluginAuthsByKeys,
|
||||||
});
|
});
|
||||||
} catch (err) {
|
} catch (err) {
|
||||||
|
|
|
||||||
|
|
@ -235,6 +235,7 @@ async function createMCPTool({ req, res, toolKey, provider: _provider }) {
|
||||||
responseFormat: AgentConstants.CONTENT_AND_ARTIFACT,
|
responseFormat: AgentConstants.CONTENT_AND_ARTIFACT,
|
||||||
});
|
});
|
||||||
toolInstance.mcp = true;
|
toolInstance.mcp = true;
|
||||||
|
toolInstance.mcpRawServerName = serverName;
|
||||||
return toolInstance;
|
return toolInstance;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
168
packages/api/src/mcp/auth.test.ts
Normal file
168
packages/api/src/mcp/auth.test.ts
Normal file
|
|
@ -0,0 +1,168 @@
|
||||||
|
import type { PluginAuthMethods } from '@librechat/data-schemas';
|
||||||
|
import type { GenericTool } from '@librechat/agents';
|
||||||
|
import { getPluginAuthMap } from '~/agents/auth';
|
||||||
|
import { getUserMCPAuthMap } from './auth';
|
||||||
|
|
||||||
|
jest.mock('~/agents/auth', () => ({
|
||||||
|
getPluginAuthMap: jest.fn(),
|
||||||
|
}));
|
||||||
|
|
||||||
|
const mockGetPluginAuthMap = getPluginAuthMap as jest.MockedFunction<typeof getPluginAuthMap>;
|
||||||
|
|
||||||
|
const createMockTool = (
|
||||||
|
name: string,
|
||||||
|
mcpRawServerName?: string,
|
||||||
|
mcp = true,
|
||||||
|
): GenericTool & { mcpRawServerName?: string; mcp?: boolean } =>
|
||||||
|
({
|
||||||
|
name,
|
||||||
|
mcpRawServerName,
|
||||||
|
mcp,
|
||||||
|
description: 'Mock tool',
|
||||||
|
}) as GenericTool & { mcpRawServerName?: string; mcp?: boolean };
|
||||||
|
|
||||||
|
const mockFindPluginAuthsByKeys: PluginAuthMethods['findPluginAuthsByKeys'] = jest.fn();
|
||||||
|
|
||||||
|
describe('getUserMCPAuthMap', () => {
|
||||||
|
beforeEach(() => {
|
||||||
|
jest.clearAllMocks();
|
||||||
|
});
|
||||||
|
|
||||||
|
describe('Core Functionality', () => {
|
||||||
|
it('should handle server names with various special characters and spaces', async () => {
|
||||||
|
const testCases = [
|
||||||
|
{
|
||||||
|
originalName: 'Connector: Company',
|
||||||
|
normalizedToolName: 'tool_mcp_Connector__Company',
|
||||||
|
},
|
||||||
|
{
|
||||||
|
originalName: 'Server (Production) @ Company.com',
|
||||||
|
normalizedToolName: 'tool_mcp_Server__Production____Company.com',
|
||||||
|
},
|
||||||
|
{
|
||||||
|
originalName: '🌟 Testing Server™ (α-β) 测试服务器',
|
||||||
|
normalizedToolName: 'tool_mcp_____Testing_Server_________',
|
||||||
|
},
|
||||||
|
];
|
||||||
|
|
||||||
|
const tools = testCases.map((testCase) =>
|
||||||
|
createMockTool(testCase.normalizedToolName, testCase.originalName),
|
||||||
|
);
|
||||||
|
|
||||||
|
const expectedKeys = testCases.map((tc) => `mcp_${tc.originalName}`);
|
||||||
|
mockGetPluginAuthMap.mockResolvedValue({});
|
||||||
|
|
||||||
|
await getUserMCPAuthMap({
|
||||||
|
userId: 'user123',
|
||||||
|
tools,
|
||||||
|
findPluginAuthsByKeys: mockFindPluginAuthsByKeys,
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(mockGetPluginAuthMap).toHaveBeenCalledWith({
|
||||||
|
userId: 'user123',
|
||||||
|
pluginKeys: expectedKeys,
|
||||||
|
throwError: false,
|
||||||
|
findPluginAuthsByKeys: mockFindPluginAuthsByKeys,
|
||||||
|
});
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe('Edge Cases', () => {
|
||||||
|
it('should return empty object when no tools have mcpRawServerName', async () => {
|
||||||
|
const tools = [
|
||||||
|
createMockTool('regular_tool', undefined, false),
|
||||||
|
createMockTool('another_tool', undefined, false),
|
||||||
|
createMockTool('test_mcp_Server_no_raw_name', undefined),
|
||||||
|
];
|
||||||
|
|
||||||
|
const result = await getUserMCPAuthMap({
|
||||||
|
userId: 'user123',
|
||||||
|
tools,
|
||||||
|
findPluginAuthsByKeys: mockFindPluginAuthsByKeys,
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(result).toEqual({});
|
||||||
|
expect(mockGetPluginAuthMap).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should handle empty or undefined tools array', async () => {
|
||||||
|
let result = await getUserMCPAuthMap({
|
||||||
|
userId: 'user123',
|
||||||
|
tools: [],
|
||||||
|
findPluginAuthsByKeys: mockFindPluginAuthsByKeys,
|
||||||
|
});
|
||||||
|
expect(result).toEqual({});
|
||||||
|
expect(mockGetPluginAuthMap).not.toHaveBeenCalled();
|
||||||
|
|
||||||
|
result = await getUserMCPAuthMap({
|
||||||
|
userId: 'user123',
|
||||||
|
tools: undefined,
|
||||||
|
findPluginAuthsByKeys: mockFindPluginAuthsByKeys,
|
||||||
|
});
|
||||||
|
expect(result).toEqual({});
|
||||||
|
expect(mockGetPluginAuthMap).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should handle database errors gracefully', async () => {
|
||||||
|
const tools = [createMockTool('test_mcp_Server1', 'Server1')];
|
||||||
|
const dbError = new Error('Database connection failed');
|
||||||
|
|
||||||
|
mockGetPluginAuthMap.mockRejectedValue(dbError);
|
||||||
|
|
||||||
|
const result = await getUserMCPAuthMap({
|
||||||
|
userId: 'user123',
|
||||||
|
tools,
|
||||||
|
findPluginAuthsByKeys: mockFindPluginAuthsByKeys,
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(result).toEqual({});
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should handle non-Error exceptions gracefully', async () => {
|
||||||
|
const tools = [createMockTool('test_mcp_Server1', 'Server1')];
|
||||||
|
|
||||||
|
mockGetPluginAuthMap.mockRejectedValue('String error');
|
||||||
|
|
||||||
|
const result = await getUserMCPAuthMap({
|
||||||
|
userId: 'user123',
|
||||||
|
tools,
|
||||||
|
findPluginAuthsByKeys: mockFindPluginAuthsByKeys,
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(result).toEqual({});
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe('Integration', () => {
|
||||||
|
it('should handle complete workflow with normalized tool names and original server names', async () => {
|
||||||
|
const originalServerName = 'Connector: Company';
|
||||||
|
const toolName = 'test_auth_mcp_Connector__Company';
|
||||||
|
|
||||||
|
const tools = [createMockTool(toolName, originalServerName)];
|
||||||
|
|
||||||
|
const mockCustomUserVars = {
|
||||||
|
'mcp_Connector: Company': {
|
||||||
|
API_KEY: 'test123',
|
||||||
|
SECRET_TOKEN: 'secret456',
|
||||||
|
},
|
||||||
|
};
|
||||||
|
|
||||||
|
mockGetPluginAuthMap.mockResolvedValue(mockCustomUserVars);
|
||||||
|
|
||||||
|
const result = await getUserMCPAuthMap({
|
||||||
|
userId: 'user123',
|
||||||
|
tools,
|
||||||
|
findPluginAuthsByKeys: mockFindPluginAuthsByKeys,
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(mockGetPluginAuthMap).toHaveBeenCalledWith({
|
||||||
|
userId: 'user123',
|
||||||
|
pluginKeys: ['mcp_Connector: Company'],
|
||||||
|
throwError: false,
|
||||||
|
findPluginAuthsByKeys: mockFindPluginAuthsByKeys,
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(result).toEqual(mockCustomUserVars);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
@ -3,17 +3,14 @@ import { Constants } from 'librechat-data-provider';
|
||||||
import type { PluginAuthMethods } from '@librechat/data-schemas';
|
import type { PluginAuthMethods } from '@librechat/data-schemas';
|
||||||
import type { GenericTool } from '@librechat/agents';
|
import type { GenericTool } from '@librechat/agents';
|
||||||
import { getPluginAuthMap } from '~/agents/auth';
|
import { getPluginAuthMap } from '~/agents/auth';
|
||||||
import { mcpToolPattern } from './utils';
|
|
||||||
|
|
||||||
export async function getUserMCPAuthMap({
|
export async function getUserMCPAuthMap({
|
||||||
userId,
|
userId,
|
||||||
tools,
|
tools,
|
||||||
appTools,
|
|
||||||
findPluginAuthsByKeys,
|
findPluginAuthsByKeys,
|
||||||
}: {
|
}: {
|
||||||
userId: string;
|
userId: string;
|
||||||
tools: GenericTool[] | undefined;
|
tools: GenericTool[] | undefined;
|
||||||
appTools: Record<string, unknown>;
|
|
||||||
findPluginAuthsByKeys: PluginAuthMethods['findPluginAuthsByKeys'];
|
findPluginAuthsByKeys: PluginAuthMethods['findPluginAuthsByKeys'];
|
||||||
}) {
|
}) {
|
||||||
if (!tools || tools.length === 0) {
|
if (!tools || tools.length === 0) {
|
||||||
|
|
@ -23,11 +20,9 @@ export async function getUserMCPAuthMap({
|
||||||
const uniqueMcpServers = new Set<string>();
|
const uniqueMcpServers = new Set<string>();
|
||||||
|
|
||||||
for (const tool of tools) {
|
for (const tool of tools) {
|
||||||
const toolKey = tool.name;
|
const mcpTool = tool as GenericTool & { mcpRawServerName?: string };
|
||||||
if (toolKey && appTools[toolKey] && mcpToolPattern.test(toolKey)) {
|
if (mcpTool.mcpRawServerName) {
|
||||||
const parts = toolKey.split(Constants.mcp_delimiter);
|
uniqueMcpServers.add(`${Constants.mcp_prefix}${mcpTool.mcpRawServerName}`);
|
||||||
const serverName = parts[parts.length - 1];
|
|
||||||
uniqueMcpServers.add(`${Constants.mcp_prefix}${serverName}`);
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue