🔧 fix: Await MCP Instructions and Filter Malformed Tool Calls (#10485)

* fix: Await MCP instructions formatting in AgentClient

* fix: don't render or aggregate malformed tool calls

* fix: implement filter for malformed tool call content parts and add tests
This commit is contained in:
Danny Avila 2025-11-13 14:17:47 -05:00 committed by GitHub
parent aff3cd3667
commit cabc8afeac
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
10 changed files with 467 additions and 9 deletions

View file

@ -47,7 +47,7 @@
"@langchain/google-genai": "^0.2.13",
"@langchain/google-vertexai": "^0.2.13",
"@langchain/textsplitters": "^0.1.0",
"@librechat/agents": "^3.0.15",
"@librechat/agents": "^3.0.17",
"@librechat/api": "*",
"@librechat/data-schemas": "*",
"@microsoft/microsoft-graph-client": "^3.0.7",

View file

@ -13,6 +13,7 @@ const {
memoryInstructions,
getTransactionsConfig,
createMemoryProcessor,
filterMalformedContentParts,
} = require('@librechat/api');
const {
Callback,
@ -344,7 +345,7 @@ class AgentClient extends BaseClient {
if (mcpServers.length > 0) {
try {
const mcpInstructions = getMCPManager().formatInstructionsForContext(mcpServers);
const mcpInstructions = await getMCPManager().formatInstructionsForContext(mcpServers);
if (mcpInstructions) {
systemContent = [systemContent, mcpInstructions].filter(Boolean).join('\n\n');
logger.debug('[AgentClient] Injected MCP instructions for servers:', mcpServers);
@ -611,7 +612,7 @@ class AgentClient extends BaseClient {
userMCPAuthMap: opts.userMCPAuthMap,
abortController: opts.abortController,
});
return this.contentParts;
return filterMalformedContentParts(this.contentParts);
}
/**

View file

@ -14,6 +14,14 @@ jest.mock('@librechat/api', () => ({
...jest.requireActual('@librechat/api'),
}));
// Mock getMCPManager
const mockFormatInstructions = jest.fn();
jest.mock('~/config', () => ({
getMCPManager: jest.fn(() => ({
formatInstructionsForContext: mockFormatInstructions,
})),
}));
describe('AgentClient - titleConvo', () => {
let client;
let mockRun;
@ -1168,6 +1176,200 @@ describe('AgentClient - titleConvo', () => {
});
});
describe('buildMessages with MCP server instructions', () => {
let client;
let mockReq;
let mockRes;
let mockAgent;
let mockOptions;
beforeEach(() => {
jest.clearAllMocks();
// Reset the mock to default behavior
mockFormatInstructions.mockResolvedValue(
'# MCP Server Instructions\n\nTest MCP instructions here',
);
const { DynamicStructuredTool } = require('@langchain/core/tools');
// Create mock MCP tools with the delimiter pattern
const mockMCPTool1 = new DynamicStructuredTool({
name: `tool1${Constants.mcp_delimiter}server1`,
description: 'Test MCP tool 1',
schema: {},
func: async () => 'result',
});
const mockMCPTool2 = new DynamicStructuredTool({
name: `tool2${Constants.mcp_delimiter}server2`,
description: 'Test MCP tool 2',
schema: {},
func: async () => 'result',
});
mockAgent = {
id: 'agent-123',
endpoint: EModelEndpoint.openAI,
provider: EModelEndpoint.openAI,
instructions: 'Base agent instructions',
model_parameters: {
model: 'gpt-4',
},
tools: [mockMCPTool1, mockMCPTool2],
};
mockReq = {
user: {
id: 'user-123',
},
body: {
endpoint: EModelEndpoint.openAI,
},
config: {},
};
mockRes = {};
mockOptions = {
req: mockReq,
res: mockRes,
agent: mockAgent,
endpoint: EModelEndpoint.agents,
};
client = new AgentClient(mockOptions);
client.conversationId = 'convo-123';
client.responseMessageId = 'response-123';
client.shouldSummarize = false;
client.maxContextTokens = 4096;
});
it('should await MCP instructions and not include [object Promise] in agent instructions', async () => {
// Set specific return value for this test
mockFormatInstructions.mockResolvedValue(
'# MCP Server Instructions\n\nUse these tools carefully',
);
const messages = [
{
messageId: 'msg-1',
parentMessageId: null,
sender: 'User',
text: 'Hello',
isCreatedByUser: true,
},
];
await client.buildMessages(messages, null, {
instructions: 'Base instructions',
additional_instructions: null,
});
// Verify formatInstructionsForContext was called with correct server names
expect(mockFormatInstructions).toHaveBeenCalledWith(['server1', 'server2']);
// Verify the instructions do NOT contain [object Promise]
expect(client.options.agent.instructions).not.toContain('[object Promise]');
// Verify the instructions DO contain the MCP instructions
expect(client.options.agent.instructions).toContain('# MCP Server Instructions');
expect(client.options.agent.instructions).toContain('Use these tools carefully');
// Verify the base instructions are also included
expect(client.options.agent.instructions).toContain('Base instructions');
});
it('should handle MCP instructions with ephemeral agent', async () => {
// Set specific return value for this test
mockFormatInstructions.mockResolvedValue(
'# Ephemeral MCP Instructions\n\nSpecial ephemeral instructions',
);
// Set up ephemeral agent with MCP servers
mockReq.body.ephemeralAgent = {
mcp: ['ephemeral-server1', 'ephemeral-server2'],
};
const messages = [
{
messageId: 'msg-1',
parentMessageId: null,
sender: 'User',
text: 'Test ephemeral',
isCreatedByUser: true,
},
];
await client.buildMessages(messages, null, {
instructions: 'Ephemeral instructions',
additional_instructions: null,
});
// Verify formatInstructionsForContext was called with ephemeral server names
expect(mockFormatInstructions).toHaveBeenCalledWith([
'ephemeral-server1',
'ephemeral-server2',
]);
// Verify no [object Promise] in instructions
expect(client.options.agent.instructions).not.toContain('[object Promise]');
// Verify ephemeral MCP instructions are included
expect(client.options.agent.instructions).toContain('# Ephemeral MCP Instructions');
expect(client.options.agent.instructions).toContain('Special ephemeral instructions');
});
it('should handle empty MCP instructions gracefully', async () => {
// Set empty return value for this test
mockFormatInstructions.mockResolvedValue('');
const messages = [
{
messageId: 'msg-1',
parentMessageId: null,
sender: 'User',
text: 'Hello',
isCreatedByUser: true,
},
];
await client.buildMessages(messages, null, {
instructions: 'Base instructions only',
additional_instructions: null,
});
// Verify the instructions still work without MCP content
expect(client.options.agent.instructions).toBe('Base instructions only');
expect(client.options.agent.instructions).not.toContain('[object Promise]');
});
it('should handle MCP instructions error gracefully', async () => {
// Set error return for this test
mockFormatInstructions.mockRejectedValue(new Error('MCP error'));
const messages = [
{
messageId: 'msg-1',
parentMessageId: null,
sender: 'User',
text: 'Hello',
isCreatedByUser: true,
},
];
// Should not throw
await client.buildMessages(messages, null, {
instructions: 'Base instructions',
additional_instructions: null,
});
// Should still have base instructions without MCP content
expect(client.options.agent.instructions).toContain('Base instructions');
expect(client.options.agent.instructions).not.toContain('[object Promise]');
});
});
describe('runMemory method', () => {
let client;
let mockReq;

View file

@ -144,6 +144,7 @@ const Part = memo(
attachments={attachments}
auth={toolCall.auth}
expires_at={toolCall.expires_at}
isLast={isLast}
/>
);
} else if (toolCall.type === ToolCallTypes.CODE_INTERPRETER) {
@ -192,6 +193,7 @@ const Part = memo(
args={toolCall.function.arguments as string}
name={toolCall.function.name}
output={toolCall.function.output}
isLast={isLast}
/>
);
}

View file

@ -11,6 +11,7 @@ import { logger, cn } from '~/utils';
export default function ToolCall({
initialProgress = 0.1,
isLast = false,
isSubmitting,
name,
args: _args = '',
@ -19,6 +20,7 @@ export default function ToolCall({
auth,
}: {
initialProgress: number;
isLast?: boolean;
isSubmitting: boolean;
name: string;
args: string | Record<string, unknown>;
@ -155,6 +157,10 @@ export default function ToolCall({
};
}, [showInfo, isAnimating]);
if (!isLast && (!function_name || function_name.length === 0) && !output) {
return null;
}
return (
<>
<div className="relative my-2.5 flex h-5 shrink-0 items-center gap-2.5">

10
package-lock.json generated
View file

@ -63,7 +63,7 @@
"@langchain/google-genai": "^0.2.13",
"@langchain/google-vertexai": "^0.2.13",
"@langchain/textsplitters": "^0.1.0",
"@librechat/agents": "^3.0.15",
"@librechat/agents": "^3.0.17",
"@librechat/api": "*",
"@librechat/data-schemas": "*",
"@microsoft/microsoft-graph-client": "^3.0.7",
@ -16666,9 +16666,9 @@
}
},
"node_modules/@librechat/agents": {
"version": "3.0.15",
"resolved": "https://registry.npmjs.org/@librechat/agents/-/agents-3.0.15.tgz",
"integrity": "sha512-iJyUZnfZrPgCeuhCEMOTJW5rp2z6QBsERwytNAr6haSXKiN4F7A6yfzJUkSTTLAu49+xoMIANAOMi8POaFdZ9Q==",
"version": "3.0.17",
"resolved": "https://registry.npmjs.org/@librechat/agents/-/agents-3.0.17.tgz",
"integrity": "sha512-gnom77oW10cdGmmQ6rExc+Blrfzib9JqrZk2fDPwkSOWXQIK4nMm6vWS+UkhH/YfN996mMnffpWoQQ6QQvkTGg==",
"license": "MIT",
"dependencies": {
"@langchain/anthropic": "^0.3.26",
@ -46470,7 +46470,7 @@
"@azure/storage-blob": "^12.27.0",
"@keyv/redis": "^4.3.3",
"@langchain/core": "^0.3.79",
"@librechat/agents": "^3.0.15",
"@librechat/agents": "^3.0.17",
"@librechat/data-schemas": "*",
"@modelcontextprotocol/sdk": "^1.21.0",
"axios": "^1.12.1",

View file

@ -83,7 +83,7 @@
"@azure/storage-blob": "^12.27.0",
"@keyv/redis": "^4.3.3",
"@langchain/core": "^0.3.79",
"@librechat/agents": "^3.0.15",
"@librechat/agents": "^3.0.17",
"@librechat/data-schemas": "*",
"@modelcontextprotocol/sdk": "^1.21.0",
"axios": "^1.12.1",

View file

@ -0,0 +1,200 @@
import { ContentTypes, ToolCallTypes } from 'librechat-data-provider';
import type { Agents, PartMetadata, TMessageContentParts } from 'librechat-data-provider';
import type { ToolCall } from '@langchain/core/messages/tool';
import { filterMalformedContentParts } from './content';
describe('filterMalformedContentParts', () => {
describe('basic filtering', () => {
it('should keep valid tool_call content parts', () => {
const parts: TMessageContentParts[] = [
{
type: ContentTypes.TOOL_CALL,
tool_call: {
id: 'test-id',
name: 'test_function',
type: ToolCallTypes.TOOL_CALL,
args: '{}',
progress: 1,
output: 'result',
},
},
];
const result = filterMalformedContentParts(parts);
expect(result).toHaveLength(1);
expect(result[0]).toEqual(parts[0]);
});
it('should filter out malformed tool_call content parts without tool_call property', () => {
const parts: TMessageContentParts[] = [
{ type: ContentTypes.TOOL_CALL } as TMessageContentParts,
];
const result = filterMalformedContentParts(parts);
expect(result).toHaveLength(0);
});
it('should keep other content types unchanged', () => {
const parts: TMessageContentParts[] = [
{ type: ContentTypes.TEXT, text: 'Hello world' },
{ type: ContentTypes.THINK, think: 'Thinking...' },
];
const result = filterMalformedContentParts(parts);
expect(result).toHaveLength(2);
expect(result).toEqual(parts);
});
it('should filter out null or undefined parts', () => {
const parts = [
{ type: ContentTypes.TEXT, text: 'Valid' },
null,
undefined,
{ type: ContentTypes.TEXT, text: 'Also valid' },
] as TMessageContentParts[];
const result = filterMalformedContentParts(parts);
expect(result).toHaveLength(2);
expect(result[0]).toHaveProperty('text', 'Valid');
expect(result[1]).toHaveProperty('text', 'Also valid');
});
it('should return non-array input unchanged', () => {
const notAnArray = { some: 'object' };
const result = filterMalformedContentParts(notAnArray);
expect(result).toBe(notAnArray);
});
});
describe('real-life example with multiple tool calls', () => {
it('should filter out malformed tool_call entries from actual MCP response', () => {
const parts: TMessageContentParts[] = [
{
type: ContentTypes.THINK,
think:
'The user is asking for 10 different time zones, similar to what would be displayed in a stock trading room floor.',
},
{
type: ContentTypes.TEXT,
text: '# Global Market Times\n\nShowing current time in 10 major financial centers:',
tool_call_ids: ['tooluse_Yjfib8PoRXCeCcHRH0JqCw'],
},
{
type: ContentTypes.TOOL_CALL,
tool_call: {
id: 'tooluse_Yjfib8PoRXCeCcHRH0JqCw',
name: 'get_current_time_mcp_time',
args: '{"timezone":"America/New_York"}',
type: ToolCallTypes.TOOL_CALL,
progress: 1,
output: '{"timezone":"America/New_York","datetime":"2025-11-13T13:43:17-05:00"}',
},
},
{ type: ContentTypes.TOOL_CALL } as TMessageContentParts,
{
type: ContentTypes.TOOL_CALL,
tool_call: {
id: 'tooluse_CPsGv9kXTrewVkcO7BEYIg',
name: 'get_current_time_mcp_time',
args: '{"timezone":"Europe/London"}',
type: ToolCallTypes.TOOL_CALL,
progress: 1,
output: '{"timezone":"Europe/London","datetime":"2025-11-13T18:43:19+00:00"}',
},
},
{ type: ContentTypes.TOOL_CALL } as TMessageContentParts,
{
type: ContentTypes.TOOL_CALL,
tool_call: {
id: 'tooluse_5jihRbd4TDWCGebwmAUlfQ',
name: 'get_current_time_mcp_time',
args: '{"timezone":"Asia/Tokyo"}',
type: ToolCallTypes.TOOL_CALL,
progress: 1,
output: '{"timezone":"Asia/Tokyo","datetime":"2025-11-14T03:43:21+09:00"}',
},
},
{ type: ContentTypes.TOOL_CALL } as TMessageContentParts,
{ type: ContentTypes.TOOL_CALL } as TMessageContentParts,
{ type: ContentTypes.TOOL_CALL } as TMessageContentParts,
{
type: ContentTypes.TEXT,
text: '## Major Financial Markets Clock:\n\n| Market | Local Time | Day |',
},
];
const result = filterMalformedContentParts(parts);
expect(result).toHaveLength(6);
expect(result[0].type).toBe(ContentTypes.THINK);
expect(result[1].type).toBe(ContentTypes.TEXT);
expect(result[2].type).toBe(ContentTypes.TOOL_CALL);
expect(result[3].type).toBe(ContentTypes.TOOL_CALL);
expect(result[4].type).toBe(ContentTypes.TOOL_CALL);
expect(result[5].type).toBe(ContentTypes.TEXT);
const toolCalls = result.filter((part) => part.type === ContentTypes.TOOL_CALL);
expect(toolCalls).toHaveLength(3);
toolCalls.forEach((toolCall) => {
if (toolCall.type === ContentTypes.TOOL_CALL) {
expect(toolCall.tool_call).toBeDefined();
expect(toolCall.tool_call).toHaveProperty('id');
expect(toolCall.tool_call).toHaveProperty('name');
}
});
});
it('should handle empty array', () => {
const result = filterMalformedContentParts([]);
expect(result).toEqual([]);
});
it('should handle array with only malformed tool calls', () => {
const parts = [
{ type: ContentTypes.TOOL_CALL },
{ type: ContentTypes.TOOL_CALL },
{ type: ContentTypes.TOOL_CALL },
] as TMessageContentParts[];
const result = filterMalformedContentParts(parts);
expect(result).toHaveLength(0);
});
});
describe('edge cases', () => {
it('should filter out tool_call with null tool_call property', () => {
const parts = [
{ type: ContentTypes.TOOL_CALL, tool_call: null as unknown as ToolCall },
] as TMessageContentParts[];
const result = filterMalformedContentParts(parts);
expect(result).toHaveLength(0);
});
it('should filter out tool_call with non-object tool_call property', () => {
const parts = [
{
type: ContentTypes.TOOL_CALL,
tool_call: 'not an object' as unknown as ToolCall & PartMetadata,
},
] as TMessageContentParts[];
const result = filterMalformedContentParts(parts);
expect(result).toHaveLength(0);
});
it('should keep tool_call with empty object as tool_call', () => {
const parts: TMessageContentParts[] = [
{
type: ContentTypes.TOOL_CALL,
tool_call: {} as unknown as Agents.ToolCall & PartMetadata,
},
];
const result = filterMalformedContentParts(parts);
expect(result).toHaveLength(1);
});
});
});

View file

@ -0,0 +1,46 @@
import { ContentTypes } from 'librechat-data-provider';
import type { TMessageContentParts } from 'librechat-data-provider';
/**
* Filters out malformed tool call content parts that don't have the required tool_call property.
* This handles edge cases where tool_call content parts may be created with only a type property
* but missing the actual tool_call data.
*
* @param contentParts - Array of content parts to filter
* @returns Filtered array with malformed tool calls removed
*
* @example
* // Removes malformed tool_call without the tool_call property
* const parts = [
* { type: 'tool_call', tool_call: { id: '123', name: 'test' } }, // valid - kept
* { type: 'tool_call' }, // invalid - filtered out
* { type: 'text', text: 'Hello' }, // valid - kept (other types pass through)
* ];
* const filtered = filterMalformedContentParts(parts);
* // Returns all parts except the malformed tool_call
*/
export function filterMalformedContentParts(
contentParts: TMessageContentParts[],
): TMessageContentParts[];
export function filterMalformedContentParts<T>(contentParts: T): T;
export function filterMalformedContentParts<T>(
contentParts: T | TMessageContentParts[],
): T | TMessageContentParts[] {
if (!Array.isArray(contentParts)) {
return contentParts;
}
return contentParts.filter((part) => {
if (!part || typeof part !== 'object') {
return false;
}
const { type } = part;
if (type === ContentTypes.TOOL_CALL) {
return 'tool_call' in part && part.tool_call != null && typeof part.tool_call === 'object';
}
return true;
});
}

View file

@ -1,6 +1,7 @@
export * from './axios';
export * from './azure';
export * from './common';
export * from './content';
export * from './email';
export * from './env';
export * from './events';