From 70b6bb69d37f1cbb53e5316e65c1cafc4a86de30 Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Mon, 11 May 2026 08:53:53 -0400 Subject: [PATCH] =?UTF-8?q?=F0=9F=A7=AC=20fix:=20Bound=20Subagent=20Expans?= =?UTF-8?q?ion=20(#13064)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: Bound subagent expansion * fix: Preserve subagent path depth --- .../services/Endpoints/agents/initialize.js | 87 ++++++++---- .../Endpoints/agents/initialize.spec.js | 131 ++++++++++++++++++ .../__tests__/run-summarization.test.ts | 84 ++++++++++- packages/api/src/agents/run.ts | 70 ++++++++-- packages/data-provider/src/config.ts | 9 ++ 5 files changed, 339 insertions(+), 42 deletions(-) diff --git a/api/server/services/Endpoints/agents/initialize.js b/api/server/services/Endpoints/agents/initialize.js index 9f24438b5f..8f673c11bc 100644 --- a/api/server/services/Endpoints/agents/initialize.js +++ b/api/server/services/Endpoints/agents/initialize.js @@ -15,9 +15,11 @@ const { ResourceType, EModelEndpoint, PermissionBits, + MAX_SUBAGENT_DEPTH, isAgentsEndpoint, getResponseSender, AgentCapabilities, + MAX_SUBAGENT_GRAPH_NODES, isEphemeralAgentId, } = require('librechat-data-provider'); const { @@ -637,6 +639,19 @@ const initializeClient = async ({ req, res, signal, endpointOption }) => { * silently break nested delegation like A → B → C where B is only * a subagent of A. */ const pureSubagentIds = new Set(); + const subagentGraphIds = new Set(); + const loadedSubagentConfigIds = new Set(); + + const assertSubagentGraphRoom = (agentId) => { + if (subagentGraphIds.has(agentId)) { + return; + } + if (subagentGraphIds.size >= MAX_SUBAGENT_GRAPH_NODES) { + throw new Error( + `Subagent graph exceeds the maximum of ${MAX_SUBAGENT_GRAPH_NODES} unique agents.`, + ); + } + }; /** * Loads `subagentAgentConfigs` for a single agent config. Shared @@ -646,13 +661,22 @@ const initializeClient = async ({ req, res, signal, endpointOption }) => { * gets them honored at runtime. Self-spawn works regardless (no DB * lookup needed). Pruning decisions are deferred to `pureSubagentIds`. */ - const loadSubagentsFor = async (config) => { + const loadSubagentsFor = async (config, depth = 0) => { const sub = config.subagents; if (!subagentsCapabilityEnabled || !sub?.enabled) { config.subagentAgentConfigs = []; return; } + if (loadedSubagentConfigIds.has(config.id)) { + if ((config.subagentAgentConfigs?.length ?? 0) > 0 && depth >= MAX_SUBAGENT_DEPTH) { + throw new Error( + `Subagent graph exceeds the maximum depth of ${MAX_SUBAGENT_DEPTH} at agent ${config.id}.`, + ); + } + return; + } + /** Dedupe and filter in one pass — a crafted payload could * legitimately include the same ID twice; the backend shouldn't * create duplicate SubagentConfig entries for the LLM to see as @@ -665,6 +689,14 @@ const initializeClient = async ({ req, res, signal, endpointOption }) => { ), ); + if (explicitSubagentIds.length > 0 && depth >= MAX_SUBAGENT_DEPTH) { + throw new Error( + `Subagent graph exceeds the maximum depth of ${MAX_SUBAGENT_DEPTH} at agent ${config.id}.`, + ); + } + + loadedSubagentConfigIds.add(config.id); + /** @type {Array} */ const resolved = []; for (const subagentId of explicitSubagentIds) { @@ -680,9 +712,11 @@ const initializeClient = async ({ req, res, signal, endpointOption }) => { continue; } + assertSubagentGraphRoom(subagentId); const subagentConfig = await loadAgentById(subagentId); if (!subagentConfig) continue; + subagentGraphIds.add(subagentConfig.id ?? subagentId); resolved.push(subagentConfig); if (!edgeAgentIds.has(subagentId)) { @@ -693,35 +727,32 @@ const initializeClient = async ({ req, res, signal, endpointOption }) => { config.subagentAgentConfigs = resolved; }; - /** BFS across the primary's subagent tree so nested chains like - * A → B → C get resolved before any pruning. Each config is - * visited once. */ - const visitedConfigIds = new Set(); - const pending = [primaryConfig]; - while (pending.length > 0) { - const cfg = pending.shift(); - if (!cfg || visitedConfigIds.has(cfg.id)) continue; - visitedConfigIds.add(cfg.id); - await loadSubagentsFor(cfg); - for (const child of cfg.subagentAgentConfigs ?? []) { - if (child?.id && !visitedConfigIds.has(child.id)) { - pending.push(child); + const maxResolvedDepthByConfigId = new Map(); + + /** BFS across subagent trees so nested chains like A → B → C get + * resolved before any pruning. Agent configs are loaded once, but + * overlapping roots can still be revisited at deeper path depths so + * the depth guard observes the deepest reachable subagent path. */ + const resolveSubagentTrees = async (rootConfigs) => { + const pending = rootConfigs.map((cfg) => ({ cfg, depth: 0 })); + for (let index = 0; index < pending.length; index++) { + const { cfg, depth } = pending[index]; + if (!cfg?.id) continue; + const previousDepth = maxResolvedDepthByConfigId.get(cfg.id); + if (previousDepth != null && previousDepth >= depth) continue; + maxResolvedDepthByConfigId.set(cfg.id, depth); + await loadSubagentsFor(cfg, depth); + for (const child of cfg.subagentAgentConfigs ?? []) { + const childDepth = depth + 1; + const previousChildDepth = child?.id ? maxResolvedDepthByConfigId.get(child.id) : undefined; + if (child?.id && (previousChildDepth == null || previousChildDepth < childDepth)) { + pending.push({ cfg: child, depth: childDepth }); + } } } - } - /** Handoff targets still in the map that weren't visited via the - * primary's subagent tree also need their subagents resolved. */ - for (const [id, cfg] of agentConfigs.entries()) { - if (id === primaryConfig.id || visitedConfigIds.has(id)) continue; - visitedConfigIds.add(id); - await loadSubagentsFor(cfg); - for (const child of cfg.subagentAgentConfigs ?? []) { - if (child?.id && !visitedConfigIds.has(child.id)) { - visitedConfigIds.add(child.id); - await loadSubagentsFor(child); - } - } - } + }; + + await resolveSubagentTrees([primaryConfig, ...agentConfigs.values()]); /** Drop pure-subagent entries now that every reachable config has * had its subagents resolved. They stay in `agentToolContexts` so diff --git a/api/server/services/Endpoints/agents/initialize.spec.js b/api/server/services/Endpoints/agents/initialize.spec.js index 2af23f1485..61f5bdfcf0 100644 --- a/api/server/services/Endpoints/agents/initialize.spec.js +++ b/api/server/services/Endpoints/agents/initialize.spec.js @@ -4,6 +4,8 @@ const { PermissionBits, PrincipalType, PrincipalModel, + MAX_SUBAGENT_DEPTH, + MAX_SUBAGENT_GRAPH_NODES, } = require('librechat-data-provider'); const { MongoMemoryServer } = require('mongodb-memory-server'); @@ -311,6 +313,24 @@ describe('initializeClient — subagent loading', () => { maxContextTokens: 4096, }); + const makeNestedSubagentConfig = (id, childIds = []) => ({ + ...makeSubagentConfig(id), + subagents: { enabled: true, allowSelf: false, agent_ids: childIds }, + }); + + const createViewableAgent = async (id) => { + const agent = await createAgent({ + id, + name: id, + provider: 'openai', + model: 'gpt-4', + author: new mongoose.Types.ObjectId(), + tools: [], + }); + await grantView(agent); + return agent; + }; + it('loads a configured subagent, populates `subagentAgentConfigs`, and keeps it out of `agentConfigs`', async () => { const subAgent = await createAgent({ id: SUBAGENT_ID, @@ -453,6 +473,117 @@ describe('initializeClient — subagent loading', () => { expect(agentClientArgs.agent.subagentAgentConfigs).toHaveLength(1); }); + it('rejects nested subagent chains deeper than MAX_SUBAGENT_DEPTH', async () => { + const ids = Array.from( + { length: MAX_SUBAGENT_DEPTH + 1 }, + (_, index) => `agent_depth_${index}`, + ); + for (const id of ids) { + await createViewableAgent(id); + } + + const primaryConfig = makePrimaryConfig({ + subagents: { enabled: true, allowSelf: false, agent_ids: [ids[0]] }, + }); + const nestedConfigs = new Map( + ids.map((id, index) => [ + id, + makeNestedSubagentConfig(id, index < ids.length - 1 ? [ids[index + 1]] : []), + ]), + ); + + mockInitializeAgent.mockImplementation(({ agent }) => + Promise.resolve(agent.id === PRIMARY_ID ? primaryConfig : nestedConfigs.get(agent.id)), + ); + + await expect( + initializeClient({ + req: makeSubagentReq(), + res: {}, + signal: new AbortController().signal, + endpointOption: makeEndpointOption(), + }), + ).rejects.toThrow(`maximum depth of ${MAX_SUBAGENT_DEPTH}`); + expect(agentClientArgs).toBeUndefined(); + }); + + it('preserves deeper path depth when a handoff root is also a nested subagent', async () => { + const chainIds = Array.from( + { length: MAX_SUBAGENT_DEPTH }, + (_, index) => `agent_overlap_depth_${index}`, + ); + const allIds = [HANDOFF_AND_SUB_ID, ...chainIds]; + for (const id of allIds) { + await createViewableAgent(id); + } + + const edges = [{ from: PRIMARY_ID, to: HANDOFF_AND_SUB_ID, edgeType: 'handoff' }]; + const primaryConfig = makePrimaryConfig({ + edges, + subagents: { enabled: true, allowSelf: false, agent_ids: [HANDOFF_AND_SUB_ID] }, + }); + const nestedConfigs = new Map([ + [HANDOFF_AND_SUB_ID, makeNestedSubagentConfig(HANDOFF_AND_SUB_ID, [chainIds[0]])], + ...chainIds.map((id, index) => [ + id, + makeNestedSubagentConfig(id, index < chainIds.length - 1 ? [chainIds[index + 1]] : []), + ]), + ]); + + mockInitializeAgent.mockImplementation(({ agent }) => + Promise.resolve(agent.id === PRIMARY_ID ? primaryConfig : nestedConfigs.get(agent.id)), + ); + + await expect( + initializeClient({ + req: makeSubagentReq(), + res: {}, + signal: new AbortController().signal, + endpointOption: makeEndpointOption(), + }), + ).rejects.toThrow(`maximum depth of ${MAX_SUBAGENT_DEPTH}`); + expect(agentClientArgs).toBeUndefined(); + }); + + it('rejects subagent graphs that exceed MAX_SUBAGENT_GRAPH_NODES unique agents', async () => { + const firstLevelIds = Array.from({ length: 10 }, (_, index) => `agent_graph_${index}`); + const secondLevelIdsByParent = new Map( + firstLevelIds.map((id) => [ + id, + Array.from({ length: 5 }, (_, index) => `${id}_child_${index}`), + ]), + ); + const allIds = [...firstLevelIds, ...Array.from(secondLevelIdsByParent.values()).flat()]; + + for (const id of allIds) { + await createViewableAgent(id); + } + + const primaryConfig = makePrimaryConfig({ + subagents: { enabled: true, allowSelf: false, agent_ids: firstLevelIds }, + }); + const nestedConfigs = new Map( + firstLevelIds.map((id) => [id, makeNestedSubagentConfig(id, secondLevelIdsByParent.get(id))]), + ); + for (const id of Array.from(secondLevelIdsByParent.values()).flat()) { + nestedConfigs.set(id, makeNestedSubagentConfig(id)); + } + + mockInitializeAgent.mockImplementation(({ agent }) => + Promise.resolve(agent.id === PRIMARY_ID ? primaryConfig : nestedConfigs.get(agent.id)), + ); + + await expect( + initializeClient({ + req: makeSubagentReq(), + res: {}, + signal: new AbortController().signal, + endpointOption: makeEndpointOption(), + }), + ).rejects.toThrow(`maximum of ${MAX_SUBAGENT_GRAPH_NODES} unique agents`); + expect(agentClientArgs).toBeUndefined(); + }); + it('keeps an agent in `agentConfigs` when it is BOTH a handoff target and a subagent', async () => { /** Overlap case: the same child is used both via handoff edges (needs to * be in agentConfigs) and as a subagent (needs to be in diff --git a/packages/api/src/agents/__tests__/run-summarization.test.ts b/packages/api/src/agents/__tests__/run-summarization.test.ts index ad47e43b0a..b3fb1577dd 100644 --- a/packages/api/src/agents/__tests__/run-summarization.test.ts +++ b/packages/api/src/agents/__tests__/run-summarization.test.ts @@ -1,6 +1,11 @@ import type { AppConfig } from '@librechat/data-schemas'; import type { SummarizationConfig, TEndpoint } from 'librechat-data-provider'; -import { EModelEndpoint, FileSources } from 'librechat-data-provider'; +import { + EModelEndpoint, + FileSources, + MAX_SUBAGENT_DEPTH, + MAX_SUBAGENT_RUN_CONFIGS, +} from 'librechat-data-provider'; import { createRun } from '~/agents/run'; // Mock winston logger @@ -53,6 +58,57 @@ function makeAgent( }; } +type TestRunAgent = ReturnType & { + subagentAgentConfigs?: TestRunAgent[]; +}; + +function makeSubagentChain(hops: number): TestRunAgent { + const agents = Array.from({ length: hops + 1 }, (_, index) => + makeAgent({ + id: `agent_chain_${index}`, + name: `Chain ${index}`, + }), + ) as TestRunAgent[]; + + for (let index = 0; index < hops; index++) { + const child = agents[index + 1]; + agents[index].subagents = { enabled: true, allowSelf: false, agent_ids: [child.id] }; + agents[index].subagentAgentConfigs = [child]; + } + + return agents[0]; +} + +function makeLayeredSubagentDag(width: number, depth: number): TestRunAgent { + const root = makeAgent({ id: 'agent_dag_root', name: 'DAG Root' }) as TestRunAgent; + const layers: TestRunAgent[][] = [[root]]; + + for (let level = 1; level <= depth; level++) { + layers.push( + Array.from({ length: width }, (_, index) => + makeAgent({ + id: `agent_dag_${level}_${index}`, + name: `DAG ${level}.${index}`, + }), + ) as TestRunAgent[], + ); + } + + for (let level = 0; level < depth; level++) { + const children = layers[level + 1]; + for (const agent of layers[level]) { + agent.subagents = { + enabled: true, + allowSelf: false, + agent_ids: children.map((child) => child.id), + }; + agent.subagentAgentConfigs = children; + } + } + + return root; +} + /** Helper: call createRun and return the captured agentInputs array */ async function callAndCapture( opts: { @@ -940,6 +996,32 @@ describe('subagentConfigs', () => { expect(childInputs.initialSummary).toBeUndefined(); expect(childInputs.discoveredTools).toBeUndefined(); }); + + it('rejects subagent graphs deeper than MAX_SUBAGENT_DEPTH before Run.create', async () => { + await expect( + createRun({ + agents: [makeSubagentChain(MAX_SUBAGENT_DEPTH + 1)] as never, + signal: new AbortController().signal, + streaming: true, + streamUsage: true, + }), + ).rejects.toThrow(`maximum depth of ${MAX_SUBAGENT_DEPTH}`); + + expect(Run.create).not.toHaveBeenCalled(); + }); + + it('rejects layered DAGs that exceed MAX_SUBAGENT_RUN_CONFIGS expanded entries', async () => { + await expect( + createRun({ + agents: [makeLayeredSubagentDag(3, MAX_SUBAGENT_DEPTH)] as never, + signal: new AbortController().signal, + streaming: true, + streamUsage: true, + }), + ).rejects.toThrow(`maximum of ${MAX_SUBAGENT_RUN_CONFIGS} expanded entries`); + + expect(Run.create).not.toHaveBeenCalled(); + }); }); // --------------------------------------------------------------------------- diff --git a/packages/api/src/agents/run.ts b/packages/api/src/agents/run.ts index 73fa9e5efc..8e5f80f7e6 100644 --- a/packages/api/src/agents/run.ts +++ b/packages/api/src/agents/run.ts @@ -2,6 +2,8 @@ import { logger } from '@librechat/data-schemas'; import { Run, Providers, Constants } from '@librechat/agents'; import { KnownEndpoints, + MAX_SUBAGENT_DEPTH, + MAX_SUBAGENT_RUN_CONFIGS, extractEnvVariable, providerEndpointMap, normalizeEndpointName, @@ -547,6 +549,27 @@ function computeEffectiveMaxContextTokens( /** Identifier for the self-spawn subagent (reuses parent's AgentInputs in an isolated child graph). */ const SELF_SUBAGENT_TYPE = 'self'; +interface SubagentBuildState { + configCount: number; +} + +function countSubagentConfig(state: SubagentBuildState): void { + state.configCount += 1; + if (state.configCount > MAX_SUBAGENT_RUN_CONFIGS) { + throw new Error( + `Subagent run configuration exceeds the maximum of ${MAX_SUBAGENT_RUN_CONFIGS} expanded entries.`, + ); + } +} + +function assertSubagentDepth(depth: number, agentId: string): void { + if (depth > MAX_SUBAGENT_DEPTH) { + throw new Error( + `Subagent graph exceeds the maximum depth of ${MAX_SUBAGENT_DEPTH} at agent ${agentId}.`, + ); + } +} + /** * Recursive any-true check across the agent tree: returns `true` if this * agent or any subagent (transitively) has the per-agent codeenv gate @@ -559,14 +582,17 @@ const SELF_SUBAGENT_TYPE = 'self'; * run — without it, the subagent's `{{toolturn}}` * placeholders would pass through to the shell unsubstituted. * - * Cycle-safe via a `visited` set, mirroring `buildSubagentConfigs`'s - * `ancestors` pattern. The bash tool description itself is still gated - * per-agent in `initializeAgent`, so only agents that actually have - * bash registered learn the `{{…}}` syntax — broadening the run-level + * Cycle-safe via a `visited` set. The bash tool description itself is + * still gated per-agent in `initializeAgent`, so only agents that actually + * have bash registered learn the `{{…}}` syntax — broadening the run-level * registry gate doesn't broaden the model-facing surface. */ -function anyAgentHasCodeEnv(agents: RunAgent[], visited: Set = new Set()): boolean { - for (const agent of agents) { +function anyAgentHasCodeEnv(agents: RunAgent[]): boolean { + const visited = new Set(); + const pending = [...agents]; + + for (let index = 0; index < pending.length; index++) { + const agent = pending[index]; if (visited.has(agent.id)) { continue; } @@ -574,11 +600,10 @@ function anyAgentHasCodeEnv(agents: RunAgent[], visited: Set = new Set() if (agent.codeEnvAvailable === true) { return true; } - if ( - agent.subagentAgentConfigs != null && - anyAgentHasCodeEnv(agent.subagentAgentConfigs, visited) - ) { - return true; + for (const child of agent.subagentAgentConfigs ?? []) { + if (!visited.has(child.id)) { + pending.push(child); + } } } return false; @@ -593,7 +618,9 @@ function buildSubagentConfigs( agent: RunAgent, agentInput: AgentInputs, toInput: (child: RunAgent, opts?: { isSubagent?: boolean }) => AgentInputs, + state: SubagentBuildState, ancestors: Set = new Set(), + depth = 0, ): SubagentConfig[] { if (!agent.subagents?.enabled) { return []; @@ -604,6 +631,7 @@ function buildSubagentConfigs( if (allowSelf) { const selfName = agentInput.name ?? agent.name ?? 'self'; + countSubagentConfig(state); configs.push({ self: true, type: SELF_SUBAGENT_TYPE, @@ -627,6 +655,9 @@ function buildSubagentConfigs( if (ancestors.has(child.id)) { continue; } + const childDepth = depth + 1; + assertSubagentDepth(childDepth, child.id); + countSubagentConfig(state); /** * `buildAgentInput` applies parent-run context (initialSummary + * discoveredTools) to the returned AgentInputs *and* to the @@ -649,7 +680,14 @@ function buildSubagentConfigs( * `subagentConfigs`, and that only runs for the outer agents in * `agents[]`. Cycle-safe via `nextAncestors`. */ - const grandchildConfigs = buildSubagentConfigs(child, childInputs, toInput, nextAncestors); + const grandchildConfigs = buildSubagentConfigs( + child, + childInputs, + toInput, + state, + nextAncestors, + childDepth, + ); if (grandchildConfigs.length > 0) { childInputs.subagentConfigs = grandchildConfigs; } @@ -884,9 +922,15 @@ export async function createRun({ }; const agentInputs: AgentInputs[] = []; + const subagentBuildState: SubagentBuildState = { configCount: 0 }; for (const agent of agents) { const agentInput = buildAgentInput(agent); - const subagentConfigs = buildSubagentConfigs(agent, agentInput, buildAgentInput); + const subagentConfigs = buildSubagentConfigs( + agent, + agentInput, + buildAgentInput, + subagentBuildState, + ); if (subagentConfigs.length > 0) { agentInput.subagentConfigs = subagentConfigs; } diff --git a/packages/data-provider/src/config.ts b/packages/data-provider/src/config.ts index 38615d5d46..e7c0add133 100644 --- a/packages/data-provider/src/config.ts +++ b/packages/data-provider/src/config.ts @@ -2179,6 +2179,15 @@ export enum Constants { /** Maximum number of explicit subagents per parent agent. UI + Zod schema share this. */ export const MAX_SUBAGENTS = 10; +/** Maximum explicit subagent hops allowed from any root agent at runtime. */ +export const MAX_SUBAGENT_DEPTH = 5; + +/** Maximum unique explicit subagent targets that may be loaded at runtime. */ +export const MAX_SUBAGENT_GRAPH_NODES = 50; + +/** Maximum expanded SubagentConfig entries embedded into one run request. */ +export const MAX_SUBAGENT_RUN_CONFIGS = 100; + export enum LocalStorageKeys { /** Key for the admin defined App Title */ APP_TITLE = 'appTitle',