🧑‍🏫 fix: Multi-Agent Instructions Handling (#11484)

* 🧑‍🏫 fix: Multi-Agent Instructions Handling

* Refactored AgentClient to streamline the process of building messages by applying shared run context and agent-specific instructions.
* Introduced new utility functions in context.ts for extracting MCP server names, fetching MCP instructions, and building combined agent instructions.
* Updated the Agent type to make instructions optional, allowing for more flexible agent configurations.
* Improved the handling of context application to agents, ensuring that all relevant information is correctly integrated before execution.

* chore: Update EphemeralAgent Type in Context

* Enhanced the context.ts file by importing the TEphemeralAgent type from librechat-data-provider.
* Updated the applyContextToAgent function to use TEphemeralAgent for the ephemeralAgent parameter, improving type safety and clarity in agent context handling.

* ci: Update Agent Instructions in Tests for Clarity

* Revised test assertions in AgentClient to clarify the source of agent instructions, ensuring they are explicitly referenced as coming from agent configuration rather than build options.
* Updated comments in tests to enhance understanding of the expected behavior regarding base agent instructions and their handling in various scenarios.

* ci: Unit Tests for Agent Context Utilities

* Introduced comprehensive unit tests for agent context utilities, including functions for extracting MCP servers, fetching MCP instructions, and building agent instructions.
* Enhanced test coverage to ensure correct behavior across various scenarios, including handling of empty tools, mixed tool types, and error cases.
* Improved type definitions for AgentWithTools to clarify the structure and requirements for agent context operations.
This commit is contained in:
Danny Avila 2026-01-22 19:36:06 -05:00 committed by GitHub
parent 7204e74390
commit cfd5c793a9
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 758 additions and 86 deletions

View file

@ -0,0 +1,528 @@
import { z } from 'zod';
import { Constants } from 'librechat-data-provider';
import { DynamicStructuredTool } from '@langchain/core/tools';
import type { Logger } from 'winston';
import type { MCPManager } from '~/mcp/MCPManager';
import {
extractMCPServers,
getMCPInstructionsForServers,
buildAgentInstructions,
applyContextToAgent,
} from './context';
// Test schema for DynamicStructuredTool
const testSchema = z.object({});
describe('Agent Context Utilities', () => {
describe('extractMCPServers', () => {
it('should return empty array when agent has no tools', () => {
const agent = { id: 'test-agent' };
expect(extractMCPServers(agent)).toEqual([]);
});
it('should return empty array when agent tools array is empty', () => {
const agent = { id: 'test-agent', tools: [] };
expect(extractMCPServers(agent)).toEqual([]);
});
it('should extract unique MCP server names from tools', () => {
const tool1 = new DynamicStructuredTool({
name: `tool1${Constants.mcp_delimiter}server1`,
description: 'Test tool 1',
schema: testSchema,
func: async () => 'result',
});
const tool2 = new DynamicStructuredTool({
name: `tool2${Constants.mcp_delimiter}server2`,
description: 'Test tool 2',
schema: testSchema,
func: async () => 'result',
});
const agent = { id: 'test-agent', tools: [tool1, tool2] };
const result = extractMCPServers(agent);
expect(result).toContain('server1');
expect(result).toContain('server2');
expect(result).toHaveLength(2);
});
it('should return unique server names when multiple tools use same server', () => {
const tool1 = new DynamicStructuredTool({
name: `tool1${Constants.mcp_delimiter}server1`,
description: 'Test tool 1',
schema: testSchema,
func: async () => 'result',
});
const tool2 = new DynamicStructuredTool({
name: `tool2${Constants.mcp_delimiter}server1`,
description: 'Test tool 2',
schema: testSchema,
func: async () => 'result',
});
const agent = { id: 'test-agent', tools: [tool1, tool2] };
const result = extractMCPServers(agent);
expect(result).toEqual(['server1']);
expect(result).toHaveLength(1);
});
it('should ignore tools without MCP delimiter', () => {
const mcpTool = new DynamicStructuredTool({
name: `tool1${Constants.mcp_delimiter}server1`,
description: 'MCP tool',
schema: testSchema,
func: async () => 'result',
});
const regularTool = new DynamicStructuredTool({
name: 'regular_tool',
description: 'Regular tool',
schema: testSchema,
func: async () => 'result',
});
const agent = { id: 'test-agent', tools: [mcpTool, regularTool] };
const result = extractMCPServers(agent);
expect(result).toEqual(['server1']);
expect(result).toHaveLength(1);
});
it('should handle mixed tool types (string and DynamicStructuredTool)', () => {
const mcpTool = new DynamicStructuredTool({
name: `tool1${Constants.mcp_delimiter}server1`,
description: 'MCP tool',
schema: testSchema,
func: async () => 'result',
});
const agent = { id: 'test-agent', tools: [mcpTool, 'string-tool'] };
const result = extractMCPServers(agent);
expect(result).toEqual(['server1']);
});
it('should filter out empty server names', () => {
const toolWithEmptyServer = new DynamicStructuredTool({
name: `tool1${Constants.mcp_delimiter}`,
description: 'Tool with empty server',
schema: testSchema,
func: async () => 'result',
});
const agent = { id: 'test-agent', tools: [toolWithEmptyServer] };
const result = extractMCPServers(agent);
expect(result).toEqual([]);
});
});
describe('getMCPInstructionsForServers', () => {
let mockMCPManager: jest.Mocked<MCPManager>;
let mockLogger: Logger;
beforeEach(() => {
mockMCPManager = {
formatInstructionsForContext: jest.fn(),
} as unknown as jest.Mocked<MCPManager>;
mockLogger = {
debug: jest.fn(),
error: jest.fn(),
} as unknown as Logger;
});
it('should return empty string when server array is empty', async () => {
const result = await getMCPInstructionsForServers([], mockMCPManager, mockLogger);
expect(result).toBe('');
expect(mockMCPManager.formatInstructionsForContext).not.toHaveBeenCalled();
});
it('should fetch and return MCP instructions successfully', async () => {
const instructions = '# MCP Instructions\nUse these tools carefully';
mockMCPManager.formatInstructionsForContext.mockResolvedValue(instructions);
const result = await getMCPInstructionsForServers(
['server1', 'server2'],
mockMCPManager,
mockLogger,
);
expect(result).toBe(instructions);
expect(mockMCPManager.formatInstructionsForContext).toHaveBeenCalledWith([
'server1',
'server2',
]);
expect(mockLogger.debug).toHaveBeenCalledWith(
'[AgentContext] Fetched MCP instructions for servers:',
['server1', 'server2'],
);
});
it('should return empty string when MCP manager returns empty', async () => {
mockMCPManager.formatInstructionsForContext.mockResolvedValue('');
const result = await getMCPInstructionsForServers(['server1'], mockMCPManager, mockLogger);
expect(result).toBe('');
expect(mockLogger.debug).not.toHaveBeenCalled();
});
it('should handle errors gracefully and log them', async () => {
const error = new Error('MCP fetch failed');
mockMCPManager.formatInstructionsForContext.mockRejectedValue(error);
const result = await getMCPInstructionsForServers(['server1'], mockMCPManager, mockLogger);
expect(result).toBe('');
expect(mockLogger.error).toHaveBeenCalledWith(
'[AgentContext] Failed to get MCP instructions:',
error,
);
});
it('should work without logger', async () => {
const instructions = 'Test instructions';
mockMCPManager.formatInstructionsForContext.mockResolvedValue(instructions);
const result = await getMCPInstructionsForServers(['server1'], mockMCPManager);
expect(result).toBe(instructions);
// Should not throw even without logger
});
it('should handle errors without logger', async () => {
mockMCPManager.formatInstructionsForContext.mockRejectedValue(new Error('Test error'));
const result = await getMCPInstructionsForServers(['server1'], mockMCPManager);
expect(result).toBe('');
// Should not throw even without logger
});
});
describe('buildAgentInstructions', () => {
it('should combine all parts with double newlines', () => {
const result = buildAgentInstructions({
sharedRunContext: 'Shared context',
baseInstructions: 'Base instructions',
mcpInstructions: 'MCP instructions',
});
expect(result).toBe('Shared context\n\nBase instructions\n\nMCP instructions');
});
it('should filter out empty parts', () => {
const result = buildAgentInstructions({
sharedRunContext: 'Shared context',
baseInstructions: '',
mcpInstructions: 'MCP instructions',
});
expect(result).toBe('Shared context\n\nMCP instructions');
});
it('should return undefined when all parts are empty', () => {
const result = buildAgentInstructions({
sharedRunContext: '',
baseInstructions: '',
mcpInstructions: '',
});
expect(result).toBeUndefined();
});
it('should handle only shared context', () => {
const result = buildAgentInstructions({
sharedRunContext: 'Shared context only',
});
expect(result).toBe('Shared context only');
});
it('should handle only base instructions', () => {
const result = buildAgentInstructions({
baseInstructions: 'Base instructions only',
});
expect(result).toBe('Base instructions only');
});
it('should handle only MCP instructions', () => {
const result = buildAgentInstructions({
mcpInstructions: 'MCP instructions only',
});
expect(result).toBe('MCP instructions only');
});
it('should trim whitespace from combined result', () => {
const result = buildAgentInstructions({
sharedRunContext: ' Shared context ',
baseInstructions: ' Base instructions ',
});
expect(result).toBe('Shared context \n\n Base instructions');
});
it('should handle undefined parts', () => {
const result = buildAgentInstructions({
sharedRunContext: undefined,
baseInstructions: 'Base',
mcpInstructions: undefined,
});
expect(result).toBe('Base');
});
});
describe('applyContextToAgent', () => {
let mockMCPManager: jest.Mocked<MCPManager>;
let mockLogger: Logger;
beforeEach(() => {
mockMCPManager = {
formatInstructionsForContext: jest.fn(),
} as unknown as jest.Mocked<MCPManager>;
mockLogger = {
debug: jest.fn(),
error: jest.fn(),
} as unknown as Logger;
});
it('should apply context successfully with all components', async () => {
const agent = {
id: 'test-agent',
instructions: 'Original instructions',
tools: [
new DynamicStructuredTool({
name: `tool${Constants.mcp_delimiter}server1`,
description: 'Test tool',
schema: testSchema,
func: async () => 'result',
}),
],
};
mockMCPManager.formatInstructionsForContext.mockResolvedValue('MCP instructions');
await applyContextToAgent({
agent,
sharedRunContext: 'Shared context',
mcpManager: mockMCPManager,
agentId: 'test-agent',
logger: mockLogger,
});
expect(agent.instructions).toBe(
'Shared context\n\nOriginal instructions\n\nMCP instructions',
);
expect(mockLogger.debug).toHaveBeenCalledWith(
'[AgentContext] Applied context to agent: test-agent',
);
});
it('should use ephemeral agent MCP servers when provided', async () => {
const agent = {
id: 'test-agent',
instructions: 'Base instructions',
tools: [],
};
mockMCPManager.formatInstructionsForContext.mockResolvedValue('Ephemeral MCP');
await applyContextToAgent({
agent,
sharedRunContext: 'Context',
mcpManager: mockMCPManager,
ephemeralAgent: { mcp: ['ephemeral-server'] },
logger: mockLogger,
});
expect(mockMCPManager.formatInstructionsForContext).toHaveBeenCalledWith([
'ephemeral-server',
]);
expect(agent.instructions).toContain('Ephemeral MCP');
});
it('should prefer agent tools over empty ephemeral MCP array', async () => {
const agent = {
id: 'test-agent',
instructions: 'Base',
tools: [
new DynamicStructuredTool({
name: `tool${Constants.mcp_delimiter}agent-server`,
description: 'Test tool',
schema: testSchema,
func: async () => 'result',
}),
],
};
mockMCPManager.formatInstructionsForContext.mockResolvedValue('Agent MCP');
await applyContextToAgent({
agent,
sharedRunContext: '',
mcpManager: mockMCPManager,
ephemeralAgent: { mcp: [] },
logger: mockLogger,
});
expect(mockMCPManager.formatInstructionsForContext).toHaveBeenCalledWith(['agent-server']);
});
it('should work without agentId', async () => {
const agent = {
id: 'test-agent',
instructions: 'Base',
tools: [],
};
mockMCPManager.formatInstructionsForContext.mockResolvedValue('');
await applyContextToAgent({
agent,
sharedRunContext: 'Context',
mcpManager: mockMCPManager,
logger: mockLogger,
});
expect(agent.instructions).toBe('Context\n\nBase');
expect(mockLogger.debug).not.toHaveBeenCalled();
});
it('should work without logger', async () => {
const agent = {
id: 'test-agent',
instructions: 'Base',
tools: [
new DynamicStructuredTool({
name: `tool${Constants.mcp_delimiter}server1`,
description: 'Test tool',
schema: testSchema,
func: async () => 'result',
}),
],
};
mockMCPManager.formatInstructionsForContext.mockResolvedValue('MCP');
await applyContextToAgent({
agent,
sharedRunContext: 'Context',
mcpManager: mockMCPManager,
});
expect(agent.instructions).toBe('Context\n\nBase\n\nMCP');
});
it('should handle MCP fetch error gracefully and set fallback instructions', async () => {
const agent = {
id: 'test-agent',
instructions: 'Base instructions',
tools: [
new DynamicStructuredTool({
name: `tool${Constants.mcp_delimiter}server1`,
description: 'Test tool',
schema: testSchema,
func: async () => 'result',
}),
],
};
const error = new Error('MCP fetch failed');
mockMCPManager.formatInstructionsForContext.mockRejectedValue(error);
await applyContextToAgent({
agent,
sharedRunContext: 'Shared context',
mcpManager: mockMCPManager,
agentId: 'test-agent',
logger: mockLogger,
});
// getMCPInstructionsForServers catches the error and returns empty string
// So agent still has shared context + base instructions (without MCP)
expect(agent.instructions).toBe('Shared context\n\nBase instructions');
// Error is logged by getMCPInstructionsForServers, not applyContextToAgent
expect(mockLogger.error).toHaveBeenCalledWith(
'[AgentContext] Failed to get MCP instructions:',
error,
);
});
it('should handle invalid tools gracefully without throwing', async () => {
const agent = {
id: 'test-agent',
instructions: 'Base',
// eslint-disable-next-line @typescript-eslint/no-explicit-any
tools: null as any, // Invalid tools - should not crash
};
mockMCPManager.formatInstructionsForContext.mockResolvedValue('');
await applyContextToAgent({
agent,
sharedRunContext: 'Context',
mcpManager: mockMCPManager,
logger: mockLogger,
});
// extractMCPServers handles null tools gracefully, returns []
// getMCPInstructionsForServers returns early with '', so no MCP instructions
// Agent should still have shared context + base instructions
expect(agent.instructions).toBe('Context\n\nBase');
expect(mockMCPManager.formatInstructionsForContext).not.toHaveBeenCalled();
});
it('should preserve empty base instructions', async () => {
const agent = {
id: 'test-agent',
instructions: '',
tools: [
new DynamicStructuredTool({
name: `tool${Constants.mcp_delimiter}server1`,
description: 'Test tool',
schema: testSchema,
func: async () => 'result',
}),
],
};
mockMCPManager.formatInstructionsForContext.mockResolvedValue('MCP only');
await applyContextToAgent({
agent,
sharedRunContext: 'Shared',
mcpManager: mockMCPManager,
});
expect(agent.instructions).toBe('Shared\n\nMCP only');
});
it('should handle missing instructions field on agent', async () => {
const agent = {
id: 'test-agent',
instructions: undefined,
tools: [],
};
mockMCPManager.formatInstructionsForContext.mockResolvedValue('');
await applyContextToAgent({
agent,
sharedRunContext: 'Context',
mcpManager: mockMCPManager,
});
expect(agent.instructions).toBe('Context');
});
});
});

View file

@ -0,0 +1,148 @@
import { DynamicStructuredTool } from '@langchain/core/tools';
import { Constants } from 'librechat-data-provider';
import type { Agent, TEphemeralAgent } from 'librechat-data-provider';
import type { Logger } from 'winston';
import type { MCPManager } from '~/mcp/MCPManager';
/**
* Agent type with optional tools array that can contain DynamicStructuredTool or string.
* For context operations, we only require id and instructions, other Agent fields are optional.
*/
export type AgentWithTools = Pick<Agent, 'id'> &
Partial<Omit<Agent, 'id' | 'tools'>> & {
tools?: Array<DynamicStructuredTool | string>;
};
/**
* Extracts unique MCP server names from an agent's tools.
* @param agent - The agent with tools
* @returns Array of unique MCP server names
*/
export function extractMCPServers(agent: AgentWithTools): string[] {
if (!agent?.tools?.length) {
return [];
}
const mcpServers = new Set<string>();
for (let i = 0; i < agent.tools.length; i++) {
const tool = agent.tools[i];
if (tool instanceof DynamicStructuredTool && tool.name.includes(Constants.mcp_delimiter)) {
const serverName = tool.name.split(Constants.mcp_delimiter).pop();
if (serverName) {
mcpServers.add(serverName);
}
}
}
return Array.from(mcpServers);
}
/**
* Fetches MCP instructions for the given server names.
* @param {string[]} mcpServers - Array of MCP server names
* @param {MCPManager} mcpManager - MCP manager instance
* @param {Logger} [logger] - Optional logger instance
* @returns {Promise<string>} MCP instructions string, empty if none
*/
export async function getMCPInstructionsForServers(
mcpServers: string[],
mcpManager: MCPManager,
logger?: Logger,
): Promise<string> {
if (!mcpServers.length) {
return '';
}
try {
const mcpInstructions = await mcpManager.formatInstructionsForContext(mcpServers);
if (mcpInstructions && logger) {
logger.debug('[AgentContext] Fetched MCP instructions for servers:', mcpServers);
}
return mcpInstructions || '';
} catch (error) {
if (logger) {
logger.error('[AgentContext] Failed to get MCP instructions:', error);
}
return '';
}
}
/**
* Builds final instructions for an agent by combining shared run context and agent-specific context.
* Order: sharedRunContext -> baseInstructions -> mcpInstructions
*
* @param {Object} params
* @param {string} [params.sharedRunContext] - Run-level context shared by all agents (file context, RAG, memory)
* @param {string} [params.baseInstructions] - Agent's base instructions
* @param {string} [params.mcpInstructions] - Agent's MCP server instructions
* @returns {string | undefined} Combined instructions, or undefined if empty
*/
export function buildAgentInstructions({
sharedRunContext,
baseInstructions,
mcpInstructions,
}: {
sharedRunContext?: string;
baseInstructions?: string;
mcpInstructions?: string;
}): string | undefined {
const parts = [sharedRunContext, baseInstructions, mcpInstructions].filter(Boolean);
const combined = parts.join('\n\n').trim();
return combined || undefined;
}
/**
* Applies run context and MCP instructions to an agent's configuration.
* Mutates the agent object in place.
*
* @param {Object} params
* @param {Agent} params.agent - The agent to update
* @param {string} params.sharedRunContext - Run-level shared context
* @param {MCPManager} params.mcpManager - MCP manager instance
* @param {Object} [params.ephemeralAgent] - Ephemeral agent config (for MCP override)
* @param {string} [params.agentId] - Agent ID for logging
* @param {Logger} [params.logger] - Optional logger instance
* @returns {Promise<void>}
*/
export async function applyContextToAgent({
agent,
sharedRunContext,
mcpManager,
ephemeralAgent,
agentId,
logger,
}: {
agent: AgentWithTools;
sharedRunContext: string;
mcpManager: MCPManager;
ephemeralAgent?: TEphemeralAgent;
agentId?: string;
logger?: Logger;
}): Promise<void> {
const baseInstructions = agent.instructions || '';
try {
const mcpServers = ephemeralAgent?.mcp?.length ? ephemeralAgent.mcp : extractMCPServers(agent);
const mcpInstructions = await getMCPInstructionsForServers(mcpServers, mcpManager, logger);
agent.instructions = buildAgentInstructions({
sharedRunContext,
baseInstructions,
mcpInstructions,
});
if (agentId && logger) {
logger.debug(`[AgentContext] Applied context to agent: ${agentId}`);
}
} catch (error) {
agent.instructions = buildAgentInstructions({
sharedRunContext,
baseInstructions,
mcpInstructions: '',
});
if (logger) {
logger.error(
`[AgentContext] Failed to apply context to agent${agentId ? ` ${agentId}` : ''}, using base instructions only:`,
error,
);
}
}
}

View file

@ -1,5 +1,6 @@
export * from './avatars';
export * from './chain';
export * from './context';
export * from './edges';
export * from './initialize';
export * from './legacy';