mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-08-27 04:07:05 +00:00
🛰️ fix: Attach Request-Scoped MCP Servers (#14780)
* fix: attach request-scoped MCP servers * fix: satisfy MCP static checks * fix: format MCP runtime hint
This commit is contained in:
parent
c44d11ebf4
commit
1a3e2aebcb
26 changed files with 861 additions and 175 deletions
|
|
@ -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);
|
||||
|
|
|
|||
|
|
@ -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 = {
|
||||
|
|
|
|||
|
|
@ -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}`);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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({
|
||||
|
|
|
|||
|
|
@ -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 };
|
||||
|
|
|
|||
|
|
@ -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 };
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue