LibreChat/api/server/services/__tests__/ToolService.spec.js
Danny Avila 275af48592
🎯 fix: MCP Tool Misclassification from Action Delimiter Collision (#12512)
* fix: prevent MCP tools with `_action` in name from being misclassified as OpenAPI action tools

Add `isActionTool()` helper that checks for the `_action_` delimiter
while guarding against cross-delimiter collision with `_mcp_`. Replace
all `includes(actionDelimiter)` classification checks with the new
helper across backend and frontend.

* test: add coverage for MCP/action cross-delimiter collision

Verify that `isActionTool` correctly rejects MCP tool names containing
`_action` and that `loadAgentTools` does not filter them based on
`actionsEnabled`. Add ToolIcon and definitions test cases.

* fix: simplify isActionTool to handle all MCP name patterns

- Use `!toolName.includes('_mcp_')` instead of checking only after the
  first `_action_` occurrence, which missed MCP tools with `_action_` in
  the middle of their name (e.g. `get_action_data_mcp_myserver`).
- Reference `Constants.mcp_delimiter` value via a local const to avoid
  circular import from config.ts, with a comment explaining why.
- Remove dead `actionDelimiter` import from definitions.ts.
- Replace double-filter with single-pass partition in loadToolsForExecution.
- Add test for mid-name `_action_` collision case.

* fix: narrow MCP exclusion to delimiter position in isActionTool

Only reject when `_mcp_` appears after `_action_` (the MCP suffix
position). `_mcp_` before `_action_` is part of the operationId and
is valid — e.g. `sync_mcp_state_action_api---example---com` is a
legitimate action tool whose operationId happens to contain `_mcp_`.

* fix: document positional _mcp_ guard and known RFC-invalid domain limitation

Expand JSDoc on isActionTool to explain the action/MCP format
disambiguation and the theoretical false negative for non-RFC-compliant
domains containing `_mcp_`. Add test documenting this known edge case.
2026-04-01 22:36:21 -04:00

539 lines
20 KiB
JavaScript

const {
Tools,
Constants,
EModelEndpoint,
isActionTool,
actionDelimiter,
AgentCapabilities,
defaultAgentCapabilities,
} = require('librechat-data-provider');
const mockGetEndpointsConfig = jest.fn();
const mockGetMCPServerTools = jest.fn();
const mockGetCachedTools = jest.fn();
jest.mock('~/server/services/Config', () => ({
getEndpointsConfig: (...args) => mockGetEndpointsConfig(...args),
getMCPServerTools: (...args) => mockGetMCPServerTools(...args),
getCachedTools: (...args) => mockGetCachedTools(...args),
}));
const mockLoadToolDefinitions = jest.fn();
const mockGetUserMCPAuthMap = jest.fn();
jest.mock('@librechat/api', () => ({
...jest.requireActual('@librechat/api'),
loadToolDefinitions: (...args) => mockLoadToolDefinitions(...args),
getUserMCPAuthMap: (...args) => mockGetUserMCPAuthMap(...args),
}));
const mockLoadToolsUtil = jest.fn();
jest.mock('~/app/clients/tools/util', () => ({
loadTools: (...args) => mockLoadToolsUtil(...args),
}));
const mockLoadActionSets = jest.fn();
jest.mock('~/server/services/Tools/credentials', () => ({
loadAuthValues: jest.fn().mockResolvedValue({}),
}));
jest.mock('~/server/services/Tools/search', () => ({
createOnSearchResults: jest.fn(),
}));
jest.mock('~/server/services/Tools/mcp', () => ({
reinitMCPServer: jest.fn(),
}));
jest.mock('~/server/services/Files/process', () => ({
processFileURL: jest.fn(),
uploadImageBuffer: jest.fn(),
}));
jest.mock('~/app/clients/tools/util/fileSearch', () => ({
primeFiles: jest.fn().mockResolvedValue({}),
}));
jest.mock('~/server/services/Files/Code/process', () => ({
primeFiles: jest.fn().mockResolvedValue({}),
}));
jest.mock('../ActionService', () => ({
loadActionSets: (...args) => mockLoadActionSets(...args),
decryptMetadata: jest.fn(),
createActionTool: jest.fn(),
domainParser: jest.fn(),
}));
jest.mock('~/server/services/Threads', () => ({
recordUsage: jest.fn(),
}));
jest.mock('~/models', () => ({
findPluginAuthsByKeys: jest.fn(),
}));
jest.mock('~/config', () => ({
getFlowStateManager: jest.fn(() => ({})),
}));
jest.mock('~/server/services/MCP', () => ({
resolveConfigServers: jest.fn().mockResolvedValue({}),
}));
jest.mock('~/cache', () => ({
getLogStores: jest.fn(() => ({})),
}));
const {
loadAgentTools,
loadToolsForExecution,
resolveAgentCapabilities,
} = require('../ToolService');
function createMockReq(capabilities) {
return {
user: { id: 'user_123' },
config: {
endpoints: {
[EModelEndpoint.agents]: {
capabilities,
},
},
},
};
}
function createEndpointsConfig(capabilities) {
return {
[EModelEndpoint.agents]: { capabilities },
};
}
describe('ToolService - Action Capability Gating', () => {
beforeEach(() => {
jest.clearAllMocks();
mockLoadToolDefinitions.mockResolvedValue({
toolDefinitions: [],
toolRegistry: new Map(),
hasDeferredTools: false,
});
mockLoadToolsUtil.mockResolvedValue({ loadedTools: [], toolContextMap: {} });
mockLoadActionSets.mockResolvedValue([]);
});
describe('resolveAgentCapabilities', () => {
it('should return capabilities from endpoints config', async () => {
const capabilities = [AgentCapabilities.tools, AgentCapabilities.actions];
const req = createMockReq(capabilities);
mockGetEndpointsConfig.mockResolvedValue(createEndpointsConfig(capabilities));
const result = await resolveAgentCapabilities(req, req.config, 'agent_123');
expect(result).toBeInstanceOf(Set);
expect(result.has(AgentCapabilities.tools)).toBe(true);
expect(result.has(AgentCapabilities.actions)).toBe(true);
expect(result.has(AgentCapabilities.web_search)).toBe(false);
});
it('should fall back to default capabilities for ephemeral agents with empty config', async () => {
const req = createMockReq(defaultAgentCapabilities);
mockGetEndpointsConfig.mockResolvedValue({});
const result = await resolveAgentCapabilities(req, req.config, Constants.EPHEMERAL_AGENT_ID);
for (const cap of defaultAgentCapabilities) {
expect(result.has(cap)).toBe(true);
}
});
it('should return empty set when no capabilities and not ephemeral', async () => {
const req = createMockReq([]);
mockGetEndpointsConfig.mockResolvedValue({});
const result = await resolveAgentCapabilities(req, req.config, 'agent_123');
expect(result.size).toBe(0);
});
});
describe('isActionTool — cross-delimiter collision guard', () => {
it('should identify real action tools', () => {
expect(isActionTool(`get_weather${actionDelimiter}api_example_com`)).toBe(true);
expect(isActionTool(`fetch_data${actionDelimiter}my---domain---com`)).toBe(true);
});
it('should identify action tools whose operationId contains _mcp_', () => {
expect(isActionTool(`sync_mcp_state${actionDelimiter}api---example---com`)).toBe(true);
expect(isActionTool(`get_mcp_config${actionDelimiter}internal---api---com`)).toBe(true);
});
it('should reject MCP tools whose name ends with _action', () => {
expect(isActionTool(`get_action${Constants.mcp_delimiter}myserver`)).toBe(false);
expect(isActionTool(`fetch_action${Constants.mcp_delimiter}server_name`)).toBe(false);
expect(isActionTool(`retrieve_action${Constants.mcp_delimiter}srv`)).toBe(false);
});
it('should reject MCP tools with _action_ in the middle of their name', () => {
expect(isActionTool(`get_action_data${Constants.mcp_delimiter}myserver`)).toBe(false);
expect(isActionTool(`create_action_item${Constants.mcp_delimiter}server`)).toBe(false);
});
it('should reject tools without the action delimiter', () => {
expect(isActionTool('calculator')).toBe(false);
expect(isActionTool(`web_search${Constants.mcp_delimiter}myserver`)).toBe(false);
});
it('known limitation: non-RFC domain with _mcp_ substring yields false negative', () => {
// RFC 952/1123 prohibit underscores in hostnames, so this is not expected in practice.
// Encoded domain `api_mcp_internal_com` places `_mcp_` after `_action_`, which
// the guard interprets as the MCP suffix.
const edgeCaseTool = `getData${actionDelimiter}api_mcp_internal_com`;
expect(isActionTool(edgeCaseTool)).toBe(false);
});
});
describe('loadAgentTools (definitionsOnly=true) — action tool filtering', () => {
const actionToolName = `get_weather${actionDelimiter}api_example_com`;
const regularTool = 'calculator';
it('should exclude action tools from definitions when actions capability is disabled', async () => {
const capabilities = [AgentCapabilities.tools, AgentCapabilities.web_search];
const req = createMockReq(capabilities);
mockGetEndpointsConfig.mockResolvedValue(createEndpointsConfig(capabilities));
await loadAgentTools({
req,
res: {},
agent: { id: 'agent_123', tools: [regularTool, actionToolName] },
definitionsOnly: true,
});
expect(mockLoadToolDefinitions).toHaveBeenCalledTimes(1);
const [callArgs] = mockLoadToolDefinitions.mock.calls[0];
expect(callArgs.tools).toContain(regularTool);
expect(callArgs.tools).not.toContain(actionToolName);
});
it('should include action tools in definitions when actions capability is enabled', async () => {
const capabilities = [AgentCapabilities.tools, AgentCapabilities.actions];
const req = createMockReq(capabilities);
mockGetEndpointsConfig.mockResolvedValue(createEndpointsConfig(capabilities));
await loadAgentTools({
req,
res: {},
agent: { id: 'agent_123', tools: [regularTool, actionToolName] },
definitionsOnly: true,
});
expect(mockLoadToolDefinitions).toHaveBeenCalledTimes(1);
const [callArgs] = mockLoadToolDefinitions.mock.calls[0];
expect(callArgs.tools).toContain(regularTool);
expect(callArgs.tools).toContain(actionToolName);
});
it('should not filter MCP tools whose name contains _action (cross-delimiter collision)', async () => {
const mcpToolWithAction = `get_action${Constants.mcp_delimiter}myserver`;
const capabilities = [AgentCapabilities.tools];
const req = createMockReq(capabilities);
mockGetEndpointsConfig.mockResolvedValue(createEndpointsConfig(capabilities));
await loadAgentTools({
req,
res: {},
agent: { id: 'agent_123', tools: [regularTool, mcpToolWithAction] },
definitionsOnly: true,
});
expect(mockLoadToolDefinitions).toHaveBeenCalledTimes(1);
const [callArgs] = mockLoadToolDefinitions.mock.calls[0];
expect(callArgs.tools).toContain(mcpToolWithAction);
expect(callArgs.tools).toContain(regularTool);
});
it('should return actionsEnabled in the result', async () => {
const capabilities = [AgentCapabilities.tools];
const req = createMockReq(capabilities);
mockGetEndpointsConfig.mockResolvedValue(createEndpointsConfig(capabilities));
const result = await loadAgentTools({
req,
res: {},
agent: { id: 'agent_123', tools: [regularTool] },
definitionsOnly: true,
});
expect(result.actionsEnabled).toBe(false);
});
});
describe('loadAgentTools (definitionsOnly=false) — action tool filtering', () => {
const actionToolName = `get_weather${actionDelimiter}api_example_com`;
const regularTool = 'calculator';
it('should not load action sets when actions capability is disabled', async () => {
const capabilities = [AgentCapabilities.tools, AgentCapabilities.web_search];
const req = createMockReq(capabilities);
mockGetEndpointsConfig.mockResolvedValue(createEndpointsConfig(capabilities));
await loadAgentTools({
req,
res: {},
agent: { id: 'agent_123', tools: [regularTool, actionToolName] },
definitionsOnly: false,
});
expect(mockLoadActionSets).not.toHaveBeenCalled();
});
it('should load action sets when actions capability is enabled and action tools present', async () => {
const capabilities = [AgentCapabilities.tools, AgentCapabilities.actions];
const req = createMockReq(capabilities);
mockGetEndpointsConfig.mockResolvedValue(createEndpointsConfig(capabilities));
await loadAgentTools({
req,
res: {},
agent: { id: 'agent_123', tools: [regularTool, actionToolName] },
definitionsOnly: false,
});
expect(mockLoadActionSets).toHaveBeenCalledWith({ agent_id: 'agent_123' });
});
});
describe('loadToolsForExecution — action tool gating', () => {
const actionToolName = `get_weather${actionDelimiter}api_example_com`;
const regularTool = Tools.web_search;
it('should skip action tool loading when actionsEnabled=false', async () => {
const req = createMockReq([]);
req.config = {};
const result = await loadToolsForExecution({
req,
res: {},
agent: { id: 'agent_123' },
toolNames: [regularTool, actionToolName],
actionsEnabled: false,
});
expect(mockLoadActionSets).not.toHaveBeenCalled();
expect(result.loadedTools).toBeDefined();
});
it('should load action tools when actionsEnabled=true', async () => {
const req = createMockReq([AgentCapabilities.actions]);
req.config = {};
await loadToolsForExecution({
req,
res: {},
agent: { id: 'agent_123' },
toolNames: [actionToolName],
actionsEnabled: true,
});
expect(mockLoadActionSets).toHaveBeenCalledWith({ agent_id: 'agent_123' });
});
it('should resolve actionsEnabled from capabilities when not explicitly provided', async () => {
const capabilities = [AgentCapabilities.tools];
const req = createMockReq(capabilities);
mockGetEndpointsConfig.mockResolvedValue(createEndpointsConfig(capabilities));
await loadToolsForExecution({
req,
res: {},
agent: { id: 'agent_123' },
toolNames: [actionToolName],
});
expect(mockGetEndpointsConfig).toHaveBeenCalled();
expect(mockLoadActionSets).not.toHaveBeenCalled();
});
it('should not call loadActionSets when there are no action tools', async () => {
const req = createMockReq([AgentCapabilities.actions]);
req.config = {};
await loadToolsForExecution({
req,
res: {},
agent: { id: 'agent_123' },
toolNames: [regularTool],
actionsEnabled: true,
});
expect(mockLoadActionSets).not.toHaveBeenCalled();
});
});
describe('checkCapability logic', () => {
const createCheckCapability = (enabledCapabilities, logger = { warn: jest.fn() }) => {
return (capability) => {
const enabled = enabledCapabilities.has(capability);
if (!enabled) {
const isToolCapability = [
AgentCapabilities.file_search,
AgentCapabilities.execute_code,
AgentCapabilities.web_search,
].includes(capability);
const suffix = isToolCapability ? ' despite configured tool.' : '.';
logger.warn(`Capability "${capability}" disabled${suffix}`);
}
return enabled;
};
};
it('should return true when capability is enabled', () => {
const enabledCapabilities = new Set([AgentCapabilities.deferred_tools]);
const checkCapability = createCheckCapability(enabledCapabilities);
expect(checkCapability(AgentCapabilities.deferred_tools)).toBe(true);
});
it('should return false when capability is not enabled', () => {
const enabledCapabilities = new Set([]);
const checkCapability = createCheckCapability(enabledCapabilities);
expect(checkCapability(AgentCapabilities.deferred_tools)).toBe(false);
});
it('should log warning with "despite configured tool" for tool capabilities', () => {
const logger = { warn: jest.fn() };
const enabledCapabilities = new Set([]);
const checkCapability = createCheckCapability(enabledCapabilities, logger);
checkCapability(AgentCapabilities.file_search);
expect(logger.warn).toHaveBeenCalledWith(expect.stringContaining('despite configured tool'));
logger.warn.mockClear();
checkCapability(AgentCapabilities.execute_code);
expect(logger.warn).toHaveBeenCalledWith(expect.stringContaining('despite configured tool'));
logger.warn.mockClear();
checkCapability(AgentCapabilities.web_search);
expect(logger.warn).toHaveBeenCalledWith(expect.stringContaining('despite configured tool'));
});
it('should log warning without "despite configured tool" for non-tool capabilities', () => {
const logger = { warn: jest.fn() };
const enabledCapabilities = new Set([]);
const checkCapability = createCheckCapability(enabledCapabilities, logger);
checkCapability(AgentCapabilities.deferred_tools);
expect(logger.warn).toHaveBeenCalledWith(
expect.stringContaining('Capability "deferred_tools" disabled.'),
);
expect(logger.warn).not.toHaveBeenCalledWith(
expect.stringContaining('despite configured tool'),
);
logger.warn.mockClear();
checkCapability(AgentCapabilities.tools);
expect(logger.warn).toHaveBeenCalledWith(
expect.stringContaining('Capability "tools" disabled.'),
);
expect(logger.warn).not.toHaveBeenCalledWith(
expect.stringContaining('despite configured tool'),
);
logger.warn.mockClear();
checkCapability(AgentCapabilities.actions);
expect(logger.warn).toHaveBeenCalledWith(
expect.stringContaining('Capability "actions" disabled.'),
);
});
it('should not log warning when capability is enabled', () => {
const logger = { warn: jest.fn() };
const enabledCapabilities = new Set([
AgentCapabilities.deferred_tools,
AgentCapabilities.file_search,
]);
const checkCapability = createCheckCapability(enabledCapabilities, logger);
checkCapability(AgentCapabilities.deferred_tools);
checkCapability(AgentCapabilities.file_search);
expect(logger.warn).not.toHaveBeenCalled();
});
});
describe('defaultAgentCapabilities', () => {
it('should include deferred_tools capability by default', () => {
expect(defaultAgentCapabilities).toContain(AgentCapabilities.deferred_tools);
});
it('should include all expected default capabilities', () => {
expect(defaultAgentCapabilities).toContain(AgentCapabilities.execute_code);
expect(defaultAgentCapabilities).toContain(AgentCapabilities.file_search);
expect(defaultAgentCapabilities).toContain(AgentCapabilities.web_search);
expect(defaultAgentCapabilities).toContain(AgentCapabilities.artifacts);
expect(defaultAgentCapabilities).toContain(AgentCapabilities.actions);
expect(defaultAgentCapabilities).toContain(AgentCapabilities.context);
expect(defaultAgentCapabilities).toContain(AgentCapabilities.tools);
expect(defaultAgentCapabilities).toContain(AgentCapabilities.chain);
expect(defaultAgentCapabilities).toContain(AgentCapabilities.ocr);
});
});
describe('userMCPAuthMap gating', () => {
const shouldFetchMCPAuth = (tools) =>
tools?.some((t) => t.includes(Constants.mcp_delimiter)) ?? false;
it('should return true when agent has MCP tools', () => {
const tools = ['web_search', `search${Constants.mcp_delimiter}my-mcp-server`, 'calculator'];
expect(shouldFetchMCPAuth(tools)).toBe(true);
});
it('should return false when agent has no MCP tools', () => {
const tools = ['web_search', 'calculator', 'code_interpreter'];
expect(shouldFetchMCPAuth(tools)).toBe(false);
});
it('should return false when tools is empty', () => {
expect(shouldFetchMCPAuth([])).toBe(false);
});
it('should return false when tools is undefined', () => {
expect(shouldFetchMCPAuth(undefined)).toBe(false);
});
it('should return false when tools is null', () => {
expect(shouldFetchMCPAuth(null)).toBe(false);
});
it('should detect MCP tools with different server names', () => {
const tools = [
`listFiles${Constants.mcp_delimiter}file-server`,
`query${Constants.mcp_delimiter}db-server`,
];
expect(shouldFetchMCPAuth(tools)).toBe(true);
});
it('should return true even when only one tool is MCP', () => {
const tools = [
'web_search',
'calculator',
'code_interpreter',
`echo${Constants.mcp_delimiter}test-server`,
];
expect(shouldFetchMCPAuth(tools)).toBe(true);
});
});
describe('deferredToolsEnabled integration', () => {
it('should correctly determine deferredToolsEnabled from capabilities set', () => {
const createCheckCapability = (enabledCapabilities) => {
return (capability) => enabledCapabilities.has(capability);
};
const withDeferred = new Set([AgentCapabilities.deferred_tools, AgentCapabilities.tools]);
const checkWithDeferred = createCheckCapability(withDeferred);
expect(checkWithDeferred(AgentCapabilities.deferred_tools)).toBe(true);
const withoutDeferred = new Set([AgentCapabilities.tools, AgentCapabilities.actions]);
const checkWithoutDeferred = createCheckCapability(withoutDeferred);
expect(checkWithoutDeferred(AgentCapabilities.deferred_tools)).toBe(false);
});
it('should use defaultAgentCapabilities when no capabilities configured', () => {
const endpointsConfig = {};
const enabledCapabilities = new Set(
endpointsConfig?.capabilities ?? defaultAgentCapabilities,
);
expect(enabledCapabilities.has(AgentCapabilities.deferred_tools)).toBe(true);
});
});
});