From e9e435b796b9088b7d03463466e2ed762181580d Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Sat, 18 Apr 2026 10:52:11 -0400 Subject: [PATCH] =?UTF-8?q?=F0=9F=A4=9D=20fix:=20load=20handoff=20sub-agen?= =?UTF-8?q?ts=20on=20OpenAI-compat=20endpoints=20(#12726)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extracts the BFS discovery + ACL-gated initialization of handoff sub-agents into a shared `discoverConnectedAgents` helper in `@librechat/api` and wires it into the OpenAI-compatible `/v1/chat/completions` and Open Responses `/v1/responses` controllers. These endpoints previously only passed the primary agent config to `createRun` while keeping `primaryConfig.edges` intact, which forced `MultiAgentGraph` into multi-agent mode without loading the referenced sub-agents and caused StateGraph to throw "Found edge ending at unknown node ". The discovery helper also filters orphaned edges (deleted sub-agents or those the caller lacks VIEW permission on), so API users see the same graceful fallback the chat UI already had. --- .../agents/__tests__/openai.spec.js | 21 + .../agents/__tests__/responses.unit.spec.js | 21 + api/server/controllers/agents/openai.js | 111 +++++- api/server/controllers/agents/responses.js | 125 ++++-- .../services/Endpoints/agents/initialize.js | 163 ++------ packages/api/src/agents/discovery.spec.ts | 374 ++++++++++++++++++ packages/api/src/agents/discovery.ts | 273 +++++++++++++ packages/api/src/agents/index.ts | 1 + packages/api/src/agents/validation.ts | 13 +- 9 files changed, 933 insertions(+), 169 deletions(-) create mode 100644 packages/api/src/agents/discovery.spec.ts create mode 100644 packages/api/src/agents/discovery.ts diff --git a/api/server/controllers/agents/__tests__/openai.spec.js b/api/server/controllers/agents/__tests__/openai.spec.js index c959be6cf4..f55b798f14 100644 --- a/api/server/controllers/agents/__tests__/openai.spec.js +++ b/api/server/controllers/agents/__tests__/openai.spec.js @@ -46,9 +46,11 @@ jest.mock('@librechat/api', () => ({ .fn() .mockReturnValue({ request: { model: 'agent-123', messages: [], stream: false } }), initializeAgent: jest.fn().mockResolvedValue({ + id: 'agent-123', model: 'gpt-4', model_parameters: {}, toolRegistry: {}, + edges: [], }), getBalanceConfig: mockGetBalanceConfig, createErrorResponse: jest.fn(), @@ -72,6 +74,24 @@ jest.mock('@librechat/api', () => ({ resolveRecursionLimit: jest.fn().mockReturnValue(50), createToolExecuteHandler: jest.fn().mockReturnValue({ handle: jest.fn() }), isChatCompletionValidationFailure: jest.fn().mockReturnValue(false), + discoverConnectedAgents: jest.fn().mockResolvedValue({ + agentConfigs: new Map(), + edges: [], + skippedAgentIds: new Set(), + userMCPAuthMap: undefined, + }), +})); + +jest.mock('~/server/controllers/ModelController', () => ({ + getModelsConfig: jest.fn().mockResolvedValue({}), +})); + +jest.mock('~/server/services/Files/permissions', () => ({ + filterFilesByAgentAccess: jest.fn(), +})); + +jest.mock('~/cache', () => ({ + logViolation: jest.fn(), })); jest.mock('~/server/services/ToolService', () => ({ @@ -91,6 +111,7 @@ jest.mock('~/server/controllers/agents/callbacks', () => ({ jest.mock('~/server/services/PermissionService', () => ({ findAccessibleResources: jest.fn().mockResolvedValue([]), + checkPermission: jest.fn().mockResolvedValue(true), })); const mockUpdateBalance = jest.fn().mockResolvedValue({}); diff --git a/api/server/controllers/agents/__tests__/responses.unit.spec.js b/api/server/controllers/agents/__tests__/responses.unit.spec.js index 26f5f5d30b..1a6fa6053f 100644 --- a/api/server/controllers/agents/__tests__/responses.unit.spec.js +++ b/api/server/controllers/agents/__tests__/responses.unit.spec.js @@ -43,9 +43,17 @@ jest.mock('@librechat/api', () => ({ buildToolSet: jest.fn().mockReturnValue(new Set()), createSafeUser: jest.fn().mockReturnValue({ id: 'user-123' }), initializeAgent: jest.fn().mockResolvedValue({ + id: 'agent-123', model: 'claude-3', model_parameters: {}, toolRegistry: {}, + edges: [], + }), + discoverConnectedAgents: jest.fn().mockResolvedValue({ + agentConfigs: new Map(), + edges: [], + skippedAgentIds: new Set(), + userMCPAuthMap: undefined, }), getBalanceConfig: mockGetBalanceConfig, getTransactionsConfig: mockGetTransactionsConfig, @@ -121,6 +129,19 @@ jest.mock('~/server/controllers/agents/callbacks', () => { jest.mock('~/server/services/PermissionService', () => ({ findAccessibleResources: jest.fn().mockResolvedValue([]), + checkPermission: jest.fn().mockResolvedValue(true), +})); + +jest.mock('~/server/controllers/ModelController', () => ({ + getModelsConfig: jest.fn().mockResolvedValue({}), +})); + +jest.mock('~/server/services/Files/permissions', () => ({ + filterFilesByAgentAccess: jest.fn(), +})); + +jest.mock('~/cache', () => ({ + logViolation: jest.fn(), })); const mockUpdateBalance = jest.fn().mockResolvedValue({}); diff --git a/api/server/controllers/agents/openai.js b/api/server/controllers/agents/openai.js index 9fa3af82c3..12e47672f4 100644 --- a/api/server/controllers/agents/openai.js +++ b/api/server/controllers/agents/openai.js @@ -21,6 +21,7 @@ const { createOpenAIStreamTracker, createOpenAIContentAggregator, isChatCompletionValidationFailure, + discoverConnectedAgents, } = require('@librechat/api'); const { buildSummarizationHandlers, @@ -29,7 +30,10 @@ const { agentLogHandlerObj, } = require('~/server/controllers/agents/callbacks'); const { loadAgentTools, loadToolsForExecution } = require('~/server/services/ToolService'); -const { findAccessibleResources } = require('~/server/services/PermissionService'); +const { findAccessibleResources, checkPermission } = require('~/server/services/PermissionService'); +const { getModelsConfig } = require('~/server/controllers/ModelController'); +const { filterFilesByAgentAccess } = require('~/server/services/Files/permissions'); +const { logViolation } = require('~/cache'); const db = require('~/models'); /** @@ -207,6 +211,19 @@ const OpenAIChatCompletionController = async (req, res) => { model_parameters: agent.model_parameters ?? {}, }; + const dbMethods = { + getConvoFiles: db.getConvoFiles, + getFiles: db.getFiles, + getUserKey: db.getUserKey, + getMessages: db.getMessages, + updateFilesUsage: db.updateFilesUsage, + getUserKeyValues: db.getUserKeyValues, + getUserCodeFiles: db.getUserCodeFiles, + getToolFilesByIds: db.getToolFilesByIds, + getCodeGeneratedFiles: db.getCodeGeneratedFiles, + filterFilesByAgentAccess, + }; + const primaryConfig = await initializeAgent( { req, @@ -220,19 +237,71 @@ const OpenAIChatCompletionController = async (req, res) => { allowedProviders, isInitialAgent: true, }, + dbMethods, + ); + + /** + * Per-agent tool-execution context map, keyed by agentId. + * Needed so the ON_TOOL_EXECUTE callback routes each sub-agent's tool calls + * to the correct toolRegistry / userMCPAuthMap / tool_resources. + * @type {Map>, + * tool_resources?: object, + * actionsEnabled?: boolean, + * }>} + */ + const agentToolContexts = new Map(); + agentToolContexts.set(primaryConfig.id, { + agent, + toolRegistry: primaryConfig.toolRegistry, + userMCPAuthMap: primaryConfig.userMCPAuthMap, + tool_resources: primaryConfig.tool_resources, + actionsEnabled: primaryConfig.actionsEnabled, + }); + + const modelsConfig = await getModelsConfig(req); + const { + agentConfigs: handoffAgentConfigs, + edges: discoveredEdges, + userMCPAuthMap: discoveredMCPAuthMap, + } = await discoverConnectedAgents( { - getConvoFiles: db.getConvoFiles, - getFiles: db.getFiles, - getUserKey: db.getUserKey, - getMessages: db.getMessages, - updateFilesUsage: db.updateFilesUsage, - getUserKeyValues: db.getUserKeyValues, - getUserCodeFiles: db.getUserCodeFiles, - getToolFilesByIds: db.getToolFilesByIds, - getCodeGeneratedFiles: db.getCodeGeneratedFiles, + req, + res, + primaryConfig, + endpointOption, + allowedProviders, + modelsConfig, + loadTools, + requestFiles: [], + conversationId, + parentMessageId, + }, + { + getAgent: db.getAgent, + checkPermission, + logViolation, + db: dbMethods, + onAgentInitialized: (agentId, handoffAgent, config) => { + agentToolContexts.set(agentId, { + agent: handoffAgent, + toolRegistry: config.toolRegistry, + userMCPAuthMap: config.userMCPAuthMap, + tool_resources: config.tool_resources, + actionsEnabled: config.actionsEnabled, + }); + }, + initializeAgent, }, ); + // Ensure edges is an array when multi-agent mode is active + // (MultiAgentGraph.categorizeEdges requires edges to be iterable) + const edges = handoffAgentConfigs.size > 0 ? (discoveredEdges ?? []) : discoveredEdges; + primaryConfig.edges = edges; + // Determine if streaming is enabled (check both request and agent config) const streamingDisabled = !!primaryConfig.model_parameters?.disableStreaming; const isStreaming = request.stream === true && !streamingDisabled; @@ -270,17 +339,18 @@ const OpenAIChatCompletionController = async (req, res) => { const toolEndCallback = createToolEndCallback({ req, res, artifactPromises, streamId: null }); const toolExecuteOptions = { - loadTools: async (toolNames) => { + loadTools: async (toolNames, agentId) => { + const ctx = agentToolContexts.get(agentId) ?? agentToolContexts.get(primaryConfig.id) ?? {}; return loadToolsForExecution({ req, res, - agent, toolNames, + agent: ctx.agent ?? agent, signal: abortController.signal, - toolRegistry: primaryConfig.toolRegistry, - userMCPAuthMap: primaryConfig.userMCPAuthMap, - tool_resources: primaryConfig.tool_resources, - actionsEnabled: primaryConfig.actionsEnabled, + toolRegistry: ctx.toolRegistry, + userMCPAuthMap: ctx.userMCPAuthMap, + tool_resources: ctx.tool_resources, + actionsEnabled: ctx.actionsEnabled, }); }, toolEndCallback, @@ -467,11 +537,14 @@ const OpenAIChatCompletionController = async (req, res) => { // Create and run the agent const userId = req.user?.id ?? 'api-user'; - // Extract userMCPAuthMap from primaryConfig (needed for MCP tool connections) - const userMCPAuthMap = primaryConfig.userMCPAuthMap; + // Extract merged userMCPAuthMap (needed for MCP tool connections across + // the primary and any discovered handoff sub-agents) + const userMCPAuthMap = discoveredMCPAuthMap ?? primaryConfig.userMCPAuthMap; + + const runAgents = [primaryConfig, ...handoffAgentConfigs.values()]; const run = await createRun({ - agents: [primaryConfig], + agents: runAgents, messages: formattedMessages, indexTokenCountMap, initialSummary, diff --git a/api/server/controllers/agents/responses.js b/api/server/controllers/agents/responses.js index 7abddf5e2f..45c3fa2249 100644 --- a/api/server/controllers/agents/responses.js +++ b/api/server/controllers/agents/responses.js @@ -12,6 +12,7 @@ const { recordCollectedUsage, getTransactionsConfig, createToolExecuteHandler, + discoverConnectedAgents, // Responses API writeDone, buildResponse, @@ -38,7 +39,10 @@ const { agentLogHandlerObj, } = require('~/server/controllers/agents/callbacks'); const { loadAgentTools, loadToolsForExecution } = require('~/server/services/ToolService'); -const { findAccessibleResources } = require('~/server/services/PermissionService'); +const { findAccessibleResources, checkPermission } = require('~/server/services/PermissionService'); +const { getModelsConfig } = require('~/server/controllers/ModelController'); +const { filterFilesByAgentAccess } = require('~/server/services/Files/permissions'); +const { logViolation } = require('~/cache'); const db = require('~/models'); /** @type {import('@librechat/api').AppConfig | null} */ @@ -345,6 +349,19 @@ const createResponse = async (req, res) => { model_parameters: agent.model_parameters ?? {}, }; + const dbMethods = { + getConvoFiles: db.getConvoFiles, + getFiles: db.getFiles, + getUserKey: db.getUserKey, + getMessages: db.getMessages, + updateFilesUsage: db.updateFilesUsage, + getUserKeyValues: db.getUserKeyValues, + getUserCodeFiles: db.getUserCodeFiles, + getToolFilesByIds: db.getToolFilesByIds, + getCodeGeneratedFiles: db.getCodeGeneratedFiles, + filterFilesByAgentAccess, + }; + const primaryConfig = await initializeAgent( { req, @@ -358,19 +375,71 @@ const createResponse = async (req, res) => { allowedProviders, isInitialAgent: true, }, + dbMethods, + ); + + /** + * Per-agent tool-execution context map, keyed by agentId. Ensures the + * ON_TOOL_EXECUTE callback routes each sub-agent's tool calls to the + * correct toolRegistry / userMCPAuthMap / tool_resources. + * @type {Map>, + * tool_resources?: object, + * actionsEnabled?: boolean, + * }>} + */ + const agentToolContexts = new Map(); + agentToolContexts.set(primaryConfig.id, { + agent, + toolRegistry: primaryConfig.toolRegistry, + userMCPAuthMap: primaryConfig.userMCPAuthMap, + tool_resources: primaryConfig.tool_resources, + actionsEnabled: primaryConfig.actionsEnabled, + }); + + const modelsConfig = await getModelsConfig(req); + const { + agentConfigs: handoffAgentConfigs, + edges: discoveredEdges, + userMCPAuthMap: discoveredMCPAuthMap, + } = await discoverConnectedAgents( { - getConvoFiles: db.getConvoFiles, - getFiles: db.getFiles, - getUserKey: db.getUserKey, - getMessages: db.getMessages, - updateFilesUsage: db.updateFilesUsage, - getUserKeyValues: db.getUserKeyValues, - getUserCodeFiles: db.getUserCodeFiles, - getToolFilesByIds: db.getToolFilesByIds, - getCodeGeneratedFiles: db.getCodeGeneratedFiles, + req, + res, + primaryConfig, + endpointOption, + allowedProviders, + modelsConfig, + loadTools, + requestFiles: [], + conversationId, + parentMessageId, + }, + { + getAgent: db.getAgent, + checkPermission, + logViolation, + db: dbMethods, + onAgentInitialized: (agentId, handoffAgent, config) => { + agentToolContexts.set(agentId, { + agent: handoffAgent, + toolRegistry: config.toolRegistry, + userMCPAuthMap: config.userMCPAuthMap, + tool_resources: config.tool_resources, + actionsEnabled: config.actionsEnabled, + }); + }, + initializeAgent, }, ); + const edges = handoffAgentConfigs.size > 0 ? (discoveredEdges ?? []) : discoveredEdges; + primaryConfig.edges = edges; + const runAgents = [primaryConfig, ...handoffAgentConfigs.values()]; + const mergedMCPAuthMap = discoveredMCPAuthMap ?? primaryConfig.userMCPAuthMap; + // Determine if streaming is enabled (check both request and agent config) const streamingDisabled = !!primaryConfig.model_parameters?.disableStreaming; const actuallyStreaming = isStreaming && !streamingDisabled; @@ -436,17 +505,19 @@ const createResponse = async (req, res) => { // Create tool execute options for event-driven tool execution const toolExecuteOptions = { - loadTools: async (toolNames) => { + loadTools: async (toolNames, agentId) => { + const ctx = + agentToolContexts.get(agentId) ?? agentToolContexts.get(primaryConfig.id) ?? {}; return loadToolsForExecution({ req, res, - agent, toolNames, + agent: ctx.agent ?? agent, signal: abortController.signal, - toolRegistry: primaryConfig.toolRegistry, - userMCPAuthMap: primaryConfig.userMCPAuthMap, - tool_resources: primaryConfig.tool_resources, - actionsEnabled: primaryConfig.actionsEnabled, + toolRegistry: ctx.toolRegistry, + userMCPAuthMap: ctx.userMCPAuthMap, + tool_resources: ctx.tool_resources, + actionsEnabled: ctx.actionsEnabled, }); }, toolEndCallback, @@ -483,10 +554,10 @@ const createResponse = async (req, res) => { // Create and run the agent const userId = req.user?.id ?? 'api-user'; - const userMCPAuthMap = primaryConfig.userMCPAuthMap; + const userMCPAuthMap = mergedMCPAuthMap; const run = await createRun({ - agents: [primaryConfig], + agents: runAgents, messages: formattedMessages, indexTokenCountMap, initialSummary, @@ -601,17 +672,19 @@ const createResponse = async (req, res) => { const toolEndCallback = createToolEndCallback({ req, res, artifactPromises, streamId: null }); const toolExecuteOptions = { - loadTools: async (toolNames) => { + loadTools: async (toolNames, agentId) => { + const ctx = + agentToolContexts.get(agentId) ?? agentToolContexts.get(primaryConfig.id) ?? {}; return loadToolsForExecution({ req, res, - agent, toolNames, + agent: ctx.agent ?? agent, signal: abortController.signal, - toolRegistry: primaryConfig.toolRegistry, - userMCPAuthMap: primaryConfig.userMCPAuthMap, - tool_resources: primaryConfig.tool_resources, - actionsEnabled: primaryConfig.actionsEnabled, + toolRegistry: ctx.toolRegistry, + userMCPAuthMap: ctx.userMCPAuthMap, + tool_resources: ctx.tool_resources, + actionsEnabled: ctx.actionsEnabled, }); }, toolEndCallback, @@ -646,10 +719,10 @@ const createResponse = async (req, res) => { }; const userId = req.user?.id ?? 'api-user'; - const userMCPAuthMap = primaryConfig.userMCPAuthMap; + const userMCPAuthMap = mergedMCPAuthMap; const run = await createRun({ - agents: [primaryConfig], + agents: runAgents, messages: formattedMessages, indexTokenCountMap, initialSummary, diff --git a/api/server/services/Endpoints/agents/initialize.js b/api/server/services/Endpoints/agents/initialize.js index 69767e191c..0a329dea0e 100644 --- a/api/server/services/Endpoints/agents/initialize.js +++ b/api/server/services/Endpoints/agents/initialize.js @@ -3,15 +3,11 @@ const { createContentAggregator } = require('@librechat/agents'); const { initializeAgent, validateAgentModel, - createEdgeCollector, - filterOrphanedEdges, GenerationJobManager, getCustomEndpointConfig, - createSequentialChainEdges, + discoverConnectedAgents, } = require('@librechat/api'); const { - ResourceType, - PermissionBits, EModelEndpoint, isAgentsEndpoint, getResponseSender, @@ -230,63 +226,29 @@ const initializeClient = async ({ req, res, signal, endpointOption }) => { actionsEnabled: primaryConfig.actionsEnabled, }); - const agent_ids = primaryConfig.agent_ids; - let userMCPAuthMap = primaryConfig.userMCPAuthMap; - - /** @type {Set} Track agents that failed to load (orphaned references) */ - const skippedAgentIds = new Set(); - - async function processAgent(agentId) { - const agent = await db.getAgent({ id: agentId }); - if (!agent) { - logger.warn( - `[processAgent] Handoff agent ${agentId} not found, skipping (orphaned reference)`, - ); - skippedAgentIds.add(agentId); - return null; - } - - const hasAccess = await checkPermission({ - userId: req.user.id, - role: req.user.role, - resourceType: ResourceType.AGENT, - resourceId: agent._id, - requiredPermission: PermissionBits.VIEW, - }); - - if (!hasAccess) { - logger.warn( - `[processAgent] User ${req.user.id} lacks VIEW access to handoff agent ${agentId}, skipping`, - ); - skippedAgentIds.add(agentId); - return null; - } - - const validationResult = await validateAgentModel({ + const { + agentConfigs: discoveredConfigs, + edges: discoveredEdges, + userMCPAuthMap: discoveredMCPAuthMap, + } = await discoverConnectedAgents( + { req, res, - agent, + primaryConfig, + agent_ids: primaryConfig.agent_ids, + endpointOption, + allowedProviders, modelsConfig, + loadTools, + requestFiles, + conversationId, + parentMessageId, + }, + { + getAgent: db.getAgent, + checkPermission, logViolation, - }); - - if (!validationResult.isValid) { - throw new Error(validationResult.error?.message); - } - - const config = await initializeAgent( - { - req, - res, - agent, - loadTools, - requestFiles, - conversationId, - parentMessageId, - endpointOption, - allowedProviders, - }, - { + db: { getFiles: db.getFiles, getUserKey: db.getUserKey, getMessages: db.getMessages, @@ -298,71 +260,33 @@ const initializeClient = async ({ req, res, signal, endpointOption }) => { getCodeGeneratedFiles: db.getCodeGeneratedFiles, filterFilesByAgentAccess, }, - ); - - if (userMCPAuthMap != null) { - Object.assign(userMCPAuthMap, config.userMCPAuthMap ?? {}); - } else { - userMCPAuthMap = config.userMCPAuthMap; - } - - /** Store handoff agent's tool context for ON_TOOL_EXECUTE callback */ - agentToolContexts.set(agentId, { - agent, - toolRegistry: config.toolRegistry, - userMCPAuthMap: config.userMCPAuthMap, - tool_resources: config.tool_resources, - actionsEnabled: config.actionsEnabled, - }); - - agentConfigs.set(agentId, config); - return agent; - } - - const checkAgentInit = (agentId) => agentId === primaryConfig.id || agentConfigs.has(agentId); - - // Graph topology discovery for recursive agent handoffs (BFS) - const { edgeMap, agentsToProcess, collectEdges } = createEdgeCollector( - checkAgentInit, - skippedAgentIds, + onAgentInitialized: (agentId, agent, config) => { + agentConfigs.set(agentId, config); + agentToolContexts.set(agentId, { + agent, + toolRegistry: config.toolRegistry, + userMCPAuthMap: config.userMCPAuthMap, + tool_resources: config.tool_resources, + actionsEnabled: config.actionsEnabled, + }); + }, + // Pass through the `@librechat/api` exports so that tests which + // `jest.mock('@librechat/api')` can override the initializer/validator. + initializeAgent, + validateAgentModel, + }, ); - // Seed with primary agent's edges - collectEdges(primaryConfig.edges); - - // BFS to load and merge all connected agents (enables transitive handoffs: A->B->C) - while (agentsToProcess.size > 0) { - const agentId = agentsToProcess.values().next().value; - agentsToProcess.delete(agentId); - try { - const agent = await processAgent(agentId); - if (agent?.edges?.length) { - collectEdges(agent.edges); - } - } catch (err) { - logger.error(`[initializeClient] Error processing agent ${agentId}:`, err); - skippedAgentIds.add(agentId); + // Merge any entries the onAgentInitialized callback did not receive + // (defensive: discoverConnectedAgents already returns a canonical map) + for (const [agentId, config] of discoveredConfigs) { + if (!agentConfigs.has(agentId)) { + agentConfigs.set(agentId, config); } } - /** @deprecated Agent Chain */ - if (agent_ids?.length) { - for (const agentId of agent_ids) { - if (checkAgentInit(agentId)) { - continue; - } - try { - await processAgent(agentId); - } catch (err) { - logger.error(`[initializeClient] Error processing chain agent ${agentId}:`, err); - skippedAgentIds.add(agentId); - } - } - const chain = await createSequentialChainEdges([primaryConfig.id].concat(agent_ids), '{convo}'); - collectEdges(chain); - } - - let edges = Array.from(edgeMap.values()); + let userMCPAuthMap = discoveredMCPAuthMap; + let edges = discoveredEdges; /** Multi-Convo: Process addedConvo for parallel agent execution */ const { userMCPAuthMap: updatedMCPAuthMap } = await processAddedConvo({ @@ -405,9 +329,6 @@ const initializeClient = async ({ req, res, signal, endpointOption }) => { edges = []; } - // Filter out edges referencing non-existent agents (orphaned references) - edges = filterOrphanedEdges(edges, skippedAgentIds); - primaryConfig.edges = edges; let endpointConfig = appConfig.endpoints?.[primaryConfig.endpoint]; diff --git a/packages/api/src/agents/discovery.spec.ts b/packages/api/src/agents/discovery.spec.ts new file mode 100644 index 0000000000..43424e07a2 --- /dev/null +++ b/packages/api/src/agents/discovery.spec.ts @@ -0,0 +1,374 @@ +import type { Agent, GraphEdge } from 'librechat-data-provider'; +import type { Request, Response } from 'express'; +import type { InitializedAgent } from './initialize'; + +jest.mock('@librechat/data-schemas', () => ({ + logger: { + debug: jest.fn(), + info: jest.fn(), + warn: jest.fn(), + error: jest.fn(), + }, +})); + +const mockInitializeAgent = jest.fn(); +jest.mock('./initialize', () => ({ + initializeAgent: (...args: unknown[]) => mockInitializeAgent(...args), +})); + +const mockValidateAgentModel = jest.fn(); +jest.mock('./validation', () => ({ + validateAgentModel: (...args: unknown[]) => mockValidateAgentModel(...args), +})); + +import { discoverConnectedAgents } from './discovery'; + +const makeReq = (userId = 'u1', role = 'USER'): Request => + ({ + user: { id: userId, role }, + }) as unknown as Request; + +const makeRes = (): Response => ({}) as unknown as Response; + +const makeAgent = (id: string, edges: GraphEdge[] = []): Agent => + ({ + id, + _id: `mongo-${id}`, + provider: 'openai', + model: 'gpt-4o', + edges, + }) as unknown as Agent; + +const makeConfig = (id: string, edges: GraphEdge[] = []): InitializedAgent => + ({ + id, + provider: 'openai', + model: 'gpt-4o', + edges, + tools: [], + }) as unknown as InitializedAgent; + +describe('discoverConnectedAgents', () => { + beforeEach(() => { + jest.clearAllMocks(); + mockValidateAgentModel.mockResolvedValue({ isValid: true }); + mockInitializeAgent.mockImplementation(async ({ agent }: { agent: Agent }) => + makeConfig(agent.id), + ); + }); + + it('returns empty configs and no edges for a single agent with no edges', async () => { + const primaryConfig = makeConfig('A'); + const result = await discoverConnectedAgents( + { + req: makeReq(), + res: makeRes(), + primaryConfig, + allowedProviders: new Set(), + modelsConfig: { openai: ['gpt-4o'] }, + loadTools: jest.fn(), + }, + { + getAgent: jest.fn(), + checkPermission: jest.fn(), + logViolation: jest.fn(), + db: {} as never, + }, + ); + + expect(result.agentConfigs.size).toBe(0); + expect(result.edges).toEqual([]); + expect(result.skippedAgentIds.size).toBe(0); + }); + + it('discovers handoff targets via BFS across a multi-hop chain A -> B -> C', async () => { + const edgesAB: GraphEdge[] = [{ from: 'A', to: 'B', edgeType: 'handoff' }]; + const edgesBC: GraphEdge[] = [{ from: 'B', to: 'C', edgeType: 'handoff' }]; + const primaryConfig = makeConfig('A', edgesAB); + + const agents: Record = { + B: makeAgent('B', edgesBC), + C: makeAgent('C', []), + }; + const getAgent = jest.fn(async ({ id }: { id: string }) => agents[id] ?? null); + const checkPermission = jest.fn().mockResolvedValue(true); + + const onAgentInitialized = jest.fn(); + + const result = await discoverConnectedAgents( + { + req: makeReq(), + res: makeRes(), + primaryConfig, + allowedProviders: new Set(), + modelsConfig: { openai: ['gpt-4o'] }, + loadTools: jest.fn(), + }, + { + getAgent, + checkPermission, + logViolation: jest.fn(), + db: {} as never, + onAgentInitialized, + }, + ); + + expect(getAgent).toHaveBeenCalledWith({ id: 'B' }); + expect(getAgent).toHaveBeenCalledWith({ id: 'C' }); + expect(result.agentConfigs.size).toBe(2); + expect(result.agentConfigs.has('B')).toBe(true); + expect(result.agentConfigs.has('C')).toBe(true); + expect(result.edges).toHaveLength(2); + expect(onAgentInitialized).toHaveBeenCalledTimes(2); + expect(onAgentInitialized.mock.calls.map((c) => c[0])).toEqual(['B', 'C']); + }); + + it('skips orphaned agents and filters out edges pointing at them', async () => { + const edges: GraphEdge[] = [ + { from: 'A', to: 'B', edgeType: 'handoff' }, + { from: 'A', to: 'MISSING', edgeType: 'handoff' }, + ]; + const primaryConfig = makeConfig('A', edges); + + const getAgent = jest.fn(async ({ id }: { id: string }) => + id === 'B' ? makeAgent('B', []) : null, + ); + const checkPermission = jest.fn().mockResolvedValue(true); + const onAgentSkipped = jest.fn(); + + const result = await discoverConnectedAgents( + { + req: makeReq(), + res: makeRes(), + primaryConfig, + allowedProviders: new Set(), + modelsConfig: { openai: ['gpt-4o'] }, + loadTools: jest.fn(), + }, + { + getAgent, + checkPermission, + logViolation: jest.fn(), + db: {} as never, + onAgentSkipped, + }, + ); + + expect(result.agentConfigs.has('B')).toBe(true); + expect(result.agentConfigs.has('MISSING')).toBe(false); + expect(result.skippedAgentIds.has('MISSING')).toBe(true); + expect(result.edges).toHaveLength(1); + expect(result.edges[0].to).toBe('B'); + expect(onAgentSkipped).toHaveBeenCalledWith('MISSING'); + }); + + it('skips sub-agents the user lacks VIEW permission on and filters their edges', async () => { + const edges: GraphEdge[] = [ + { from: 'A', to: 'B', edgeType: 'handoff' }, + { from: 'A', to: 'FORBIDDEN', edgeType: 'handoff' }, + ]; + const primaryConfig = makeConfig('A', edges); + + const agentMap: Record = { + B: makeAgent('B', []), + FORBIDDEN: makeAgent('FORBIDDEN', []), + }; + const getAgent = jest.fn(async ({ id }: { id: string }) => agentMap[id] ?? null); + const checkPermission = jest.fn( + async ({ resourceId }: { resourceId: unknown }) => resourceId !== 'mongo-FORBIDDEN', + ); + + const result = await discoverConnectedAgents( + { + req: makeReq(), + res: makeRes(), + primaryConfig, + allowedProviders: new Set(), + modelsConfig: { openai: ['gpt-4o'] }, + loadTools: jest.fn(), + }, + { + getAgent, + checkPermission, + logViolation: jest.fn(), + db: {} as never, + }, + ); + + expect(result.agentConfigs.has('B')).toBe(true); + expect(result.agentConfigs.has('FORBIDDEN')).toBe(false); + expect(result.skippedAgentIds.has('FORBIDDEN')).toBe(true); + expect(result.edges.map((e) => e.to)).toEqual(['B']); + }); + + it('does not re-initialize agents that are already known (dedup)', async () => { + const edges: GraphEdge[] = [ + { from: 'A', to: 'B', edgeType: 'handoff' }, + { from: 'B', to: 'A', edgeType: 'handoff' }, + { from: 'A', to: 'B', edgeType: 'handoff' }, + ]; + const primaryConfig = makeConfig('A', edges); + + const getAgent = jest.fn(async ({ id }: { id: string }) => + id === 'B' ? makeAgent('B', [{ from: 'B', to: 'A', edgeType: 'handoff' }]) : null, + ); + const checkPermission = jest.fn().mockResolvedValue(true); + + const result = await discoverConnectedAgents( + { + req: makeReq(), + res: makeRes(), + primaryConfig, + allowedProviders: new Set(), + modelsConfig: { openai: ['gpt-4o'] }, + loadTools: jest.fn(), + }, + { + getAgent, + checkPermission, + logViolation: jest.fn(), + db: {} as never, + }, + ); + + // B should be fetched once; A is the primary and is never re-fetched + expect(getAgent).toHaveBeenCalledTimes(1); + expect(result.agentConfigs.size).toBe(1); + expect(result.agentConfigs.has('B')).toBe(true); + // Deduped edges: A->B and B->A (not the duplicate A->B) + expect(result.edges).toHaveLength(2); + }); + + it('processes legacy agent_ids chain and adds sequential chain edges', async () => { + const primaryConfig = makeConfig('A', []); + + const agentMap: Record = { + B: makeAgent('B', []), + C: makeAgent('C', []), + }; + const getAgent = jest.fn(async ({ id }: { id: string }) => agentMap[id] ?? null); + const checkPermission = jest.fn().mockResolvedValue(true); + + const result = await discoverConnectedAgents( + { + req: makeReq(), + res: makeRes(), + primaryConfig, + agent_ids: ['B', 'C'], + allowedProviders: new Set(), + modelsConfig: { openai: ['gpt-4o'] }, + loadTools: jest.fn(), + }, + { + getAgent, + checkPermission, + logViolation: jest.fn(), + db: {} as never, + }, + ); + + expect(result.agentConfigs.size).toBe(2); + expect(result.agentConfigs.has('B')).toBe(true); + expect(result.agentConfigs.has('C')).toBe(true); + // Sequential chain: A -> B -> C (2 direct edges) + expect(result.edges).toHaveLength(2); + expect(result.edges.every((e) => e.edgeType === 'direct')).toBe(true); + }); + + it('merges userMCPAuthMap across primary and sub-agents', async () => { + const primaryConfig = makeConfig('A', [{ from: 'A', to: 'B', edgeType: 'handoff' }]); + primaryConfig.userMCPAuthMap = { serverA: { token: 'primary' } }; + + const getAgent = jest.fn(async () => makeAgent('B', [])); + const checkPermission = jest.fn().mockResolvedValue(true); + + mockInitializeAgent.mockImplementation(async ({ agent }: { agent: Agent }) => { + const cfg = makeConfig(agent.id); + cfg.userMCPAuthMap = { serverB: { token: 'secondary' } }; + return cfg; + }); + + const result = await discoverConnectedAgents( + { + req: makeReq(), + res: makeRes(), + primaryConfig, + allowedProviders: new Set(), + modelsConfig: { openai: ['gpt-4o'] }, + loadTools: jest.fn(), + }, + { + getAgent, + checkPermission, + logViolation: jest.fn(), + db: {} as never, + }, + ); + + expect(result.userMCPAuthMap).toEqual({ + serverA: { token: 'primary' }, + serverB: { token: 'secondary' }, + }); + }); + + it('throws when a sub-agent model validation fails', async () => { + mockValidateAgentModel.mockResolvedValueOnce({ + isValid: false, + error: { message: 'bad model' }, + }); + + const primaryConfig = makeConfig('A', [{ from: 'A', to: 'B', edgeType: 'handoff' }]); + + const getAgent = jest.fn(async () => makeAgent('B', [])); + const checkPermission = jest.fn().mockResolvedValue(true); + + const result = await discoverConnectedAgents( + { + req: makeReq(), + res: makeRes(), + primaryConfig, + allowedProviders: new Set(), + modelsConfig: { openai: ['gpt-4o'] }, + loadTools: jest.fn(), + }, + { + getAgent, + checkPermission, + logViolation: jest.fn(), + db: {} as never, + }, + ); + + // Validation error is caught inside the BFS and the agent is skipped + expect(result.skippedAgentIds.has('B')).toBe(true); + expect(result.agentConfigs.has('B')).toBe(false); + }); + + it('skips when request has no authenticated user', async () => { + const primaryConfig = makeConfig('A', [{ from: 'A', to: 'B', edgeType: 'handoff' }]); + + const getAgent = jest.fn(async () => makeAgent('B', [])); + const checkPermission = jest.fn(); + + const result = await discoverConnectedAgents( + { + req: { user: undefined } as unknown as Request, + res: makeRes(), + primaryConfig, + allowedProviders: new Set(), + modelsConfig: { openai: ['gpt-4o'] }, + loadTools: jest.fn(), + }, + { + getAgent, + checkPermission, + logViolation: jest.fn(), + db: {} as never, + }, + ); + + expect(checkPermission).not.toHaveBeenCalled(); + expect(result.skippedAgentIds.has('B')).toBe(true); + expect(result.edges).toHaveLength(0); + }); +}); diff --git a/packages/api/src/agents/discovery.ts b/packages/api/src/agents/discovery.ts new file mode 100644 index 0000000000..9aef0ee9b6 --- /dev/null +++ b/packages/api/src/agents/discovery.ts @@ -0,0 +1,273 @@ +import { logger } from '@librechat/data-schemas'; +import { ResourceType, PermissionBits } from 'librechat-data-provider'; +import type { Agent, GraphEdge, TModelsConfig, TEndpointOption } from 'librechat-data-provider'; +import type { Response as ServerResponse } from 'express'; +import type { ServerRequest } from '~/types'; +import type { + InitializedAgent, + InitializeAgentParams, + InitializeAgentDbMethods, +} from './initialize'; +import type { ValidateAgentModelParams } from './validation'; +import { createEdgeCollector, filterOrphanedEdges } from './edges'; +import { createSequentialChainEdges } from './chain'; +import { validateAgentModel as defaultValidateAgentModel } from './validation'; +import { initializeAgent as defaultInitializeAgent } from './initialize'; + +/** + * Callback invoked after a sub-agent is successfully initialized. + * Used by callers that need to track per-agent tool context (e.g., for + * the ON_TOOL_EXECUTE event handler closure). + */ +export type OnAgentInitializedCallback = ( + agentId: string, + agent: Agent, + config: InitializedAgent, +) => void; + +/** + * Minimal permission check signature used to verify VIEW access on a + * candidate sub-agent before it is loaded into the run. + */ +export type CheckAgentPermission = (params: { + userId: string; + role?: string; + resourceType: string; + resourceId: unknown; + requiredPermission: number; +}) => Promise; + +export interface DiscoverConnectedAgentsParams { + req: ServerRequest; + res: ServerResponse; + /** The already-initialized primary agent config (starting point for BFS). */ + primaryConfig: InitializedAgent; + /** + * Optional legacy chain: agent IDs to append as sequential direct edges. + * Used by the deprecated Agent Chain feature. + */ + agent_ids?: string[]; + endpointOption?: Partial; + allowedProviders: Set; + modelsConfig: TModelsConfig; + loadTools: InitializeAgentParams['loadTools']; + requestFiles?: InitializeAgentParams['requestFiles']; + conversationId?: string | null; + parentMessageId?: string | null; +} + +export interface DiscoverConnectedAgentsDeps { + /** Fetch an agent by string id from the database. */ + getAgent: (filter: { id: string }) => Promise; + /** Permission check (typically a wrapper around PermissionService.checkPermission). */ + checkPermission: CheckAgentPermission; + /** Violation logger passed through to validateAgentModel. */ + logViolation: ValidateAgentModelParams['logViolation']; + /** DB methods consumed by initializeAgent for each sub-agent. */ + db: InitializeAgentDbMethods; + /** Optional callback invoked after each sub-agent is initialized. */ + onAgentInitialized?: OnAgentInitializedCallback; + /** Optional callback invoked when an agent id is skipped (missing or no access). */ + onAgentSkipped?: (agentId: string) => void; + /** + * Optional override for `initializeAgent`. Exists primarily so JS callers + * can inject their test doubles via `jest.mock('@librechat/api')` — since + * this module's own direct import would otherwise bypass that mock. + */ + initializeAgent?: typeof defaultInitializeAgent; + /** Optional override for `validateAgentModel` (same DI rationale). */ + validateAgentModel?: typeof defaultValidateAgentModel; +} + +export interface DiscoverConnectedAgentsResult { + /** Map of agentId -> initialized config for every discovered sub-agent. */ + agentConfigs: Map; + /** Deduplicated, orphan-filtered edges across the primary and sub-agents. */ + edges: GraphEdge[]; + /** Agent ids that were requested but could not be loaded (missing or no access). */ + skippedAgentIds: Set; + /** Merged MCP auth map from the primary and all sub-agents. */ + userMCPAuthMap?: Record>; +} + +/** + * Discovers and initializes all agents reachable from `primaryConfig.edges` + * via BFS. This is the shared graph-topology discovery logic that enables + * multi-agent handoffs (A -> B -> C) in both the primary chat flow and the + * OpenAI-compatible / Responses API controllers. + * + * Skips agents the caller cannot load (missing from DB or lacking VIEW + * permission) and filters out orphaned edges so `createRun` never sees an + * edge pointing at a missing node — which would otherwise trigger a + * `Found edge ending at unknown node` validation error from StateGraph. + */ +export async function discoverConnectedAgents( + params: DiscoverConnectedAgentsParams, + deps: DiscoverConnectedAgentsDeps, +): Promise { + const { + req, + res, + primaryConfig, + agent_ids, + endpointOption, + allowedProviders, + modelsConfig, + loadTools, + requestFiles, + conversationId, + parentMessageId, + } = params; + + const { + getAgent, + checkPermission, + logViolation, + db, + onAgentInitialized, + onAgentSkipped, + initializeAgent = defaultInitializeAgent, + validateAgentModel = defaultValidateAgentModel, + } = deps; + + const agentConfigs = new Map(); + const skippedAgentIds = new Set(); + let userMCPAuthMap: Record> | undefined = + primaryConfig.userMCPAuthMap; + + const markSkipped = (agentId: string): void => { + skippedAgentIds.add(agentId); + onAgentSkipped?.(agentId); + }; + + const processAgent = async (agentId: string): Promise => { + const agent = await getAgent({ id: agentId }); + if (!agent) { + logger.warn( + `[discoverConnectedAgents] Handoff agent ${agentId} not found, skipping (orphaned reference)`, + ); + markSkipped(agentId); + return null; + } + + const userId = req.user?.id; + if (!userId) { + logger.warn( + `[discoverConnectedAgents] No authenticated user on request, skipping handoff agent ${agentId}`, + ); + markSkipped(agentId); + return null; + } + + const hasAccess = await checkPermission({ + userId, + role: req.user?.role, + resourceType: ResourceType.AGENT, + resourceId: agent._id, + requiredPermission: PermissionBits.VIEW, + }); + + if (!hasAccess) { + logger.warn( + `[discoverConnectedAgents] User ${userId} lacks VIEW access to handoff agent ${agentId}, skipping`, + ); + markSkipped(agentId); + return null; + } + + const validation = await validateAgentModel({ + req, + res, + agent, + modelsConfig, + logViolation, + }); + + if (!validation.isValid) { + throw new Error(validation.error?.message); + } + + const config = await initializeAgent( + { + req, + res, + agent, + loadTools, + requestFiles, + conversationId, + parentMessageId, + endpointOption, + allowedProviders, + }, + db, + ); + + if (userMCPAuthMap != null) { + Object.assign(userMCPAuthMap, config.userMCPAuthMap ?? {}); + } else { + userMCPAuthMap = config.userMCPAuthMap; + } + + agentConfigs.set(agentId, config); + onAgentInitialized?.(agentId, agent, config); + return agent; + }; + + const checkAgentInit = (agentId: string): boolean => + agentId === primaryConfig.id || agentConfigs.has(agentId); + + const { edgeMap, agentsToProcess, collectEdges } = createEdgeCollector( + checkAgentInit, + skippedAgentIds, + ); + + collectEdges(primaryConfig.edges); + + while (agentsToProcess.size > 0) { + const agentId = agentsToProcess.values().next().value as string; + agentsToProcess.delete(agentId); + try { + const agent = await processAgent(agentId); + if (agent?.edges?.length) { + collectEdges(agent.edges); + } + } catch (err) { + logger.error(`[discoverConnectedAgents] Error processing agent ${agentId}:`, err); + markSkipped(agentId); + } + } + + /** @deprecated Agent Chain — sequential direct-edge fallback */ + if (agent_ids?.length) { + for (const agentId of agent_ids) { + if (checkAgentInit(agentId)) { + continue; + } + try { + await processAgent(agentId); + } catch (err) { + logger.error(`[discoverConnectedAgents] Error processing chain agent ${agentId}:`, err); + markSkipped(agentId); + } + } + /** + * `createSequentialChainEdges` is typed against `@librechat/agents`'s + * `GraphEdge` (which uses `BaseMessage` from `@langchain/core`) whereas + * `collectEdges` uses the `librechat-data-provider` variant (structural + * `BaseMessage`). The produced chain edges are structurally identical and + * only carry `edgeType`, `from`, `to`, `prompt`, `excludeResults` — + * interchangeable for edge collection purposes. + */ + const chain = await createSequentialChainEdges([primaryConfig.id].concat(agent_ids), '{convo}'); + collectEdges(chain as unknown as GraphEdge[]); + } + + const edges = filterOrphanedEdges(Array.from(edgeMap.values()), skippedAgentIds); + + return { + agentConfigs, + edges, + skippedAgentIds, + userMCPAuthMap, + }; +} diff --git a/packages/api/src/agents/index.ts b/packages/api/src/agents/index.ts index 53f7f60a93..336eaea479 100644 --- a/packages/api/src/agents/index.ts +++ b/packages/api/src/agents/index.ts @@ -3,6 +3,7 @@ export * from './chain'; export * from './client'; export * from './config'; export * from './context'; +export * from './discovery'; export * from './edges'; export * from './handlers'; export * from './initialize'; diff --git a/packages/api/src/agents/validation.ts b/packages/api/src/agents/validation.ts index 8119c97204..6e3a1c200d 100644 --- a/packages/api/src/agents/validation.ts +++ b/packages/api/src/agents/validation.ts @@ -3,6 +3,13 @@ import { ViolationTypes, ErrorTypes } from 'librechat-data-provider'; import type { Agent, TModelsConfig } from 'librechat-data-provider'; import type { Request, Response } from 'express'; +/** + * Permissive Request alias used by {@link validateAgentModel}. Accepts either + * the default Express `Request` or the project-specific `ServerRequest` + * (see `~/types/http`), whose `params` type is widened to `unknown`. + */ +type LooseRequest = Request; + /** Avatar schema shared between create and update */ export const agentAvatarSchema = z.object({ filepath: z.string(), @@ -96,13 +103,13 @@ export const agentUpdateSchema = agentBaseSchema.extend({ model: z.string().nullable().optional(), }); -interface ValidateAgentModelParams { - req: Request; +export interface ValidateAgentModelParams { + req: LooseRequest; res: Response; agent: Agent; modelsConfig: TModelsConfig; logViolation: ( - req: Request, + req: LooseRequest, res: Response, type: string, errorMessage: Record,