mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-08-04 14:57:42 +00:00
* fix: Attach Request-Scoped MCP Servers from the Agent Builder via mcp_all Follow-up to #14148 / #14074: request-scoped MCP servers (runtime {{LIBRECHAT_BODY_*}} placeholder headers) defer their connection on reinitialize, so their tools are never enumerable in the agent builder and the attach flow (which waits for isConnected && hasTools) silently attaches nothing. The runtime already resolves an mcp_all (sys__all__sys_mcp_<server>) tool entry into the server's full tool set at chat-turn time - the builder just never writes that token. - reinitMCPServer returns connectionDeferred: true on the deferred branch so clients can distinguish it from a plain empty success (server configs are sanitized client-side, so the response is the only reliable signal) - /mcp/:serverName/reinitialize forwards the flag; data-provider mutation type includes it - McpSection attaches [mcp_server, mcp_all] tokens on a deferred connect (idempotent) and shows a "tools are resolved at runtime" hint instead of "no tools yet" when wildcard-attached - selectors: mcpAllToken() helper beside mcpServerToken() Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix: address review — deferred attach via init state; strip stale wildcard Two review findings: 1. Servers with customUserVars route Connect through the config dialog, whose save path calls initializeServer inside the manager — the McpSection never awaits that response, so the deferred attach was unreachable. Record connectionDeferred in the shared per-server init state (MCPServerInitState) on every initialize attempt and key the attach off that state in the auto-select effect: one attach site now covers both the direct Connect and the config-dialog path. 2. updateFormTools kept an existing mcp_all wildcard when rewriting a per-tool selection, so a server that later exposes a normal tool list would still grant every tool at runtime while the UI showed a subset. The wildcard is now stripped unless explicitly re-passed, making per-tool selection always supersede it. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix: address review — stale deferred state; fold wildcard into display Second review round: 1. connectionDeferred persisted across attempts, so a later Connect click could attach the wildcard from a stale flag before the new attempt reported. Reset it at the start of every initializeServer call, and clear it before routing into the customUserVars config dialog (resetConnectionDeferred) so only the current attempt's outcome can trigger the auto-attach effect. 2. With a wildcard attached and the server's tools later enumerable, the dialog showed every tool unchecked while runtime granted all of them. getSelectedTools now folds the wildcard into the display (all tools selected); any selection interaction rewrites the form with concrete ids and drops the wildcard, converting the attachment on first touch. Also sorts imports in McpSection.tsx (CI sort-imports gate). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
285 lines
8.5 KiB
JavaScript
285 lines
8.5 KiB
JavaScript
const { Constants } = require('librechat-data-provider');
|
|
|
|
const mockGetConnection = jest.fn();
|
|
const mockDiscoverServerTools = jest.fn();
|
|
const mockGetGraphApiToken = jest.fn();
|
|
const mockUpdateMCPServerTools = jest.fn();
|
|
|
|
jest.mock('~/config', () => ({
|
|
getMCPManager: jest.fn(() => ({
|
|
getConnection: mockGetConnection,
|
|
discoverServerTools: mockDiscoverServerTools,
|
|
})),
|
|
getMCPServersRegistry: jest.fn(() => ({ getServerConfig: jest.fn() })),
|
|
getFlowStateManager: jest.fn(() => ({})),
|
|
}));
|
|
jest.mock('~/models', () => ({
|
|
findToken: jest.fn(),
|
|
createToken: jest.fn(),
|
|
updateToken: jest.fn(),
|
|
deleteTokens: jest.fn(),
|
|
}));
|
|
jest.mock('~/server/services/Config', () => ({
|
|
updateMCPServerTools: mockUpdateMCPServerTools,
|
|
}));
|
|
jest.mock('~/server/services/GraphTokenService', () => ({
|
|
getGraphApiToken: mockGetGraphApiToken,
|
|
}));
|
|
jest.mock('~/cache', () => ({
|
|
getLogStores: jest.fn(() => ({})),
|
|
}));
|
|
|
|
const { reinitMCPServer } = require('./mcp');
|
|
|
|
describe('reinitMCPServer — customUserVars gating (issue #10969)', () => {
|
|
const user = { id: 'user-123' };
|
|
const serverName = 'Thingy';
|
|
const serverConfig = {
|
|
type: 'streamable-http',
|
|
url: 'https://thingy.example.com/mcp',
|
|
customUserVars: {
|
|
THINGY_TOKEN: { title: 'Thingy Access Token', description: 'Create this in Thingy' },
|
|
},
|
|
};
|
|
|
|
beforeEach(() => {
|
|
jest.clearAllMocks();
|
|
mockUpdateMCPServerTools.mockResolvedValue({});
|
|
});
|
|
|
|
it('does not connect and exposes no tools when a required customUserVar is unset', async () => {
|
|
const result = await reinitMCPServer({
|
|
user,
|
|
serverName,
|
|
serverConfig,
|
|
userMCPAuthMap: undefined,
|
|
});
|
|
|
|
expect(mockGetConnection).not.toHaveBeenCalled();
|
|
expect(result).toMatchObject({
|
|
availableTools: null,
|
|
success: false,
|
|
tools: null,
|
|
oauthRequired: false,
|
|
serverName,
|
|
});
|
|
expect(result.message).toContain('THINGY_TOKEN');
|
|
});
|
|
|
|
it('does not connect when the stored value for a required customUserVar is empty', async () => {
|
|
const result = await reinitMCPServer({
|
|
user,
|
|
serverName,
|
|
serverConfig,
|
|
userMCPAuthMap: { [`${Constants.mcp_prefix}${serverName}`]: { THINGY_TOKEN: '' } },
|
|
});
|
|
|
|
expect(mockGetConnection).not.toHaveBeenCalled();
|
|
expect(result.success).toBe(false);
|
|
expect(result.availableTools).toBeNull();
|
|
});
|
|
|
|
it('proceeds to connect once every required customUserVar is provided', async () => {
|
|
mockGetConnection.mockResolvedValue({ fetchTools: jest.fn().mockResolvedValue([]) });
|
|
|
|
await reinitMCPServer({
|
|
user,
|
|
serverName,
|
|
serverConfig,
|
|
userMCPAuthMap: {
|
|
[`${Constants.mcp_prefix}${serverName}`]: { THINGY_TOKEN: 'secret-token' },
|
|
},
|
|
});
|
|
|
|
expect(mockGetConnection).toHaveBeenCalledTimes(1);
|
|
expect(mockGetConnection).toHaveBeenCalledWith(
|
|
expect.objectContaining({
|
|
serverName,
|
|
customUserVars: { THINGY_TOKEN: 'secret-token' },
|
|
}),
|
|
);
|
|
});
|
|
|
|
it('passes request body and Graph resolver into connection creation', async () => {
|
|
mockGetConnection.mockResolvedValue({ fetchTools: jest.fn().mockResolvedValue([]) });
|
|
const requestBody = { conversationId: 'conv-123', messageId: 'msg-123' };
|
|
|
|
await reinitMCPServer({
|
|
user,
|
|
serverName,
|
|
serverConfig: { type: 'streamable-http', url: 'https://thingy.example.com/mcp' },
|
|
requestBody,
|
|
userMCPAuthMap: undefined,
|
|
});
|
|
|
|
expect(mockGetConnection).toHaveBeenCalledWith(
|
|
expect.objectContaining({
|
|
requestBody,
|
|
graphTokenResolver: mockGetGraphApiToken,
|
|
}),
|
|
);
|
|
});
|
|
|
|
it('passes request body and Graph resolver into OAuth discovery fallback', async () => {
|
|
mockGetConnection.mockRejectedValue(new Error('OAuth authentication required'));
|
|
mockDiscoverServerTools.mockResolvedValue({ tools: [], oauthRequired: true, oauthUrl: null });
|
|
const requestBody = { conversationId: 'conv-456', messageId: 'msg-456' };
|
|
|
|
await reinitMCPServer({
|
|
user,
|
|
serverName,
|
|
serverConfig: { type: 'streamable-http', url: 'https://thingy.example.com/mcp' },
|
|
requestBody,
|
|
userMCPAuthMap: undefined,
|
|
});
|
|
|
|
expect(mockDiscoverServerTools).toHaveBeenCalledWith(
|
|
expect.objectContaining({
|
|
requestBody,
|
|
graphTokenResolver: mockGetGraphApiToken,
|
|
}),
|
|
);
|
|
});
|
|
|
|
it('disconnects ephemeral BODY-scoped connections after loading tools', async () => {
|
|
const disconnect = jest.fn().mockResolvedValue(undefined);
|
|
const tools = [{ name: 'search', inputSchema: { type: 'object', properties: {} } }];
|
|
const serverConfig = {
|
|
type: 'streamable-http',
|
|
url: 'https://thingy.example.com/messages/{{LIBRECHAT_BODY_MESSAGEID}}/mcp',
|
|
source: 'yaml',
|
|
};
|
|
mockGetConnection.mockResolvedValue({
|
|
disconnect,
|
|
fetchTools: jest.fn().mockResolvedValue(tools),
|
|
});
|
|
|
|
await reinitMCPServer({
|
|
user,
|
|
serverName,
|
|
serverConfig,
|
|
requestBody: { messageId: 'msg-789' },
|
|
userMCPAuthMap: undefined,
|
|
});
|
|
|
|
expect(disconnect).toHaveBeenCalledTimes(1);
|
|
expect(mockUpdateMCPServerTools).toHaveBeenCalledWith(
|
|
expect.objectContaining({
|
|
tools,
|
|
serverConfig,
|
|
}),
|
|
);
|
|
});
|
|
|
|
it('proceeds to connect when the server declares no customUserVars', async () => {
|
|
mockGetConnection.mockResolvedValue({ fetchTools: jest.fn().mockResolvedValue([]) });
|
|
|
|
await reinitMCPServer({
|
|
user,
|
|
serverName,
|
|
serverConfig: { type: 'streamable-http', url: 'https://thingy.example.com/mcp' },
|
|
userMCPAuthMap: undefined,
|
|
});
|
|
|
|
expect(mockGetConnection).toHaveBeenCalledTimes(1);
|
|
});
|
|
});
|
|
|
|
describe('reinitMCPServer — runtime BODY placeholder pre-check (issue #14074)', () => {
|
|
const user = { id: 'user-123' };
|
|
const serverName = 'Thingy';
|
|
const serverConfig = {
|
|
type: 'streamable-http',
|
|
url: 'https://thingy.example.com/mcp',
|
|
source: 'yaml',
|
|
headers: { 'X-Conversation-Id': '{{LIBRECHAT_BODY_CONVERSATIONID}}' },
|
|
};
|
|
|
|
beforeEach(() => {
|
|
jest.clearAllMocks();
|
|
mockUpdateMCPServerTools.mockResolvedValue({});
|
|
});
|
|
|
|
it('defers connection without failing when body placeholders cannot resolve outside a chat turn', async () => {
|
|
const result = await reinitMCPServer({
|
|
user,
|
|
serverName,
|
|
serverConfig,
|
|
userMCPAuthMap: undefined,
|
|
});
|
|
|
|
expect(mockGetConnection).not.toHaveBeenCalled();
|
|
expect(mockDiscoverServerTools).not.toHaveBeenCalled();
|
|
expect(result).toMatchObject({
|
|
availableTools: null,
|
|
success: true,
|
|
connectionDeferred: true,
|
|
tools: null,
|
|
oauthRequired: false,
|
|
serverName,
|
|
});
|
|
expect(result.message).toContain('first use in a chat turn');
|
|
});
|
|
|
|
it('treats an empty-string body field as missing', async () => {
|
|
const result = await reinitMCPServer({
|
|
user,
|
|
serverName,
|
|
serverConfig,
|
|
requestBody: { conversationId: ' ' },
|
|
userMCPAuthMap: undefined,
|
|
});
|
|
|
|
expect(mockGetConnection).not.toHaveBeenCalled();
|
|
expect(result.success).toBe(true);
|
|
});
|
|
|
|
it('connects normally when the request body provides the placeholder fields', async () => {
|
|
const disconnect = jest.fn().mockResolvedValue(undefined);
|
|
mockGetConnection.mockResolvedValue({
|
|
disconnect,
|
|
fetchTools: jest.fn().mockResolvedValue([]),
|
|
});
|
|
|
|
const result = await reinitMCPServer({
|
|
user,
|
|
serverName,
|
|
serverConfig,
|
|
requestBody: { conversationId: 'convo-1' },
|
|
userMCPAuthMap: undefined,
|
|
});
|
|
|
|
expect(mockGetConnection).toHaveBeenCalledTimes(1);
|
|
expect(result.connectionDeferred).toBeUndefined();
|
|
});
|
|
|
|
it('reports missing customUserVars before deferring on body placeholders', async () => {
|
|
const result = await reinitMCPServer({
|
|
user,
|
|
serverName,
|
|
serverConfig: {
|
|
...serverConfig,
|
|
customUserVars: { THINGY_TOKEN: { title: 'Thingy Access Token' } },
|
|
},
|
|
userMCPAuthMap: undefined,
|
|
});
|
|
|
|
expect(result.success).toBe(false);
|
|
expect(result.message).toContain('THINGY_TOKEN');
|
|
});
|
|
|
|
it('still treats unrelated connection errors as real failures', async () => {
|
|
mockGetConnection.mockRejectedValue(new Error('ECONNREFUSED'));
|
|
|
|
const result = await reinitMCPServer({
|
|
user,
|
|
serverName,
|
|
serverConfig: { type: 'streamable-http', url: 'https://thingy.example.com/mcp' },
|
|
userMCPAuthMap: undefined,
|
|
});
|
|
|
|
expect(mockDiscoverServerTools).not.toHaveBeenCalled();
|
|
expect(result.success).toBe(false);
|
|
expect(result.message).toBe(`Failed to reinitialize MCP server '${serverName}'`);
|
|
});
|
|
});
|