diff --git a/packages/api/src/agents/__tests__/run-summarization.test.ts b/packages/api/src/agents/__tests__/run-summarization.test.ts index ad47e43b0a..b5933e7198 100644 --- a/packages/api/src/agents/__tests__/run-summarization.test.ts +++ b/packages/api/src/agents/__tests__/run-summarization.test.ts @@ -1,19 +1,39 @@ import type { AppConfig } from '@librechat/data-schemas'; +import { ToolMessage } from '@librechat/agents/langchain/messages'; +import type { BaseMessage } from '@librechat/agents/langchain/messages'; import type { SummarizationConfig, TEndpoint } from 'librechat-data-provider'; import { EModelEndpoint, FileSources } from 'librechat-data-provider'; import { createRun } from '~/agents/run'; // Mock winston logger -jest.mock('winston', () => ({ - createLogger: jest.fn(() => ({ - debug: jest.fn(), - warn: jest.fn(), - error: jest.fn(), - info: jest.fn(), - })), - format: { combine: jest.fn(), colorize: jest.fn(), simple: jest.fn() }, - transports: { Console: jest.fn() }, -})); +jest.mock('winston', () => { + const format = jest.fn(() => jest.fn(() => ({}))); + Object.assign(format, { + colorize: jest.fn(() => ({})), + combine: jest.fn(() => ({})), + errors: jest.fn(() => ({})), + json: jest.fn(() => ({})), + printf: jest.fn(() => ({})), + simple: jest.fn(() => ({})), + splat: jest.fn(() => ({})), + timestamp: jest.fn(() => ({})), + }); + + return { + addColors: jest.fn(), + createLogger: jest.fn(() => ({ + debug: jest.fn(), + warn: jest.fn(), + error: jest.fn(), + info: jest.fn(), + })), + format, + transports: { + Console: jest.fn(), + DailyRotateFile: jest.fn(), + }, + }; +}); // Mock env utilities so header resolution doesn't fail jest.mock('~/utils/env', () => ({ @@ -60,6 +80,7 @@ async function callAndCapture( summarizationConfig?: SummarizationConfig; initialSummary?: { text: string; tokenCount: number }; appConfig?: AppConfig; + messages?: BaseMessage[]; } = {}, ) { const agents = opts.agents ?? [makeAgent()]; @@ -71,6 +92,7 @@ async function callAndCapture( summarizationConfig: opts.summarizationConfig, initialSummary: opts.initialSummary, appConfig: opts.appConfig, + messages: opts.messages, streaming: true, streamUsage: true, }); @@ -940,6 +962,58 @@ describe('subagentConfigs', () => { expect(childInputs.initialSummary).toBeUndefined(); expect(childInputs.discoveredTools).toBeUndefined(); }); + + it('keeps discovered root tools from leaking into the same object used as a subagent', async () => { + const deferredTool = { + name: 'deferred_tool', + description: 'Deferred tool', + parameters: {}, + defer_loading: true, + allowed_callers: ['direct'], + }; + const child = makeAgent({ + id: 'agent_child', + name: 'Child', + hasDeferredTools: true, + toolRegistry: new Map([['deferred_tool', deferredTool]]), + toolDefinitions: [deferredTool], + }); + const parent = makeAgent({ + id: 'agent_parent', + subagents: { enabled: true, allowSelf: false, agent_ids: ['agent_child'] }, + subagentAgentConfigs: [child], + }); + + const agents = await callAndCapture({ + agents: [parent, child], + messages: [ + new ToolMessage({ + name: 'tool_search', + tool_call_id: 'call_1', + content: JSON.stringify({ tools: [{ name: 'deferred_tool' }] }), + }), + ], + }); + + const rootChild = agents.find((agent) => agent.agentId === 'agent_child'); + expect((rootChild?.toolDefinitions as Array>)[0].defer_loading).toBe( + false, + ); + + const rootParent = agents.find((agent) => agent.agentId === 'agent_parent'); + const childConfig = (rootParent?.subagentConfigs as Array>)[0]; + const childInputs = childConfig.agentInputs as { + discoveredTools?: unknown; + toolDefinitions?: Array>; + }; + expect(childInputs.discoveredTools).toBeUndefined(); + expect(childInputs.toolDefinitions?.[0].defer_loading).toBe(true); + expect(deferredTool.defer_loading).toBe(true); + expect( + (child.toolRegistry as Map>).get('deferred_tool') + ?.defer_loading, + ).toBe(true); + }); }); // --------------------------------------------------------------------------- diff --git a/packages/api/src/agents/run.ts b/packages/api/src/agents/run.ts index d082120d39..a6548b33eb 100644 --- a/packages/api/src/agents/run.ts +++ b/packages/api/src/agents/run.ts @@ -203,6 +203,26 @@ export function overrideDeferLoadingForDiscoveredTools( return overrideCount; } +function cloneToolRegistry(toolRegistry?: LCToolRegistry): LCToolRegistry | undefined { + if (!toolRegistry) { + return undefined; + } + + const cloned = new Map(); + for (const [name, tool] of toolRegistry.entries()) { + cloned.set(name, { ...tool }); + } + return cloned; +} + +function cloneToolDefinitions(toolDefinitions: LCTool[] | undefined): LCTool[] { + if (!toolDefinitions) { + return []; + } + + return toolDefinitions.map((def) => ({ ...def })); +} + const customProviders = new Set([ Providers.XAI, Providers.DEEPSEEK, @@ -630,16 +650,10 @@ async function buildSubagentConfigs( } /** * `buildAgentInput` applies parent-run context (initialSummary + - * discoveredTools) to the returned AgentInputs *and* to the - * passed-in agent's `toolRegistry` / `toolDefinitions` — flipping - * `defer_loading: true → false` on tools the parent had previously - * searched for, and injecting those tools' definitions into the - * child's `toolDefinitions`. Clearing fields on the returned - * object post-hoc would leave those side-effects in place, leaking - * the parent's tool-search state into an "isolated" subagent and - * inflating the child's prompt/token budget. The `isSubagent` flag - * skips both the field stamping and the registry mutation at the - * source so children truly start fresh. + * discoveredTools) to returned top-level AgentInputs. The `isSubagent` + * flag skips that field stamping so child agents start from an isolated + * context even when the same initialized agent object also appears in the + * outer top-level run list. */ const childInputs = await toInput(child, { isSubagent: true }); /** @@ -811,6 +825,24 @@ export async function createRun({ llmConfig.usage = true; } + /** + * Clone mutable tool metadata before any deferred-tool override. The same + * initialized agent object can be used both as a top-level root and as a + * subagent child, and root inputs are built in parallel for latency. If the + * root path mutates `agent.toolRegistry` directly, a sibling parent building + * this object as a subagent can observe the flipped `defer_loading` flags + * and lose isolation. Cloning only when a mutation/isolation boundary exists + * keeps the common no-discovery path allocation-light. + */ + const shouldCloneToolState = + agent.toolRegistry != null && (isSubagent || discoveredTools.size > 0); + const toolRegistry = shouldCloneToolState + ? cloneToolRegistry(agent.toolRegistry) + : agent.toolRegistry; + let toolDefinitions = shouldCloneToolState + ? cloneToolDefinitions(agent.toolDefinitions) + : (agent.toolDefinitions ?? []); + /** * Override defer_loading for tools that were discovered in previous * turns. This prevents the LLM from having to re-discover tools via @@ -820,15 +852,15 @@ export async function createRun({ * Skipped for subagent children (`isSubagent`) — they run in an * isolated context by contract, so inheriting the parent's * tool-search state leaks unrelated history and pre-loads tools the - * child shouldn't care about. Mutations on `agent.toolRegistry` - * and additions to `toolDefinitions` both happen here, so the flag - * has to gate the whole block (clearing fields post-return can't - * undo registry writes). + * child shouldn't care about. */ - let toolDefinitions = agent.toolDefinitions ?? []; - let toolRegistry = agent.toolRegistry; - if (!isSubagent && discoveredTools.size > 0 && agent.toolRegistry) { - overrideDeferLoadingForDiscoveredTools(agent.toolRegistry, discoveredTools); + if (!isSubagent && discoveredTools.size > 0 && toolRegistry) { + overrideDeferLoadingForDiscoveredTools(toolRegistry, discoveredTools); + toolDefinitions = toolDefinitions.map((def) => + discoveredTools.has(def.name) && def.defer_loading === true + ? { ...def, defer_loading: false } + : def, + ); /** Add discovered tools' definitions so the LLM can see their schemas */ const existingToolNames = new Set(toolDefinitions.map((d) => d.name)); @@ -836,30 +868,11 @@ export async function createRun({ if (existingToolNames.has(toolName)) { continue; } - const toolDef = agent.toolRegistry.get(toolName); + const toolDef = toolRegistry.get(toolName); if (toolDef) { toolDefinitions = [...toolDefinitions, toolDef]; } } - } else if (isSubagent && agent.toolRegistry) { - /** - * Subagent children: hand the child a deep-enough clone of the - * registry so later parent-graph builds (e.g. when the same - * agent also appears as a handoff target in the outer loop) - * can't mutate `defer_loading` on tool definitions the child - * already holds a reference to. Clone the `Map` *and* each - * `LCTool` — `overrideDeferLoadingForDiscoveredTools` writes - * through to the tool object itself, so a shallow Map copy - * alone wouldn't isolate the flag. - */ - toolRegistry = new Map(); - for (const [name, tool] of agent.toolRegistry.entries()) { - toolRegistry.set(name, { ...tool }); - } - /** Child's own `toolDefinitions` list gets the same shallow- - * copied view so any later parent mutation of shared definitions - * is contained to the parent-graph path. */ - toolDefinitions = toolDefinitions.map((def) => ({ ...def })); } const effectiveMaxContextTokens = computeEffectiveMaxContextTokens(