LibreChat/api/server/services/Tools/mcp.spec.js
Danny Avila 49859c04a2
🗄️ fix: Gate Request-Scoped MCP Servers Out of Persistent Tool Cache (#13672)
* 🗄️ 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.
2026-06-13 11:26:49 -04:00

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);
});
});