-
-
Notifications
You must be signed in to change notification settings - Fork 7.5k
π€ fix: Load Handoff Agents for Agents API #12740
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 12 commits
e9e435b
5cc74e3
7418dd8
e491a23
7ec1ba2
4cbfffc
e54440b
e000354
6450aa8
4982f1c
6879f45
222716d
0de3684
7cc7aed
54de02c
6ce3462
718b000
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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,79 @@ 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<string, { | ||
| * agent: object, | ||
| * toolRegistry?: import('@librechat/agents').LCToolRegistry, | ||
| * userMCPAuthMap?: Record<string, Record<string, string>>, | ||
| * 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, | ||
| checkPermission, | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This discovery call checks sub-agents with Useful? React with πΒ / π.
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good catch β confirmed. Fixed in 0de3684. Both API controllers now build their |
||
| 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 +347,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 +545,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, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Passing
filterFilesByAgentAccessintoinitializeAgenthere makes remote API calls enforceResourceType.AGENTon attached context files (api/server/services/Files/permissions.jsusesresourceType: ResourceType.AGENTat the access check), even though this route authorizes callers withREMOTE_AGENTpermissions viagetRemoteAgentPermissions. A user who hasREMOTE_AGENT_VIEWERbut noAGENT_VIEWcan invoke the shared agent, but all owner-attached context files get filtered out, so tools likefile_search/context-backed retrieval silently stop working for remote consumers; the same regression is also introduced inresponses.jswith the identicaldbMethodswiring.Useful? React with πΒ / π.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right β this was a regression I introduced when I refactored the DB-method bundle into a shared
dbMethodsobject. Fixed in 6ce3462: both API controllers now omitfilterFilesByAgentAccessfromdbMethods(and drop the now-unused import). The in-app chatinitialize.jskeeps it because that path legitimately authorizes at the AGENT level.This restores the pre-refactor API behavior β
primeResourcesseesfilterFiles: undefinedand skips the per-file ACL check, soREMOTE_AGENT_VIEWER-only callers get the owner-attached context files the route'sgetRemoteAgentPermissionshas already decided they can see.