mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-05-13 07:46:47 +00:00
fix: isolate deferred tool metadata during parallel builds
This commit is contained in:
parent
a2105df07e
commit
52be3062eb
2 changed files with 135 additions and 48 deletions
|
|
@ -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<Record<string, unknown>>)[0].defer_loading).toBe(
|
||||
false,
|
||||
);
|
||||
|
||||
const rootParent = agents.find((agent) => agent.agentId === 'agent_parent');
|
||||
const childConfig = (rootParent?.subagentConfigs as Array<Record<string, unknown>>)[0];
|
||||
const childInputs = childConfig.agentInputs as {
|
||||
discoveredTools?: unknown;
|
||||
toolDefinitions?: Array<Record<string, unknown>>;
|
||||
};
|
||||
expect(childInputs.discoveredTools).toBeUndefined();
|
||||
expect(childInputs.toolDefinitions?.[0].defer_loading).toBe(true);
|
||||
expect(deferredTool.defer_loading).toBe(true);
|
||||
expect(
|
||||
(child.toolRegistry as Map<string, Record<string, unknown>>).get('deferred_tool')
|
||||
?.defer_loading,
|
||||
).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
|
|
|
|||
|
|
@ -203,6 +203,26 @@ export function overrideDeferLoadingForDiscoveredTools(
|
|||
return overrideCount;
|
||||
}
|
||||
|
||||
function cloneToolRegistry(toolRegistry?: LCToolRegistry): LCToolRegistry | undefined {
|
||||
if (!toolRegistry) {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
const cloned = new Map<string, LCTool>();
|
||||
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(
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue