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..6b07993715 100644 --- a/api/server/controllers/agents/openai.js +++ b/api/server/controllers/agents/openai.js @@ -1,7 +1,12 @@ const { nanoid } = require('nanoid'); const { logger } = require('@librechat/data-schemas'); const { Callback, ToolEndHandler, formatAgentMessages } = require('@librechat/agents'); -const { EModelEndpoint, ResourceType, PermissionBits } = require('librechat-data-provider'); +const { + EModelEndpoint, + ResourceType, + PermissionBits, + hasPermissions, +} = require('librechat-data-provider'); const { writeSSE, createRun, @@ -21,6 +26,8 @@ const { createOpenAIStreamTracker, createOpenAIContentAggregator, isChatCompletionValidationFailure, + discoverConnectedAgents, + getRemoteAgentPermissions, } = require('@librechat/api'); const { buildSummarizationHandlers, @@ -29,7 +36,12 @@ const { agentLogHandlerObj, } = require('~/server/controllers/agents/callbacks'); const { loadAgentTools, loadToolsForExecution } = require('~/server/services/ToolService'); -const { findAccessibleResources } = require('~/server/services/PermissionService'); +const { + findAccessibleResources, + getEffectivePermissions, +} = require('~/server/services/PermissionService'); +const { getModelsConfig } = require('~/server/controllers/ModelController'); +const { logViolation } = require('~/cache'); const db = require('~/models'); /** @@ -207,6 +219,24 @@ const OpenAIChatCompletionController = async (req, res) => { model_parameters: agent.model_parameters ?? {}, }; + // `filterFilesByAgentAccess` is intentionally omitted: it calls + // `checkPermission` with `resourceType: AGENT`, but this route + // authorizes callers through `REMOTE_AGENT` (via + // `getRemoteAgentPermissions`), so including it would silently drop + // owner-attached context files for any remote user who has + // `REMOTE_AGENT_VIEWER` but not direct `AGENT_VIEW`. + 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, + }; + const primaryConfig = await initializeAgent( { req, @@ -220,19 +250,92 @@ const OpenAIChatCompletionController = async (req, res) => { allowedProviders, isInitialAgent: true, }, - { - 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, - }, + 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, + }); + + // Only run BFS discovery (and pay `getModelsConfig` upfront) when the + // primary has edges to follow — the common API case is single-agent. + let handoffAgentConfigs = new Map(); + let discoveredEdges = []; + let discoveredMCPAuthMap; + if (primaryConfig.edges?.length) { + const modelsConfig = await getModelsConfig(req); + ({ + agentConfigs: handoffAgentConfigs, + edges: discoveredEdges, + userMCPAuthMap: discoveredMCPAuthMap, + } = await discoverConnectedAgents( + { + req, + res, + primaryConfig, + endpointOption, + allowedProviders, + modelsConfig, + loadTools, + requestFiles: [], + conversationId, + parentMessageId, + // The route enforces REMOTE_AGENT on the primary; every discovered + // sub-agent must clear the same sharing boundary, not the looser + // in-app AGENT one. + resourceType: ResourceType.REMOTE_AGENT, + }, + { + getAgent: db.getAgent, + // Use `getRemoteAgentPermissions` so sub-agent authorization + // matches what the route's `createCheckRemoteAgentAccess` + // middleware does for the primary: AGENT owners with the SHARE + // bit are treated as remotely authorized even without an + // explicit REMOTE_AGENT grant. + checkPermission: async ({ userId, role, resourceId, requiredPermission }) => { + const permissions = await getRemoteAgentPermissions( + { getEffectivePermissions }, + userId, + role, + resourceId, + ); + return hasPermissions(permissions, requiredPermission); + }, + 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, + }, + )); + } + + primaryConfig.edges = discoveredEdges; + // 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 +373,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 +571,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..beade07414 100644 --- a/api/server/controllers/agents/responses.js +++ b/api/server/controllers/agents/responses.js @@ -2,7 +2,12 @@ const { nanoid } = require('nanoid'); const { v4: uuidv4 } = require('uuid'); const { logger } = require('@librechat/data-schemas'); const { Callback, ToolEndHandler, formatAgentMessages } = require('@librechat/agents'); -const { EModelEndpoint, ResourceType, PermissionBits } = require('librechat-data-provider'); +const { + EModelEndpoint, + ResourceType, + PermissionBits, + hasPermissions, +} = require('librechat-data-provider'); const { createRun, buildToolSet, @@ -12,6 +17,8 @@ const { recordCollectedUsage, getTransactionsConfig, createToolExecuteHandler, + discoverConnectedAgents, + getRemoteAgentPermissions, // Responses API writeDone, buildResponse, @@ -38,7 +45,12 @@ const { agentLogHandlerObj, } = require('~/server/controllers/agents/callbacks'); const { loadAgentTools, loadToolsForExecution } = require('~/server/services/ToolService'); -const { findAccessibleResources } = require('~/server/services/PermissionService'); +const { + findAccessibleResources, + getEffectivePermissions, +} = require('~/server/services/PermissionService'); +const { getModelsConfig } = require('~/server/controllers/ModelController'); +const { logViolation } = require('~/cache'); const db = require('~/models'); /** @type {import('@librechat/api').AppConfig | null} */ @@ -345,6 +357,24 @@ const createResponse = async (req, res) => { model_parameters: agent.model_parameters ?? {}, }; + // `filterFilesByAgentAccess` is intentionally omitted: it calls + // `checkPermission` with `resourceType: AGENT`, but this route + // authorizes callers through `REMOTE_AGENT` (via + // `getRemoteAgentPermissions`), so including it would silently drop + // owner-attached context files for any remote user who has + // `REMOTE_AGENT_VIEWER` but not direct `AGENT_VIEW`. + 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, + }; + const primaryConfig = await initializeAgent( { req, @@ -358,19 +388,94 @@ const createResponse = async (req, res) => { allowedProviders, isInitialAgent: true, }, - { - 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, - }, + 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, + }); + + // Only run BFS discovery (and pay `getModelsConfig` upfront) when the + // primary has edges to follow — the common API case is single-agent. + let handoffAgentConfigs = new Map(); + let discoveredEdges = []; + let discoveredMCPAuthMap; + if (primaryConfig.edges?.length) { + const modelsConfig = await getModelsConfig(req); + ({ + agentConfigs: handoffAgentConfigs, + edges: discoveredEdges, + userMCPAuthMap: discoveredMCPAuthMap, + } = await discoverConnectedAgents( + { + req, + res, + primaryConfig, + endpointOption, + allowedProviders, + modelsConfig, + loadTools, + requestFiles: [], + conversationId, + parentMessageId, + // The route enforces REMOTE_AGENT on the primary; every discovered + // sub-agent must clear the same sharing boundary, not the looser + // in-app AGENT one. + resourceType: ResourceType.REMOTE_AGENT, + }, + { + getAgent: db.getAgent, + // Use `getRemoteAgentPermissions` so sub-agent authorization + // matches what the route's `createCheckRemoteAgentAccess` + // middleware does for the primary: AGENT owners with the SHARE + // bit are treated as remotely authorized even without an + // explicit REMOTE_AGENT grant. + checkPermission: async ({ userId, role, resourceId, requiredPermission }) => { + const permissions = await getRemoteAgentPermissions( + { getEffectivePermissions }, + userId, + role, + resourceId, + ); + return hasPermissions(permissions, requiredPermission); + }, + 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, + }, + )); + } + + primaryConfig.edges = discoveredEdges; + 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 +541,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 +590,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 +708,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 +755,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..549e9047b5 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,38 @@ 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, + // The callback fires during BFS, before the helper prunes agents + // whose edges end up filtered. Don't populate `agentConfigs` here — + // `discoveredConfigs` (returned below) is the authoritative pruned + // set. The per-agent tool context map is OK to keep populated even + // for pruned ids: it's only read by closure in ON_TOOL_EXECUTE, + // stale entries are unreachable at runtime. + onAgentInitialized: (agentId, agent, 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); - } + // Copy the pruned discovery result into the outer map. Anything the + // helper dropped (skipped or unreachable after edge filtering) is + // intentionally absent. `processAddedConvo` below may still add more + // entries for parallel multi-convo execution. + for (const [agentId, config] of discoveredConfigs) { + 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({ @@ -399,15 +328,8 @@ const initializeClient = async ({ req, res, signal, endpointOption }) => { }); } - // Ensure edges is an array when we have multiple agents (multi-agent mode) - // MultiAgentGraph.categorizeEdges requires edges to be iterable - if (agentConfigs.size > 0 && !edges) { - edges = []; - } - - // Filter out edges referencing non-existent agents (orphaned references) - edges = filterOrphanedEdges(edges, skippedAgentIds); - + // `discoverConnectedAgents` always returns a concrete array, so no + // further normalization is needed before handing this to `createRun`. 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..5b86cd194e --- /dev/null +++ b/packages/api/src/agents/discovery.spec.ts @@ -0,0 +1,999 @@ +import { EModelEndpoint } from 'librechat-data-provider'; +import type { Agent, GraphEdge } from 'librechat-data-provider'; +import type { Response } from 'express'; +import type { ServerRequest } from '~/types'; +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'): ServerRequest => + ({ + user: { id: userId, role }, + }) as unknown as ServerRequest; + +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('forces `endpoint: agents` on sub-agent init so allowedProviders is always enforced', async () => { + const primaryConfig = makeConfig('A', [{ from: 'A', to: 'B', edgeType: 'handoff' }]); + const getAgent = jest.fn(async () => makeAgent('B', [])); + const checkPermission = jest.fn().mockResolvedValue(true); + + await discoverConnectedAgents( + { + req: makeReq(), + res: makeRes(), + primaryConfig, + // Caller passes a non-agents endpoint (e.g. the OpenAI-compat + // controllers do this — endpoint = primary provider) + endpointOption: { endpoint: EModelEndpoint.openAI, model_parameters: {} }, + allowedProviders: new Set(['openai']), + modelsConfig: { openai: ['gpt-4o'] }, + loadTools: jest.fn(), + }, + { + getAgent, + checkPermission, + logViolation: jest.fn(), + db: {} as never, + }, + ); + + // The helper must override `endpoint` to 'agents' so + // `isAgentsEndpoint` is true and the allowedProviders guard in + // `initializeAgent` actually runs for sub-agents. + expect(mockInitializeAgent).toHaveBeenCalled(); + const initArgs = mockInitializeAgent.mock.calls[0][0]; + expect(initArgs.endpointOption.endpoint).toBe(EModelEndpoint.agents); + }); + + it('passes the configured resourceType (e.g. REMOTE_AGENT) to checkPermission', async () => { + const primaryConfig = makeConfig('A', [{ from: 'A', to: 'B', edgeType: 'handoff' }]); + const getAgent = jest.fn(async () => makeAgent('B', [])); + const checkPermission = jest.fn().mockResolvedValue(true); + + await discoverConnectedAgents( + { + req: makeReq(), + res: makeRes(), + primaryConfig, + allowedProviders: new Set(), + modelsConfig: { openai: ['gpt-4o'] }, + loadTools: jest.fn(), + resourceType: 'REMOTE_AGENT', + }, + { + getAgent, + checkPermission, + logViolation: jest.fn(), + db: {} as never, + }, + ); + + expect(checkPermission).toHaveBeenCalledWith( + expect.objectContaining({ resourceType: 'REMOTE_AGENT' }), + ); + }); + + it('prunes sub-agents disconnected from the primary after edge filtering', async () => { + // Graph: primary A has edges [A->B, B->C] stored directly on A. + // BFS loads B and C before permission is checked. B is skipped, and + // both edges reference B so they are filtered out. C was initialized + // but is now unreachable from A through any surviving edge — it must + // NOT be returned (otherwise it becomes a stray start node when + // createRun sees `agents.length > 1`). + const edges: GraphEdge[] = [ + { from: 'A', to: 'B', edgeType: 'handoff' }, + { from: 'B', to: 'C', edgeType: 'handoff' }, + ]; + const primaryConfig = makeConfig('A', edges); + + const agentMap: Record = { + B: makeAgent('B', []), + C: makeAgent('C', []), + }; + const getAgent = jest.fn(async ({ id }: { id: string }) => agentMap[id] ?? null); + // B fails permission, C passes. + const checkPermission = jest.fn( + async ({ resourceId }: { resourceId: unknown }) => resourceId !== 'mongo-B', + ); + + 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.skippedAgentIds.has('B')).toBe(true); + expect(result.edges).toHaveLength(0); + // C is unreachable once B's edges are filtered → must be pruned. + expect(result.agentConfigs.has('C')).toBe(false); + expect(result.agentConfigs.size).toBe(0); + }); + + it('preserves valid routes when one co-source of a multi-source edge is skipped', async () => { + // Primary A has edge `{ from: ['A','B'], to: 'C' }`, but B lacks + // VIEW permission. The SDK treats each source independently, so the + // `A -> C` route is still valid. Discovery must strip B from the + // edge's sources rather than dropping the whole edge. + const edges: GraphEdge[] = [{ from: ['A', 'B'], to: 'C', edgeType: 'handoff' }]; + const primaryConfig = makeConfig('A', edges); + + const agentMap: Record = { + B: makeAgent('B', []), + C: makeAgent('C', []), + }; + const getAgent = jest.fn(async ({ id }: { id: string }) => agentMap[id] ?? null); + // Only B is forbidden. + const checkPermission = jest.fn( + async ({ resourceId }: { resourceId: unknown }) => resourceId !== 'mongo-B', + ); + + 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.skippedAgentIds.has('B')).toBe(true); + expect(result.agentConfigs.has('C')).toBe(true); + expect(result.agentConfigs.has('B')).toBe(false); + expect(result.edges).toHaveLength(1); + expect(result.edges[0].from).toEqual(['A']); + expect(result.edges[0].to).toBe('C'); + }); + + it('advances through a multi-source edge on ANY reachable source (SDK OR semantics)', async () => { + // Primary A has a single edge `{from: ['A','B'], to: 'C'}`. B loads + // successfully but has no incoming path from A. The agents SDK adds + // one LangGraph edge per `from` source (see + // `agents/src/graphs/MultiAgentGraph.ts`), so `A -> C` is a real + // routing regardless of B's reachability. Discovery must preserve it: + // - C reachable via A + // - edge kept (A source reachable, C dest reachable) + // - B kept in agentConfigs because it's still referenced by the + // surviving edge; otherwise the SDK's `validateEdgeAgents` + // safety-net rejects the graph for a missing `from` endpoint. + const edges: GraphEdge[] = [{ from: ['A', 'B'], to: 'C', edgeType: 'handoff' }]; + const primaryConfig = makeConfig('A', edges); + + 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, + 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('C')).toBe(true); + expect(result.edges).toHaveLength(1); + expect(result.edges[0].from).toEqual(['A', 'B']); + expect(result.edges[0].to).toBe('C'); + }); + + it('advances through a multi-source edge once all sources are reachable', async () => { + // Two independent paths converge: `A -> B` and `A -> C` both reach + // the fan-in edge `['B','C'] -> D`. Once both B and C are reachable, + // D should become reachable in a subsequent fixed-point iteration. + const edges: GraphEdge[] = [ + { from: 'A', to: 'B', edgeType: 'direct' }, + { from: 'A', to: 'C', edgeType: 'direct' }, + { from: ['B', 'C'], to: 'D', edgeType: 'direct' }, + ]; + const primaryConfig = makeConfig('A', edges); + + const agentMap: Record = { + B: makeAgent('B', []), + C: makeAgent('C', []), + D: makeAgent('D', []), + }; + 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, + 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('C')).toBe(true); + expect(result.agentConfigs.has('D')).toBe(true); + expect(result.edges).toHaveLength(3); + }); + + it('strips unreachable co-sources from surviving multi-source edges (no stray parallel root)', async () => { + // Primary has `[A -> C, X -> B, [B,C] -> D]`. X is skipped, so + // `X -> B` is filtered and B loses its only upstream. The fan-in + // edge `[B,C] -> D` still has C as a reachable source, so it + // survives — but discovery must strip B from that edge's `from` + // list and prune B from `agentConfigs`. Leaving B behind would + // make it an incoming-less agent that `MultiAgentGraph.analyzeGraph` + // runs as an unintended parallel root. + const edges: GraphEdge[] = [ + { from: 'A', to: 'C', edgeType: 'handoff' }, + { from: 'X', to: 'B', edgeType: 'handoff' }, + { from: ['B', 'C'], to: 'D', edgeType: 'direct' }, + ]; + const primaryConfig = makeConfig('A', edges); + + const agentMap: Record = { + B: makeAgent('B', []), + C: makeAgent('C', []), + D: makeAgent('D', []), + X: makeAgent('X', []), + }; + const getAgent = jest.fn(async ({ id }: { id: string }) => agentMap[id] ?? null); + // X is forbidden; B, C, D pass. + const checkPermission = jest.fn( + async ({ resourceId }: { resourceId: unknown }) => resourceId !== 'mongo-X', + ); + + 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.skippedAgentIds.has('X')).toBe(true); + expect(result.agentConfigs.has('B')).toBe(false); + expect(result.agentConfigs.has('C')).toBe(true); + expect(result.agentConfigs.has('D')).toBe(true); + + // Every returned edge must reference only reachable agents — no B + // anywhere, not in a co-source slot and not as a node the SDK would + // need to register. + const endpoints = result.edges.flatMap((edge) => + [edge.from, edge.to].flatMap((v) => (Array.isArray(v) ? v : [v])), + ); + expect(endpoints).not.toContain('B'); + // The fan-in edge survived (C is a reachable co-source) but now + // lists only C as the source. + const fanInEdge = result.edges.find((edge) => { + const to = Array.isArray(edge.to) ? edge.to : [edge.to]; + return to.includes('D'); + }); + expect(fanInEdge).toBeDefined(); + expect(fanInEdge!.from).toEqual(['C']); + }); + + it('does not promote a downstream orphan to a parallel start when its only upstream is skipped', async () => { + // Primary A has `[A -> B, X -> Y]`. X is skipped (no VIEW), Y loads. + // Y had an incoming edge pre-filter (`X -> Y`), so it's a downstream + // agent — not a legitimate parallel starting node. When X is + // skipped, Y becomes a stranded orphan and must be pruned; the + // weaker "not pre-filter reachable from primary" rule would have + // incorrectly treated Y as an intentional parallel start. + const edges: GraphEdge[] = [ + { from: 'A', to: 'B', edgeType: 'handoff' }, + { from: 'X', to: 'Y', edgeType: 'direct' }, + ]; + const primaryConfig = makeConfig('A', edges); + + const agentMap: Record = { + B: makeAgent('B', []), + X: makeAgent('X', []), + Y: makeAgent('Y', []), + }; + const getAgent = jest.fn(async ({ id }: { id: string }) => agentMap[id] ?? null); + // X is forbidden — should be skipped. + const checkPermission = jest.fn( + async ({ resourceId }: { resourceId: unknown }) => resourceId !== 'mongo-X', + ); + + 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.skippedAgentIds.has('X')).toBe(true); + expect(result.agentConfigs.has('B')).toBe(true); + expect(result.agentConfigs.has('Y')).toBe(false); + expect(result.edges).toHaveLength(1); + expect(result.edges[0].to).toBe('B'); + }); + + it('preserves user-defined parallel-start branches disconnected from the primary', async () => { + // Primary A has edges `[A -> B, X -> Y]`. The `X -> Y` branch is + // intentionally disconnected from A — `MultiAgentGraph.analyzeGraph` + // treats X as a starting node (no incoming edges) and runs it in + // parallel with A. Discovery must keep this branch intact. + const edges: GraphEdge[] = [ + { from: 'A', to: 'B', edgeType: 'handoff' }, + { from: 'X', to: 'Y', edgeType: 'direct' }, + ]; + const primaryConfig = makeConfig('A', edges); + + const agentMap: Record = { + B: makeAgent('B', []), + X: makeAgent('X', []), + Y: makeAgent('Y', []), + }; + 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, + 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('X')).toBe(true); + expect(result.agentConfigs.has('Y')).toBe(true); + expect(result.edges).toHaveLength(2); + expect(result.edges.map((e) => e.to).sort()).toEqual(['B', 'Y']); + }); + + it('prunes surviving-but-unreachable edges from the return value (A->B->C->D, B skipped)', async () => { + // All three edges are stored on the primary A. When B is skipped, + // `filterOrphanedEdges` removes A->B (to=B) and B->C (from=B) — but + // `C->D` has no skipped endpoint and would otherwise survive, + // referencing agents that the reachability pass then prunes from + // `agentConfigs`. That combination is the exact shape that re-raises + // the "Found edge ending at unknown node" crash in `createRun`. + const edges: GraphEdge[] = [ + { from: 'A', to: 'B', edgeType: 'handoff' }, + { from: 'B', to: 'C', edgeType: 'handoff' }, + { from: 'C', to: 'D', edgeType: 'handoff' }, + ]; + const primaryConfig = makeConfig('A', edges); + + const agentMap: Record = { + B: makeAgent('B', []), + C: makeAgent('C', []), + D: makeAgent('D', []), + }; + const getAgent = jest.fn(async ({ id }: { id: string }) => agentMap[id] ?? null); + const checkPermission = jest.fn( + async ({ resourceId }: { resourceId: unknown }) => resourceId !== 'mongo-B', + ); + + 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.skippedAgentIds.has('B')).toBe(true); + expect(result.agentConfigs.has('C')).toBe(false); + expect(result.agentConfigs.has('D')).toBe(false); + // Every returned edge must reference only agents present in + // `agentConfigs` or the primary — no stranded `C->D` survives. + for (const edge of result.edges) { + const endpoints = [edge.from, edge.to].flatMap((v) => (Array.isArray(v) ? v : [v])); + for (const id of endpoints) { + expect(id === primaryConfig.id || result.agentConfigs.has(id)).toBe(true); + } + } + expect(result.edges).toHaveLength(0); + }); + + it('does not mutate a sub-agent userMCPAuthMap when merging later sub-agents', async () => { + // Primary has no MCP auth; first sub-agent (B) seeds `userMCPAuthMap`, + // and the second sub-agent (C) merges into it. Make sure B's own + // `config.userMCPAuthMap` object is not mutated by C's merge. + const bAuth = { serverB: { token: 'b' } }; + const cAuth = { serverC: { token: 'c' } }; + const primaryConfig = makeConfig('A', [ + { from: 'A', to: 'B', edgeType: 'handoff' }, + { from: 'A', to: 'C', edgeType: 'handoff' }, + ]); + primaryConfig.userMCPAuthMap = undefined; + + const agentMap: Record = { + B: makeAgent('B', []), + C: makeAgent('C', []), + }; + const authById: Record>> = { + B: bAuth, + C: cAuth, + }; + + const getAgent = jest.fn(async ({ id }: { id: string }) => agentMap[id] ?? null); + const checkPermission = jest.fn().mockResolvedValue(true); + mockInitializeAgent.mockImplementation(async ({ agent }: { agent: Agent }) => { + const cfg = makeConfig(agent.id); + cfg.userMCPAuthMap = authById[agent.id]; + 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, + }, + ); + + // B's own map must not have picked up C's entry. + expect(bAuth).toEqual({ serverB: { token: 'b' } }); + expect(cAuth).toEqual({ serverC: { token: 'c' } }); + // The returned merged map should still contain both. + expect(result.userMCPAuthMap).toEqual({ + serverB: { token: 'b' }, + serverC: { token: 'c' }, + }); + }); + + it('does not mutate the caller-supplied primaryConfig.userMCPAuthMap', async () => { + const primaryAuth = { serverA: { token: 'primary' } }; + const primaryConfig = makeConfig('A', [{ from: 'A', to: 'B', edgeType: 'handoff' }]); + primaryConfig.userMCPAuthMap = primaryAuth; + + 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, + }, + ); + + // Caller's map is unchanged — only the returned merged map has the + // sub-agent entries. + expect(primaryConfig.userMCPAuthMap).toBe(primaryAuth); + expect(primaryConfig.userMCPAuthMap).toEqual({ serverA: { token: 'primary' } }); + expect(result.userMCPAuthMap).toEqual({ + serverA: { token: 'primary' }, + serverB: { token: 'secondary' }, + }); + expect(result.userMCPAuthMap).not.toBe(primaryAuth); + }); + + it('keeps sub-agents that remain reachable via surviving edges', async () => { + // Graph: A -> [B, C]; B is skipped, C is kept. C must remain because + // A -> C survives filtering. + const edges: GraphEdge[] = [ + { from: 'A', to: 'B', edgeType: 'handoff' }, + { from: 'A', to: 'C', edgeType: 'handoff' }, + ]; + const primaryConfig = makeConfig('A', edges); + + const agentMap: Record = { + B: makeAgent('B', []), + C: makeAgent('C', []), + }; + const getAgent = jest.fn(async ({ id }: { id: string }) => agentMap[id] ?? null); + const checkPermission = jest.fn( + async ({ resourceId }: { resourceId: unknown }) => resourceId !== 'mongo-B', + ); + + 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('C')).toBe(true); + expect(result.agentConfigs.has('B')).toBe(false); + expect(result.edges).toHaveLength(1); + expect(result.edges[0].to).toBe('C'); + }); + + it('drops edges whose `from` references a skipped agent (bidirectional graph)', async () => { + // Orchestrator A with bidirectional handoffs A<->B. B is inaccessible, + // so `A -> B` and `B -> A` must both be filtered — otherwise `B -> A` + // leaks into createRun and triggers `Found edge ending at unknown node`. + const edges: GraphEdge[] = [ + { from: 'A', to: 'B', edgeType: 'handoff' }, + { from: 'B', to: 'A', edgeType: 'handoff' }, + ]; + const primaryConfig = makeConfig('A', edges); + + const getAgent = jest.fn(async () => makeAgent('B', [])); + const checkPermission = jest.fn().mockResolvedValue(false); + + 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.skippedAgentIds.has('B')).toBe(true); + expect(result.edges).toHaveLength(0); + }); + + 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 ServerRequest, + 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..e773c7be50 --- /dev/null +++ b/packages/api/src/agents/discovery.ts @@ -0,0 +1,450 @@ +import { logger } from '@librechat/data-schemas'; +import { ResourceType, PermissionBits, EModelEndpoint } 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; + /** + * ResourceType to check each sub-agent's access against. Defaults to + * `AGENT` for the in-app chat flow. Callers whose entry-point gates on + * a different resource type (e.g. the OpenAI-compat controllers gate on + * `REMOTE_AGENT`) must pass the matching resource type so sub-agents + * don't bypass the same sharing boundary enforced at the route. + */ + resourceType?: string; +} + +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, + resourceType = ResourceType.AGENT, + } = params; + + const { + getAgent, + checkPermission, + logViolation, + db, + onAgentInitialized, + onAgentSkipped, + initializeAgent = defaultInitializeAgent, + validateAgentModel = defaultValidateAgentModel, + } = deps; + + const agentConfigs = new Map(); + const skippedAgentIds = new Set(); + // Shallow-clone so the sub-agent merges below don't silently mutate + // `primaryConfig.userMCPAuthMap` on the caller's object. + let userMCPAuthMap: Record> | undefined = + primaryConfig.userMCPAuthMap ? { ...primaryConfig.userMCPAuthMap } : undefined; + + 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, + 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); + } + + /** + * Force `endpoint: agents` on the per-sub-agent init call so + * `initializeAgent`'s `isAgentsEndpoint`-gated `allowedProviders` + * check always fires for handoff sub-agents, regardless of which + * endpoint the caller entered through. Without this, the OpenAI- + * compat routes (whose `endpointOption.endpoint` is the primary + * provider, not `agents`) would silently bypass the provider + * allowlist configured under `endpoints.agents.allowedProviders`. + */ + const subAgentEndpointOption: Partial = { + ...(endpointOption ?? {}), + endpoint: EModelEndpoint.agents, + }; + + const config = await initializeAgent( + { + req, + res, + agent, + loadTools, + requestFiles, + conversationId, + parentMessageId, + endpointOption: subAgentEndpointOption, + allowedProviders, + }, + db, + ); + + if (userMCPAuthMap != null) { + Object.assign(userMCPAuthMap, config.userMCPAuthMap ?? {}); + } else if (config.userMCPAuthMap) { + // Clone so subsequent sub-agent merges don't mutate the first + // sub-agent's own `config.userMCPAuthMap` in place — symmetric with + // the shallow clone applied to the primary's map above. + 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 preFilterEdges = Array.from(edgeMap.values()); + const filteredEdges = filterOrphanedEdges(preFilterEdges, skippedAgentIds); + + /** + * Keep discovery's reachability model aligned with the agents SDK's + * runtime semantics. `MultiAgentGraph.createWorkflow` adds one + * LangGraph edge per `from` source, so a multi-source edge + * `{ from: ['A', 'B'], to: 'C' }` is really `A -> C` OR `B -> C` — + * either source firing routes to `C`. Reachability therefore advances + * through an edge whenever ANY of its sources is already reachable. + * + * Two semantics to reconcile when pruning after orphan-filter: + * + * 1. Accidental orphans — agents loaded via BFS from the primary's + * edges that lost their only path when an intermediate agent was + * skipped (e.g. `A -> B -> C` with B skipped leaves C stranded). + * These should be pruned; leaving them flips `createRun` into + * multi-agent mode with a disconnected C and the SDK runs C as an + * unintended parallel root. + * + * 2. Intentional multi-start branches — agents referenced by edges the + * user explicitly defined without wiring them to the primary + * (e.g. `A -> B` plus `X -> Y` as two independent starting + * branches). The SDK's `MultiAgentGraph.analyzeGraph` treats + * `no-incoming-edge` agents as start nodes, so these run in + * parallel with the primary by design. These must be preserved. + * + * Distinguish the two by asking: did the agent have any incoming edge + * in the user's original (pre-filter) graph? If yes, it was wired as + * a downstream step, and losing that wiring post-filter makes it an + * accidental orphan — prune. If no, the user declared it a start + * node; seed it so the SDK's `analyzeGraph` behavior of running + * incoming-less agents in parallel is preserved. + * + * "No incoming edge pre-filter" is stricter than "not reachable from + * primary pre-filter": a downstream agent like Y in `X -> Y` where X + * is skipped was never reachable from the primary pre-filter either, + * but it's still an orphan (its upstream X would have routed to it). + * The incoming-edge test catches that case correctly. + * + * - Post-filter reachability is seeded with the primary AND every + * agent in `agentConfigs` that had no pre-filter incoming edge + * (legitimate parallel start). + * - Agents whose pre-filter incoming edges got filtered out lose + * reachability and get pruned. + * - Surviving edges are filtered to the post-filter reachable set + * so no stale edge references a pruned agent. + * - Agents referenced as an endpoint in a surviving edge are always + * kept (a multi-source edge co-source like B in + * `{ from: ['A','B'], to: 'C' }` where nothing reaches B still + * needs B present for the SDK's per-source `addEdge` to compile). + */ + const anyReachable = (value: string | string[], reachableSet: Set): boolean => { + const ids = Array.isArray(value) ? value : [value]; + return ids.some((id) => typeof id === 'string' && reachableSet.has(id)); + }; + const allReachable = (value: string | string[], reachableSet: Set): boolean => { + const ids = Array.isArray(value) ? value : [value]; + return ids.every((id) => typeof id !== 'string' || reachableSet.has(id)); + }; + const expandReachable = (seeds: Set, edgeList: GraphEdge[]): Set => { + const result = new Set(seeds); + let changed = true; + while (changed) { + changed = false; + for (const edge of edgeList) { + if (!anyReachable(edge.from, result)) { + continue; + } + const dests = Array.isArray(edge.to) ? edge.to : [edge.to]; + for (const dest of dests) { + if (typeof dest === 'string' && !result.has(dest)) { + result.add(dest); + changed = true; + } + } + } + } + return result; + }; + + // A legitimate parallel-start agent is one that has NO incoming edge + // in the pre-filter graph — the user declared it as a starting node. + // "Not reachable from primary pre-filter" is too permissive: a + // downstream agent whose only upstream got skipped (`X -> Y` with X + // skipped but Y loaded) would qualify under that weaker rule and be + // promoted to a parallel root even though it's actually a stranded + // orphan. Using "no incoming edge in pre-filter" tightens the criterion + // to match the SDK's `analyzeGraph` definition of a start node applied + // to the user's ORIGINAL graph topology, before any orphan filtering. + const hadIncomingEdgePreFilter = new Set(); + for (const edge of preFilterEdges) { + const dests = Array.isArray(edge.to) ? edge.to : [edge.to]; + for (const dest of dests) { + if (typeof dest === 'string') { + hadIncomingEdgePreFilter.add(dest); + } + } + } + + const postFilterSeeds = new Set([primaryConfig.id]); + for (const agentId of agentConfigs.keys()) { + if (!hadIncomingEdgePreFilter.has(agentId)) { + postFilterSeeds.add(agentId); + } + } + + const reachable = expandReachable(postFilterSeeds, filteredEdges); + + /** + * Filter + sanitize edges: + * - Keep an edge if at least one `from` source is reachable AND every + * `to` destination is reachable (a missing destination would still + * crash `StateGraph.compile` with `Found edge ending at unknown + * node`). + * - For kept edges with an array `from`, strip out unreachable + * co-sources. The SDK's per-source `addEdge` fires independently + * (each source becomes its own `addEdge(source, dest)` call), so + * losing an unreachable co-source doesn't invalidate the routes + * through the surviving ones. Leaving the dead co-source in the + * array was propping up agents that `reachable` had already + * excluded — in `MultiAgentGraph.analyzeGraph` they'd then show up + * as incoming-less nodes and execute as unintended parallel roots. + * + * After sanitization every endpoint in every surviving edge is + * guaranteed to be in `reachable`, which lets the agent prune below + * collapse to a strict reachability check. + */ + const edges: GraphEdge[] = []; + for (const edge of filteredEdges) { + if (!anyReachable(edge.from, reachable) || !allReachable(edge.to, reachable)) { + continue; + } + if (!Array.isArray(edge.from)) { + edges.push(edge); + continue; + } + const reachableSources = edge.from.filter((s) => typeof s !== 'string' || reachable.has(s)); + if (reachableSources.length === edge.from.length) { + edges.push(edge); + } else { + edges.push({ ...edge, from: reachableSources }); + } + } + + for (const agentId of [...agentConfigs.keys()]) { + if (!reachable.has(agentId)) { + agentConfigs.delete(agentId); + } + } + + return { + agentConfigs, + edges, + skippedAgentIds, + userMCPAuthMap, + }; +} diff --git a/packages/api/src/agents/edges.spec.ts b/packages/api/src/agents/edges.spec.ts index b23f00f63f..447c65fdb5 100644 --- a/packages/api/src/agents/edges.spec.ts +++ b/packages/api/src/agents/edges.spec.ts @@ -131,36 +131,66 @@ describe('edges utilities', () => { expect(filterOrphanedEdges(edges, skipped)).toEqual(edges); }); - it('should filter out edges with orphaned to targets', () => { + it('should filter out edges with orphaned to targets and orphaned from sources', () => { const skipped = new Set(['agent_b']); const result = filterOrphanedEdges(edges, skipped); - // Only filters edges where `to` is in skipped set // agent_a -> agent_b (filtered - to=agent_b is skipped) - // agent_a -> agent_c (kept - to=agent_c not skipped) - // agent_b -> agent_d (kept - to=agent_d not skipped, from is not checked) - expect(result).toHaveLength(2); - expect(result.map((e) => e.to)).toEqual(['agent_c', 'agent_d']); + // agent_a -> agent_c (kept - neither endpoint skipped) + // agent_b -> agent_d (filtered - from=agent_b is skipped; a source-side + // orphan would otherwise leave the graph referencing an absent agent) + expect(result).toHaveLength(1); + expect(result[0].to).toBe('agent_c'); }); it('should filter out multiple orphaned edges', () => { const skipped = new Set(['agent_b', 'agent_c']); const result = filterOrphanedEdges(edges, skipped); - // agent_a -> agent_b (filtered) - // agent_a -> agent_c (filtered) - // agent_b -> agent_d (kept - to=agent_d not skipped) - expect(result).toHaveLength(1); - expect(result[0].to).toBe('agent_d'); + // agent_a -> agent_b (filtered, to) + // agent_a -> agent_c (filtered, to) + // agent_b -> agent_d (filtered, from) + expect(result).toHaveLength(0); }); - it('should handle array to values', () => { + it('strips skipped co-sources from multi-source edges instead of dropping the whole edge', () => { + // The SDK adds one `builder.addEdge(source, dest)` per source, so + // `agent_a -> agent_d` is a valid route even when `agent_b` can't + // be loaded. Keep the edge with the surviving sources. + const edgesWithArrayFrom: GraphEdge[] = [ + { from: ['agent_a', 'agent_b'], to: 'agent_d', edgeType: 'handoff' }, + { from: 'agent_a', to: 'agent_d', edgeType: 'handoff' }, + ]; + const skipped = new Set(['agent_b']); + const result = filterOrphanedEdges(edgesWithArrayFrom, skipped); + expect(result).toHaveLength(2); + expect(result[0].from).toEqual(['agent_a']); + expect(result[0].to).toBe('agent_d'); + expect(result[1].from).toBe('agent_a'); + }); + + it('strips skipped co-destinations from multi-destination edges', () => { const edgesWithArray: GraphEdge[] = [ { from: 'agent_a', to: ['agent_b', 'agent_c'], edgeType: 'handoff' }, { from: 'agent_a', to: ['agent_d'], edgeType: 'handoff' }, ]; const skipped = new Set(['agent_b']); const result = filterOrphanedEdges(edgesWithArray, skipped); - expect(result).toHaveLength(1); - expect(result[0].to).toEqual(['agent_d']); + // First edge: `['agent_b','agent_c']` → `['agent_c']` — kept. + // Second edge: unchanged. + expect(result).toHaveLength(2); + expect(result[0].to).toEqual(['agent_c']); + expect(result[1].to).toEqual(['agent_d']); + }); + + it('drops multi-member edges only when every member on a side is skipped', () => { + const edgesSide: GraphEdge[] = [ + // All sources skipped → drop. + { from: ['agent_b', 'agent_c'], to: 'agent_a', edgeType: 'handoff' }, + // All destinations skipped → drop. + { from: 'agent_a', to: ['agent_b', 'agent_c'], edgeType: 'handoff' }, + ]; + const skipped = new Set(['agent_b', 'agent_c']); + const result = filterOrphanedEdges(edgesSide, skipped); + expect(result).toHaveLength(0); }); it('should return original edges array when edges is null/undefined', () => { diff --git a/packages/api/src/agents/edges.ts b/packages/api/src/agents/edges.ts index 9a36105b74..9bc3705c60 100644 --- a/packages/api/src/agents/edges.ts +++ b/packages/api/src/agents/edges.ts @@ -30,17 +30,80 @@ export function getEdgeParticipants(edge: GraphEdge): string[] { } /** - * Filters out edges that reference non-existent (orphaned) agents. - * Only filters based on the 'to' field since those are the handoff targets. + * Drops skipped agent ids from an edge endpoint. + * + * Scalars: returns the value unchanged, or `null` if the skipped set + * contains it (the edge has lost its only endpoint on that side and + * must be dropped by the caller). + * + * Arrays: returns a new array with skipped ids removed; if every id was + * skipped, returns `null`. A single-element result stays an array — the + * SDK normalizes both shapes internally, so preserving array-ness is + * simpler than switching representation. + * + * Returns the original reference when nothing changed, so callers can + * cheaply detect and skip a re-spread. + */ +function sanitizeEndpoint( + value: string | string[], + skippedAgentIds: Set, +): string | string[] | null { + if (Array.isArray(value)) { + const kept: string[] = []; + let removed = false; + for (const id of value) { + if (typeof id === 'string' && skippedAgentIds.has(id)) { + removed = true; + continue; + } + kept.push(id); + } + if (kept.length === 0) { + return null; + } + return removed ? kept : value; + } + if (typeof value === 'string' && skippedAgentIds.has(value)) { + return null; + } + return value; +} + +/** + * Drops orphaned agents from edge endpoints, returning only the edges + * that still have at least one valid source and one valid destination. + * + * For multi-source / multi-destination edges, orphaned ids are stripped + * from the array rather than dropping the whole edge: `{ from: ['A','B'], + * to: 'C' }` with B skipped becomes `{ from: ['A'], to: 'C' }`. This + * matches the agents SDK's `MultiAgentGraph.createWorkflow`, which adds + * one LangGraph edge per source/destination pair (`builder.addEdge(src, + * dest)`), so losing one co-source (or one co-destination) doesn't + * invalidate the routes through the remaining members. + * + * An edge is dropped only when it has no surviving source OR no + * surviving destination — the SDK's per-source `addEdge` can't run if + * either side is empty, and any orphaned id left in place would still + * trigger `Found edge ending at unknown node` at compile time. */ export function filterOrphanedEdges(edges: GraphEdge[], skippedAgentIds: Set): GraphEdge[] { if (!edges || skippedAgentIds.size === 0) { return edges; } - return edges.filter((edge) => { - const toIds = Array.isArray(edge.to) ? edge.to : [edge.to]; - return !toIds.some((id) => typeof id === 'string' && skippedAgentIds.has(id)); - }); + const result: GraphEdge[] = []; + for (const edge of edges) { + const from = sanitizeEndpoint(edge.from, skippedAgentIds); + const to = sanitizeEndpoint(edge.to, skippedAgentIds); + if (from === null || to === null) { + continue; + } + if (from === edge.from && to === edge.to) { + result.push(edge); + } else { + result.push({ ...edge, from, to }); + } + } + return result; } /** Collects all unique agent IDs referenced across an array of edges. */ 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 ee37c26f44..a59153c597 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,