mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-06-28 18:31:24 +00:00
* 🗄️ fix: Gate Request-Scoped MCP Servers Out of Persistent Tool Cache PR #13626 established that request-scoped MCP servers (runtime OPENID/GRAPH/BODY placeholders) must not use the persistent 12h tool cache, but only gated three of five touchpoints. The panel endpoint still back-filled the cache and the OAuth callback still wrote to it, while agent loading read those entries ungated — pinning ephemeral model-spec/agent toolsets to stale definitions for up to 12h. Centralize the invariant in createMCPToolCacheService: a getServerConfig resolver dep gates both writers and a new service-owned getMCPServerTools read, so every current and future caller is covered. Callers that already hold the parsed config pass it to skip resolution; the per-call skipCache flag and duplicated call-site gates are removed in favor of the single config-based mechanism. Resolution failures fail open to preserve prior behavior. * 🩹 fix: Address Codex Review on Cache Gating - Repair getCachedTools.spec.js, which destructured the relocated getMCPServerTools directly from the module; its coverage now lives in the service-level tools.spec.ts. - Resolve the merged (Config-tier-aware) server config in the OAuth callback before writing tool definitions, so the cache gate detects request-scoped servers supplied via admin Config overlays that the base registry lookup cannot see. - Discover tools actively for request-scoped servers in the panel endpoint via ephemeral reinitialization: such servers have no stored app/user connections, so the previous getServerToolFunctions fallback returned an empty toolset once the cache read was gated. * 🧵 fix: Address Second Codex Review on Cache Gating - Resolve the merged server config before the OAuth callback reconnects, so the connection itself uses Config-tier overlays rather than only the subsequent cache write. - Pass Config-tier candidates into the panel's request-scoped discovery, matching the reinitialize route: reinitMCPServer forwards configServers (not the provided serverConfig) to its OAuth discovery fallback. - Document the accepted read-path trade-off: the gate resolver sees base configs only, all writers pass merged configs, so a pre-gating or overlay-divergent entry survives at most one cache TTL. * 🚏 chore: Rework Cache Gating for BODY-Only Request Scoping After #13673 narrowed requiresEphemeralUserConnection to BODY placeholders, the central gate follows the predicate unchanged, but the panel's active discovery no longer serves a purpose: the only remaining request-scoped class cannot connect outside a chat turn, so the reinitialization attempt would always fail at the missing-body check. Remove that path; OpenID/Graph servers are persistent user-scoped again and flow through the stored-connection and cache lookups as before. Flip test fixtures that used OPENID placeholders to denote request-scoped configs over to BODY placeholders. * 🪟 fix: Check Config Overlays in Agent-Loading Cache Reads The cache service's registry resolver sees only base YAML/DB configs, so a BODY placeholder introduced by a request-tier Config overlay was invisible to the gate on the agent-loading read path: model-spec and ephemeral-agent expansion could read a leftover persistent entry and pin stale concrete tool names instead of the mcp_all fresh-discovery path. Check the raw overlay candidate inline in loadEphemeralAgent and loadAddedAgent — a pure placeholder scan with no extra IO — and skip the cache read when the overlay makes the server request-scoped. Widen UserScopedConnectionConfig so raw (pre-inspection) configs qualify for the scoping predicates, which only check key presence. * 🧪 test: Guard Run-Scoped MCP Definition Handoff Boundaries The original ClickHouse breaker storm regressed precisely at field pass-through boundaries that unit tests of each end could not see: initializeAgent dropping mcpAvailableTools from its destructure, and the agent tool context losing it on the way into ON_TOOL_EXECUTE. Add direct guards on both hops: the loadTools result must surface on the initialized agent, and the captured toolExecuteOptions closure must forward it to loadToolsForExecution.
186 lines
5.6 KiB
JavaScript
186 lines
5.6 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);
|
|
});
|
|
});
|