mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-10-21 17:44:11 +00:00
📜 fix: Scope Read File Prompt For Code Agents (#13040)
* fix: Scope read_file prompt for code agents * fix: Align code read_file prompt behavior
This commit is contained in:
parent
c67e2b54dc
commit
80ce956c94
5 changed files with 225 additions and 29 deletions
|
|
@ -5,6 +5,25 @@
|
|||
*/
|
||||
jest.mock('@librechat/agents', () => ({
|
||||
...jest.requireActual('@librechat/agents'),
|
||||
ReadFileToolDefinition: {
|
||||
name: 'read_file',
|
||||
description: 'read skill files using {skillName}/{filePath} and SKILL.md',
|
||||
parameters: {
|
||||
type: 'object',
|
||||
properties: {
|
||||
file_path: {
|
||||
type: 'string',
|
||||
description: 'For skill files: "{skillName}/{path}".',
|
||||
},
|
||||
},
|
||||
},
|
||||
responseFormat: 'content',
|
||||
},
|
||||
BashExecutionToolDefinition: {
|
||||
name: 'bash_tool',
|
||||
description: 'bash',
|
||||
schema: { type: 'object', properties: {} },
|
||||
},
|
||||
buildBashExecutionToolDescription: ({
|
||||
enableToolOutputReferences,
|
||||
}: {
|
||||
|
|
@ -1064,6 +1083,52 @@ describe('initializeAgent — execute_code capability expansion', () => {
|
|||
path — the string stays in `agent.tools` as the capability trigger
|
||||
but never appears in the tool definitions the LLM sees. */
|
||||
expect(names).not.toContain('execute_code');
|
||||
const readFile = result.toolDefinitions?.find((d) => d.name === 'read_file');
|
||||
expect(readFile?.description).toContain('code-execution sandbox');
|
||||
expect(readFile?.description).not.toContain('{skillName}');
|
||||
expect(readFile?.description).not.toContain('SKILL.md');
|
||||
});
|
||||
|
||||
it('upgrades read_file to the skill-aware description when active skills are in scope', async () => {
|
||||
const { agent, req, res, loadTools, db } = createMocks();
|
||||
agent.tools = ['execute_code'];
|
||||
const { Types } = await import('mongoose');
|
||||
const skillId = new Types.ObjectId();
|
||||
const author = { toString: () => req.user?.id } as unknown as import('mongoose').Types.ObjectId;
|
||||
|
||||
const result = await initializeAgent(
|
||||
{
|
||||
req,
|
||||
res,
|
||||
agent,
|
||||
loadTools,
|
||||
endpointOption: { endpoint: EModelEndpoint.agents },
|
||||
allowedProviders: new Set([Providers.OPENAI]),
|
||||
isInitialAgent: true,
|
||||
codeEnvAvailable: true,
|
||||
accessibleSkillIds: [skillId],
|
||||
},
|
||||
{
|
||||
...db,
|
||||
listSkillsByAccess: jest.fn().mockResolvedValue({
|
||||
skills: [
|
||||
{
|
||||
_id: skillId,
|
||||
name: 'data-cleaner',
|
||||
description: 'Clean tabular data.',
|
||||
author,
|
||||
},
|
||||
],
|
||||
has_more: false,
|
||||
after: null,
|
||||
}),
|
||||
},
|
||||
);
|
||||
|
||||
const readFile = result.toolDefinitions?.find((d) => d.name === 'read_file');
|
||||
expect(readFile?.description).toContain('{skillName}/{filePath}');
|
||||
expect(readFile?.description).toContain('SKILL.md');
|
||||
expect(result.toolDefinitions?.map((d) => d.name)).toContain('skill');
|
||||
});
|
||||
|
||||
it('does not register bash_tool + read_file when codeEnvAvailable=false', async () => {
|
||||
|
|
|
|||
|
|
@ -777,7 +777,9 @@ export async function initializeAgent(
|
|||
* (backed by `CodeExecutionToolDefinition` + `primeCodeFiles`) is no
|
||||
* longer registered; the string `execute_code` on the agent document
|
||||
* stays as the capability-trigger marker but expands into the
|
||||
* skill-flavored tool pair here.
|
||||
* `bash_tool` + `read_file` pair here. The initial `read_file`
|
||||
* definition is code-only; `injectSkillCatalog` upgrades it later in
|
||||
* this initializer when skill files are actually available.
|
||||
*
|
||||
* `effectiveCodeEnvAvailable` is the per-agent truth: the admin-level
|
||||
* `params.codeEnvAvailable` AND the agent actually asking for code
|
||||
|
|
@ -792,8 +794,9 @@ export async function initializeAgent(
|
|||
* execute-code-only agents on Google/Vertex still trip the conflict
|
||||
* guard when provider-specific tools are also configured. Also before
|
||||
* `injectSkillCatalog` so the skill path's own
|
||||
* `registerCodeExecutionTools` call becomes a no-op via the registry
|
||||
* `.has()` dedupe — exactly one copy of each tool reaches the LLM.
|
||||
* `registerCodeExecutionTools` call upgrades `read_file` from the
|
||||
* code-only description to the skill-aware description without adding a
|
||||
* duplicate — exactly one copy of each tool reaches the LLM.
|
||||
*/
|
||||
const agentRequestsCodeExec = (agent.tools ?? []).includes(Tools.execute_code);
|
||||
const effectiveCodeEnvAvailable = params.codeEnvAvailable === true && agentRequestsCodeExec;
|
||||
|
|
@ -802,6 +805,7 @@ export async function initializeAgent(
|
|||
toolRegistry,
|
||||
toolDefinitions,
|
||||
includeBash: true,
|
||||
includeSkillFileInstructions: false,
|
||||
enableToolOutputReferences: effectiveCodeEnvAvailable,
|
||||
});
|
||||
toolDefinitions = codeExecResult.toolDefinitions;
|
||||
|
|
|
|||
|
|
@ -426,10 +426,11 @@ export async function injectSkillCatalog(
|
|||
* `skill` tool is conditional on having anything for the model to invoke.
|
||||
* `read_file` + `bash_tool` go through `registerCodeExecutionTools` so
|
||||
* a prior registration from `initializeAgent` (for the `execute_code`
|
||||
* capability) doesn't produce a duplicate copy. `read_file` is always
|
||||
* included — manually-primed `disable-model-invocation: true` skills
|
||||
* still need it to load their `references/*` from storage. `bash_tool`
|
||||
* follows `codeEnvAvailable` as before.
|
||||
* capability) upgrades to the skill-aware `read_file` definition without
|
||||
* producing a duplicate copy. `read_file` is always included —
|
||||
* manually-primed `disable-model-invocation: true` skills still need it
|
||||
* to load their `references/*` from storage. `bash_tool` follows
|
||||
* `codeEnvAvailable` as before.
|
||||
*/
|
||||
let workingDefs: LCTool[] = [...(inputDefs ?? [])];
|
||||
if (catalogVisibleSkills.length > 0) {
|
||||
|
|
@ -440,9 +441,9 @@ export async function injectSkillCatalog(
|
|||
/**
|
||||
* Forward `enableToolOutputReferences` to keep the skills caller
|
||||
* symmetric with `initializeAgent`'s call. Today `initializeAgent`
|
||||
* registers `bash_tool` first and the registry `.has()` check makes
|
||||
* this call a no-op — but if call order ever flips (skills-first),
|
||||
* a missing flag here would silently produce a `bash_tool`
|
||||
* registers `bash_tool` first and this call only keeps the existing
|
||||
* bash definition — but if call order ever flips (skills-first), a
|
||||
* missing flag here would silently produce a `bash_tool`
|
||||
* description without the `{{tool<idx>turn<turn>}}` guide, and the
|
||||
* `initializeAgent` pass would become the no-op. Mirror the gate
|
||||
* `initializeAgent` uses (`effectiveCodeEnvAvailable`, which here
|
||||
|
|
|
|||
|
|
@ -8,8 +8,16 @@ jest.mock('@librechat/agents', () => ({
|
|||
...jest.requireActual('@librechat/agents'),
|
||||
ReadFileToolDefinition: {
|
||||
name: 'read_file',
|
||||
description: 'read file',
|
||||
parameters: { type: 'object', properties: {} },
|
||||
description: 'read skill files using {skillName}/{filePath} and SKILL.md',
|
||||
parameters: {
|
||||
type: 'object',
|
||||
properties: {
|
||||
file_path: {
|
||||
type: 'string',
|
||||
description: 'For skill files: "{skillName}/{path}".',
|
||||
},
|
||||
},
|
||||
},
|
||||
responseFormat: 'content',
|
||||
},
|
||||
BashExecutionToolDefinition: {
|
||||
|
|
@ -191,6 +199,47 @@ describe('registerCodeExecutionTools', () => {
|
|||
expect(toolRegistry.has('bash_tool')).toBe(false);
|
||||
});
|
||||
|
||||
it('uses a code-only read_file description when skill instructions are disabled', () => {
|
||||
const toolRegistry = makeRegistry();
|
||||
const result = registerCodeExecutionTools({
|
||||
toolRegistry,
|
||||
toolDefinitions: [],
|
||||
includeBash: true,
|
||||
includeSkillFileInstructions: false,
|
||||
});
|
||||
|
||||
const readFile = result.toolDefinitions.find((d) => d.name === 'read_file');
|
||||
expect(readFile?.description).toContain('code-execution sandbox');
|
||||
expect(readFile?.description).toContain('/mnt/data/');
|
||||
expect(readFile?.description).toContain('truncated around 256KB');
|
||||
expect(readFile?.description).toContain('may return an error');
|
||||
expect(readFile?.description).not.toContain('{skillName}');
|
||||
expect(readFile?.description).not.toContain('SKILL.md');
|
||||
expect(JSON.stringify(readFile?.parameters)).not.toContain('{skillName}');
|
||||
});
|
||||
|
||||
it('upgrades a code-only read_file definition when skills are enabled later in the run', () => {
|
||||
const toolRegistry = makeRegistry();
|
||||
const codeOnly = registerCodeExecutionTools({
|
||||
toolRegistry,
|
||||
toolDefinitions: [],
|
||||
includeBash: true,
|
||||
includeSkillFileInstructions: false,
|
||||
});
|
||||
const upgraded = registerCodeExecutionTools({
|
||||
toolRegistry,
|
||||
toolDefinitions: codeOnly.toolDefinitions,
|
||||
includeBash: false,
|
||||
includeSkillFileInstructions: true,
|
||||
});
|
||||
|
||||
const readFile = upgraded.toolDefinitions.find((d) => d.name === 'read_file');
|
||||
expect(upgraded.registered).toEqual([]);
|
||||
expect(readFile?.description).toContain('{skillName}/{filePath}');
|
||||
expect(readFile?.description).toContain('SKILL.md');
|
||||
expect(toolRegistry.get('read_file')?.description).toBe(readFile?.description);
|
||||
});
|
||||
|
||||
it('preserves pre-existing unrelated tool definitions', () => {
|
||||
const toolRegistry = makeRegistry();
|
||||
const existing: LCTool[] = [
|
||||
|
|
|
|||
|
|
@ -64,6 +64,13 @@ export interface RegisterCodeExecutionToolsParams {
|
|||
* no-op so there is exactly one copy of each tool in `toolDefinitions`.
|
||||
*/
|
||||
includeBash: boolean;
|
||||
/**
|
||||
* When `true`, `read_file` tells the model about skill-file paths
|
||||
* (`{skillName}/{path}`). When `false`, it is described only as a
|
||||
* code-execution sandbox reader so execute-code-only agents don't get
|
||||
* prompted to discover skills that are not enabled for the run.
|
||||
*/
|
||||
includeSkillFileInstructions?: boolean;
|
||||
/**
|
||||
* When `true`, the registered `bash_tool` description includes the
|
||||
* LLM-facing `{{tool<idx>turn<turn>}}` reference syntax guide so the
|
||||
|
|
@ -80,11 +87,11 @@ export interface RegisterCodeExecutionToolsResult {
|
|||
}
|
||||
|
||||
/**
|
||||
* Hoisted module-level definition for `read_file` so
|
||||
* Hoisted module-level definition for skill-aware `read_file` so
|
||||
* `registerCodeExecutionTools` doesn't re-allocate on every call. The
|
||||
* shape is derived entirely from a static `@librechat/agents` export —
|
||||
* no per-request state — so a single frozen object is safe to share
|
||||
* across every agent init.
|
||||
* shape is derived from a static `@librechat/agents` export — no
|
||||
* per-request state — so a single frozen object is safe to share across
|
||||
* every agent init.
|
||||
*/
|
||||
const READ_FILE_DEF: LCTool = Object.freeze({
|
||||
name: ReadFileToolDefinition.name,
|
||||
|
|
@ -93,6 +100,47 @@ const READ_FILE_DEF: LCTool = Object.freeze({
|
|||
responseFormat: ReadFileToolDefinition.responseFormat,
|
||||
}) as LCTool;
|
||||
|
||||
const CODE_READ_FILE_DESCRIPTION = `Read the contents of a file from the code-execution sandbox or from prior code-execution output. Returns text content with line numbers for easy reference.
|
||||
|
||||
BEHAVIOR:
|
||||
- Text files: returned with numbered lines.
|
||||
- Large text files are truncated around 256KB with a note to use bash_tool for the full content.
|
||||
- Binary files and formats that are not safe to serialize as text may return an error. Use bash_tool to inspect or process them.
|
||||
|
||||
CONSTRAINTS:
|
||||
- Only files produced by code execution or attached to the code-execution sandbox are accessible.
|
||||
- Use paths returned by tool output or paths under /mnt/data/.
|
||||
- Do not guess file paths. Use bash_tool to inspect available sandbox files when needed.`;
|
||||
|
||||
const CODE_READ_FILE_PARAMETERS: LCTool['parameters'] = Object.freeze({
|
||||
type: 'object',
|
||||
properties: {
|
||||
file_path: {
|
||||
type: 'string',
|
||||
description:
|
||||
'Path to a file from code execution output, such as "/mnt/data/result.csv" or another path returned by the execution tool.',
|
||||
},
|
||||
},
|
||||
required: ['file_path'],
|
||||
}) as LCTool['parameters'];
|
||||
|
||||
const CODE_READ_FILE_DEF: LCTool = Object.freeze({
|
||||
name: ReadFileToolDefinition.name,
|
||||
description: CODE_READ_FILE_DESCRIPTION,
|
||||
parameters: CODE_READ_FILE_PARAMETERS,
|
||||
responseFormat: ReadFileToolDefinition.responseFormat,
|
||||
}) as LCTool;
|
||||
|
||||
function buildReadFileDef(includeSkillFileInstructions: boolean): LCTool {
|
||||
return includeSkillFileInstructions ? READ_FILE_DEF : CODE_READ_FILE_DEF;
|
||||
}
|
||||
|
||||
function isCodeOnlyReadFileDef(def: LCTool | undefined): boolean {
|
||||
return (
|
||||
def?.name === ReadFileToolDefinition.name && def?.description === CODE_READ_FILE_DESCRIPTION
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* The `bash_tool` description varies along exactly one axis — whether
|
||||
* the LLM-facing `{{tool<idx>turn<turn>}}` reference syntax guide is
|
||||
|
|
@ -122,9 +170,11 @@ function buildBashToolDef(opts: { enableToolOutputReferences: boolean }): LCTool
|
|||
}
|
||||
|
||||
/**
|
||||
* Idempotently registers the skill-flavored code-execution tool pair
|
||||
* (`bash_tool` + `read_file`) into the run's tool registry and
|
||||
* tool-definition list.
|
||||
* Idempotently registers the code-execution tool pair (`bash_tool` +
|
||||
* `read_file`) into the run's tool registry and tool-definition list.
|
||||
* `read_file` can be registered with a code-only description first and
|
||||
* upgraded to the skill-aware description later in the same run if
|
||||
* skills are actually injected.
|
||||
*
|
||||
* Replaces the legacy `CodeExecutionToolDefinition` / `execute_code`
|
||||
* registration. `execute_code` as a capability name and as an
|
||||
|
|
@ -136,19 +186,46 @@ function buildBashToolDef(opts: { enableToolOutputReferences: boolean }): LCTool
|
|||
export function registerCodeExecutionTools(
|
||||
params: RegisterCodeExecutionToolsParams,
|
||||
): RegisterCodeExecutionToolsResult {
|
||||
const { toolRegistry, toolDefinitions, includeBash, enableToolOutputReferences = false } = params;
|
||||
const {
|
||||
toolRegistry,
|
||||
toolDefinitions,
|
||||
includeBash,
|
||||
includeSkillFileInstructions = true,
|
||||
enableToolOutputReferences = false,
|
||||
} = params;
|
||||
|
||||
const readFileDef = buildReadFileDef(includeSkillFileInstructions);
|
||||
const candidates: LCTool[] = includeBash
|
||||
? [READ_FILE_DEF, buildBashToolDef({ enableToolOutputReferences })]
|
||||
: [READ_FILE_DEF];
|
||||
? [readFileDef, buildBashToolDef({ enableToolOutputReferences })]
|
||||
: [readFileDef];
|
||||
|
||||
const existingNames = new Set((toolDefinitions ?? []).map((d) => d.name));
|
||||
const inputDefinitions = toolDefinitions ?? [];
|
||||
let workingDefinitions = inputDefinitions;
|
||||
|
||||
const registered: string[] = [];
|
||||
const newDefs: LCTool[] = [];
|
||||
for (const def of candidates) {
|
||||
const existingIndex = workingDefinitions.findIndex((d) => d.name === def.name);
|
||||
const existingDef = existingIndex >= 0 ? workingDefinitions[existingIndex] : undefined;
|
||||
const registryDef = toolRegistry?.get(def.name);
|
||||
if (
|
||||
def.name === ReadFileToolDefinition.name &&
|
||||
includeSkillFileInstructions &&
|
||||
(isCodeOnlyReadFileDef(existingDef) || isCodeOnlyReadFileDef(registryDef))
|
||||
) {
|
||||
if (isCodeOnlyReadFileDef(existingDef)) {
|
||||
workingDefinitions =
|
||||
workingDefinitions === inputDefinitions ? [...inputDefinitions] : workingDefinitions;
|
||||
workingDefinitions[existingIndex] = def;
|
||||
}
|
||||
if (isCodeOnlyReadFileDef(registryDef)) {
|
||||
toolRegistry?.set(def.name, def);
|
||||
}
|
||||
continue;
|
||||
}
|
||||
|
||||
const inRegistry = toolRegistry?.has(def.name) === true;
|
||||
const inDefs = existingNames.has(def.name);
|
||||
const inDefs = existingIndex >= 0;
|
||||
if (inRegistry || inDefs) {
|
||||
continue;
|
||||
}
|
||||
|
|
@ -158,15 +235,15 @@ export function registerCodeExecutionTools(
|
|||
}
|
||||
|
||||
/**
|
||||
* Skip the array spread on the common second-call no-op path (both tools
|
||||
* already registered by the first caller in the same run). Returns the
|
||||
* input array by reference; callers treat the return value as immutable.
|
||||
* Skip the array spread on the common second-call path when no tools were
|
||||
* newly registered. Returns the input array by reference unless an existing
|
||||
* code-only `read_file` definition was upgraded above.
|
||||
*/
|
||||
if (newDefs.length === 0) {
|
||||
return { toolDefinitions: toolDefinitions ?? [], registered };
|
||||
return { toolDefinitions: workingDefinitions, registered };
|
||||
}
|
||||
return {
|
||||
toolDefinitions: [...(toolDefinitions ?? []), ...newDefs],
|
||||
toolDefinitions: [...workingDefinitions, ...newDefs],
|
||||
registered,
|
||||
};
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue