mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-07-01 11:53:55 +00:00
Skill files were primed into the sandbox at `/mnt/data/{skillName}/...`,
but the read_file/create_file/edit_file tool descriptions and the
read_file bash-fallback hints all assume the `skills/{skillName}/...`
namespace (sandbox cwd is `/mnt/data`). Agents therefore reached for
`./skills/my-skill/...` in bash and missed ~100% of the time.
- Add shared `SKILL_FILE_PREFIX` to agents/skills.ts (moved out of
handlers.ts; single source of truth across the three layers).
- Prefix the prime upload filenames and session names with `skills/` in
skillFiles.ts so the physical mount matches the model-facing namespace;
recover the bare relativePath by stripping `skills/{name}/`.
- Canonicalize the read_file bash-fallback hints to
`/mnt/data/skills/{skillName}/{relativePath}` so the implicit
`{name}/...` addressing form is corrected too.
Closes #13961
This commit is contained in:
parent
12fea693bb
commit
4fa86be424
5 changed files with 159 additions and 23 deletions
|
|
@ -2455,6 +2455,106 @@ describe('createToolExecuteHandler', () => {
|
|||
expect(result.content).toContain('Recovered Body');
|
||||
});
|
||||
|
||||
it('points the bash fallback at the skills/ mount for a binary skill file (#13961)', async () => {
|
||||
/**
|
||||
* Bundled skill files are primed into the sandbox under the
|
||||
* `skills/{skillName}/...` namespace (see `primeSkillFiles`), so the
|
||||
* binary/large bash hint must reference `/mnt/data/skills/...` — the
|
||||
* real on-disk path — not a prefix-less `/mnt/data/{skillName}/...`
|
||||
* that points nowhere.
|
||||
*/
|
||||
const getSkillByName = jest.fn(async () => ({
|
||||
_id: '507f1f77bcf86cd799439099' as unknown as never,
|
||||
name: 'brand-skill',
|
||||
body: '# Brand skill',
|
||||
fileCount: 1,
|
||||
version: 1,
|
||||
}));
|
||||
const getSkillFileByPath = jest.fn(async () => ({
|
||||
content: '',
|
||||
isBinary: true,
|
||||
mimeType: 'application/vnd.openxmlformats-officedocument.wordprocessingml.document',
|
||||
bytes: 4096,
|
||||
filepath: '/storage/brand-skill/references/guide.docx',
|
||||
source: 'local',
|
||||
relativePath: 'references/guide.docx',
|
||||
}));
|
||||
const handler = createToolExecuteHandler({
|
||||
loadTools: jest.fn(async () => ({
|
||||
loadedTools: [],
|
||||
configurable: {
|
||||
codeEnvAvailable: true,
|
||||
accessibleSkillIds: skillsInScope(),
|
||||
activeSkillNames: new Set(['brand-skill']),
|
||||
},
|
||||
})),
|
||||
getSkillByName,
|
||||
getSkillFileByPath,
|
||||
});
|
||||
|
||||
const [result] = await invokeHandler(handler, [
|
||||
{
|
||||
id: 'call_binary_skill_read',
|
||||
name: Constants.READ_FILE,
|
||||
args: { path: 'skills/brand-skill/references/guide.docx' },
|
||||
},
|
||||
]);
|
||||
|
||||
expect(result.status).toBe('success');
|
||||
expect(result.content).toContain(
|
||||
'Use bash to process: /mnt/data/skills/brand-skill/references/guide.docx',
|
||||
);
|
||||
});
|
||||
|
||||
it('canonicalizes the bash hint to skills/ even when addressed without the prefix (#13961)', async () => {
|
||||
/**
|
||||
* The implicit `{skillName}/...` addressing form resolves the same
|
||||
* skill file, so its bash hint must also point at the canonical
|
||||
* `/mnt/data/skills/...` mount rather than echoing the prefix-less
|
||||
* `args.path`.
|
||||
*/
|
||||
const getSkillByName = jest.fn(async () => ({
|
||||
_id: '507f1f77bcf86cd799439099' as unknown as never,
|
||||
name: 'brand-skill',
|
||||
body: '# Brand skill',
|
||||
fileCount: 1,
|
||||
version: 1,
|
||||
}));
|
||||
const getSkillFileByPath = jest.fn(async () => ({
|
||||
content: '',
|
||||
isBinary: true,
|
||||
mimeType: 'application/vnd.openxmlformats-officedocument.wordprocessingml.document',
|
||||
bytes: 4096,
|
||||
filepath: '/storage/brand-skill/references/guide.docx',
|
||||
source: 'local',
|
||||
relativePath: 'references/guide.docx',
|
||||
}));
|
||||
const handler = createToolExecuteHandler({
|
||||
loadTools: jest.fn(async () => ({
|
||||
loadedTools: [],
|
||||
configurable: {
|
||||
codeEnvAvailable: true,
|
||||
accessibleSkillIds: skillsInScope(),
|
||||
activeSkillNames: new Set(['brand-skill']),
|
||||
},
|
||||
})),
|
||||
getSkillByName,
|
||||
getSkillFileByPath,
|
||||
});
|
||||
|
||||
const [result] = await invokeHandler(handler, [
|
||||
{
|
||||
id: 'call_binary_skill_read_implicit',
|
||||
name: Constants.READ_FILE,
|
||||
args: { path: 'brand-skill/references/guide.docx' },
|
||||
},
|
||||
]);
|
||||
|
||||
expect(result.status).toBe('success');
|
||||
expect(result.content).toContain('/mnt/data/skills/brand-skill/references/guide.docx');
|
||||
expect(result.content).not.toContain('/mnt/data/brand-skill/references/guide.docx');
|
||||
});
|
||||
|
||||
it('routes through sandbox when skills are not effectively enabled (empty accessibleSkillIds)', async () => {
|
||||
/**
|
||||
* `accessibleSkillIds: []` is what `resolveAgentScopedSkillIds`
|
||||
|
|
|
|||
|
|
@ -21,9 +21,9 @@ import {
|
|||
HOST_FILE_AUTHORING_ARTIFACT_KEY,
|
||||
isCodeSessionToolName,
|
||||
} from './tools';
|
||||
import { buildSkillPrimeMessage, SKILL_FILE_PREFIX } from './skills';
|
||||
import { logAxiosError, runOutsideTracing } from '~/utils';
|
||||
import { parseFrontmatter } from '../skills/import';
|
||||
import { buildSkillPrimeMessage } from './skills';
|
||||
import { cleanCodeToolOutput } from './cleanup';
|
||||
import { primeSkillFiles } from './skillFiles';
|
||||
|
||||
|
|
@ -260,7 +260,6 @@ const MAX_CACHE_BYTES = 512 * 1024;
|
|||
const MAX_AUTHORING_BYTES = 10 * 1024 * 1024;
|
||||
const MAX_TOOL_ERROR_MESSAGE_CHARS = 12_000;
|
||||
const MAX_TOOL_ERROR_STACK_CHARS = 4_000;
|
||||
const SKILL_FILE_PREFIX = 'skills/';
|
||||
const SKILL_MD = 'SKILL.md';
|
||||
|
||||
const IMAGE_MIMES = new Set(['image/png', 'image/jpeg', 'image/gif', 'image/webp']);
|
||||
|
|
@ -2767,6 +2766,14 @@ async function handleReadFileCall(
|
|||
};
|
||||
}
|
||||
|
||||
/* Bundled skill files are primed into the sandbox under the `skills/`
|
||||
* namespace (see `primeSkillFiles`), so the on-disk path is always
|
||||
* `/mnt/data/skills/{skillName}/{relativePath}` regardless of whether the
|
||||
* model addressed the file with or without the explicit prefix. Use this
|
||||
* canonical path in the bash-fallback hints below so they never echo a
|
||||
* prefix-less `args.path` that points nowhere on disk. */
|
||||
const sandboxFilePath = `/mnt/data/${SKILL_FILE_PREFIX}${skillName}/${relativePath}`;
|
||||
|
||||
if (!getSkillFileByPath) {
|
||||
return {
|
||||
toolCallId: tc.id,
|
||||
|
|
@ -2794,7 +2801,7 @@ async function handleReadFileCall(
|
|||
return {
|
||||
toolCallId: tc.id,
|
||||
status: 'success',
|
||||
content: `Binary file (${file.mimeType}, ${file.bytes} bytes). Use bash to process: /mnt/data/${args.path}`,
|
||||
content: `Binary file (${file.mimeType}, ${file.bytes} bytes). Use bash to process: ${sandboxFilePath}`,
|
||||
};
|
||||
}
|
||||
}
|
||||
|
|
@ -2814,14 +2821,14 @@ async function handleReadFileCall(
|
|||
return {
|
||||
toolCallId: tc.id,
|
||||
status: 'success',
|
||||
content: `File "${args.path}" is too large to read directly (${file.bytes} bytes, limit: ${MAX_READABLE_BYTES}). Invoke the skill first, then use bash to read it at /mnt/data/${args.path}.`,
|
||||
content: `File "${args.path}" is too large to read directly (${file.bytes} bytes, limit: ${MAX_READABLE_BYTES}). Invoke the skill first, then use bash to read it at ${sandboxFilePath}.`,
|
||||
};
|
||||
}
|
||||
if (isImage && file.bytes > MAX_BINARY_BYTES) {
|
||||
return {
|
||||
toolCallId: tc.id,
|
||||
status: 'success',
|
||||
content: `File too large (${file.bytes} bytes, limit: ${MAX_BINARY_BYTES}). Use bash to process: /mnt/data/${args.path}`,
|
||||
content: `File too large (${file.bytes} bytes, limit: ${MAX_BINARY_BYTES}). Use bash to process: ${sandboxFilePath}`,
|
||||
};
|
||||
}
|
||||
|
||||
|
|
@ -2865,7 +2872,7 @@ async function handleReadFileCall(
|
|||
return {
|
||||
toolCallId: tc.id,
|
||||
status: 'success',
|
||||
content: `File "${args.path}" exceeded streaming limit (${streamLimit} bytes). Invoke the skill first, then use bash to read it at /mnt/data/${args.path}.`,
|
||||
content: `File "${args.path}" exceeded streaming limit (${streamLimit} bytes). Invoke the skill first, then use bash to read it at ${sandboxFilePath}.`,
|
||||
};
|
||||
}
|
||||
chunks.push(chunk);
|
||||
|
|
@ -2919,7 +2926,7 @@ async function handleReadFileCall(
|
|||
return {
|
||||
toolCallId: tc.id,
|
||||
status: 'success',
|
||||
content: `Binary file (${file.mimeType}, ${buffer.length} bytes). Use bash to process: /mnt/data/${args.path}`,
|
||||
content: `Binary file (${file.mimeType}, ${buffer.length} bytes). Use bash to process: ${sandboxFilePath}`,
|
||||
};
|
||||
}
|
||||
|
||||
|
|
@ -2941,7 +2948,7 @@ async function handleReadFileCall(
|
|||
return {
|
||||
toolCallId: tc.id,
|
||||
status: 'success',
|
||||
content: `File too large (${buffer.length} bytes, limit: ${MAX_READABLE_BYTES}). Use bash: cat /mnt/data/${args.path}`,
|
||||
content: `File too large (${buffer.length} bytes, limit: ${MAX_READABLE_BYTES}). Use bash: cat ${sandboxFilePath}`,
|
||||
};
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -85,7 +85,7 @@ describe('primeInvokedSkills — execute_code capability gate', () => {
|
|||
});
|
||||
const batchUploadCodeEnvFiles = jest.fn().mockResolvedValue({
|
||||
storage_session_id: 'session-42',
|
||||
files: [{ fileId: 'file-1', filename: 'brand-guidelines/references/style.md' }],
|
||||
files: [{ fileId: 'file-1', filename: 'skills/brand-guidelines/references/style.md' }],
|
||||
});
|
||||
|
||||
const deps = makeDeps({
|
||||
|
|
@ -112,7 +112,10 @@ describe('primeInvokedSkills — execute_code capability gate', () => {
|
|||
expect(uploadArgs.version).toBe(SKILL_VERSION);
|
||||
expect(uploadArgs.files).toHaveLength(fileRecords.length + 1);
|
||||
expect(uploadArgs.files.map((f: { filename: string }) => f.filename)).toEqual(
|
||||
expect.arrayContaining(['brand-guidelines/SKILL.md', 'brand-guidelines/references/style.md']),
|
||||
expect.arrayContaining([
|
||||
'skills/brand-guidelines/SKILL.md',
|
||||
'skills/brand-guidelines/references/style.md',
|
||||
]),
|
||||
);
|
||||
});
|
||||
|
||||
|
|
@ -146,7 +149,7 @@ describe('primeInvokedSkills — execute_code capability gate', () => {
|
|||
});
|
||||
const batchUploadCodeEnvFiles = jest.fn().mockResolvedValue({
|
||||
storage_session_id: 'session-42',
|
||||
files: [{ fileId: 'file-1', filename: 'brand-guidelines/references/style.md' }],
|
||||
files: [{ fileId: 'file-1', filename: 'skills/brand-guidelines/references/style.md' }],
|
||||
});
|
||||
|
||||
const deps = makeDeps({
|
||||
|
|
@ -168,7 +171,7 @@ describe('primeInvokedSkills — execute_code capability gate', () => {
|
|||
* where codeapi computed sessionKey from the storage nanoid
|
||||
* instead of the skill _id. */
|
||||
resource_id: SKILL_ID.toString(),
|
||||
name: 'brand-guidelines/references/style.md',
|
||||
name: 'skills/brand-guidelines/references/style.md',
|
||||
storage_session_id: 'session-42',
|
||||
kind: 'skill',
|
||||
version: SKILL_VERSION,
|
||||
|
|
@ -199,7 +202,7 @@ describe('primeInvokedSkills — execute_code capability gate', () => {
|
|||
});
|
||||
const batchUploadCodeEnvFiles = jest.fn().mockResolvedValue({
|
||||
storage_session_id: 'session-42',
|
||||
files: [{ fileId: 'file-1', filename: 'brand-guidelines/references/style.md' }],
|
||||
files: [{ fileId: 'file-1', filename: 'skills/brand-guidelines/references/style.md' }],
|
||||
});
|
||||
|
||||
/* Defer resolution so we can assert the prime hasn't returned yet
|
||||
|
|
@ -303,7 +306,7 @@ describe('primeInvokedSkills — execute_code capability gate', () => {
|
|||
/* From the cache-hit path: pulls `resource_id` directly off
|
||||
* the persisted `codeEnvRef.id` (the skill `_id`). */
|
||||
resource_id: SKILL_ID.toString(),
|
||||
name: 'brand-guidelines/references/style.md',
|
||||
name: 'skills/brand-guidelines/references/style.md',
|
||||
storage_session_id: 'session-cached',
|
||||
kind: 'skill',
|
||||
version: SKILL_VERSION,
|
||||
|
|
@ -341,8 +344,8 @@ describe('primeSkillFiles — resource identity propagation', () => {
|
|||
batchUploadCodeEnvFiles: jest.fn().mockResolvedValue({
|
||||
storage_session_id: 'session-fresh',
|
||||
files: [
|
||||
{ fileId: 'file-fresh', filename: 'brand-guidelines/references/style.md' },
|
||||
{ fileId: 'file-skillmd', filename: 'brand-guidelines/SKILL.md' },
|
||||
{ fileId: 'file-fresh', filename: 'skills/brand-guidelines/references/style.md' },
|
||||
{ fileId: 'file-skillmd', filename: 'skills/brand-guidelines/SKILL.md' },
|
||||
],
|
||||
}),
|
||||
...overrides,
|
||||
|
|
@ -369,7 +372,7 @@ describe('primeSkillFiles — resource identity propagation', () => {
|
|||
id: 'file-fresh',
|
||||
resource_id: SKILL_ID.toString(),
|
||||
storage_session_id: 'session-fresh',
|
||||
name: 'brand-guidelines/references/style.md',
|
||||
name: 'skills/brand-guidelines/references/style.md',
|
||||
kind: 'skill',
|
||||
version: SKILL_VERSION,
|
||||
},
|
||||
|
|
@ -409,7 +412,7 @@ describe('primeSkillFiles — resource identity propagation', () => {
|
|||
id: 'file-cached',
|
||||
resource_id: SKILL_ID.toString(),
|
||||
storage_session_id: 'session-cached',
|
||||
name: 'brand-guidelines/references/style.md',
|
||||
name: 'skills/brand-guidelines/references/style.md',
|
||||
kind: 'skill',
|
||||
version: SKILL_VERSION,
|
||||
},
|
||||
|
|
|
|||
|
|
@ -6,6 +6,7 @@ import type { CodeEnvRef } from 'librechat-data-provider';
|
|||
import type { Types } from 'mongoose';
|
||||
import type { ServerRequest } from '~/types';
|
||||
import { extractInvokedSkillsFromPayload } from './run';
|
||||
import { SKILL_FILE_PREFIX } from './skills';
|
||||
import { logAxiosError } from '~/utils';
|
||||
|
||||
export interface SkillFileRecord {
|
||||
|
|
@ -151,7 +152,7 @@ export async function primeSkillFiles(
|
|||
id: ref.file_id,
|
||||
resource_id: ref.id,
|
||||
storage_session_id: ref.storage_session_id,
|
||||
name: `${skill.name}/${sf.relativePath}`,
|
||||
name: `${SKILL_FILE_PREFIX}${skill.name}/${sf.relativePath}`,
|
||||
kind: ref.kind,
|
||||
...(ref.kind === 'skill' ? { version: ref.version } : {}),
|
||||
});
|
||||
|
|
@ -175,7 +176,10 @@ export async function primeSkillFiles(
|
|||
|
||||
// SKILL.md from the skill body
|
||||
const bodyBuffer = Buffer.from(skill.body, 'utf-8');
|
||||
filesToUpload.push({ stream: Readable.from(bodyBuffer), filename: `${skill.name}/SKILL.md` });
|
||||
filesToUpload.push({
|
||||
stream: Readable.from(bodyBuffer),
|
||||
filename: `${SKILL_FILE_PREFIX}${skill.name}/SKILL.md`,
|
||||
});
|
||||
|
||||
// Bundled files from storage (parallel stream acquisition)
|
||||
const streamResults = await Promise.allSettled(
|
||||
|
|
@ -188,7 +192,7 @@ export async function primeSkillFiles(
|
|||
return null;
|
||||
}
|
||||
const stream = await strategy.getDownloadStream(req, file.filepath);
|
||||
return { stream, filename: `${skill.name}/${file.relativePath}` };
|
||||
return { stream, filename: `${SKILL_FILE_PREFIX}${skill.name}/${file.relativePath}` };
|
||||
}),
|
||||
);
|
||||
for (const result of streamResults) {
|
||||
|
|
@ -269,6 +273,12 @@ export async function primeSkillFiles(
|
|||
* returned to the caller are still valid.
|
||||
*/
|
||||
if (updateSkillFileCodeEnvIds) {
|
||||
/* Uploaded filenames are namespaced `skills/{skillName}/{relativePath}`
|
||||
* so the sandbox mount mirrors the model-facing skill namespace. The
|
||||
* persisted `relativePath` is the bare path (e.g. `references/style.md`),
|
||||
* so strip the `skills/{skillName}/` prefix rather than just the first
|
||||
* segment. */
|
||||
const sandboxPrefix = `${SKILL_FILE_PREFIX}${skill.name}/`;
|
||||
const updates = result.files
|
||||
.filter((f) => !f.filename.endsWith('/SKILL.md'))
|
||||
.map((f) => {
|
||||
|
|
@ -281,7 +291,9 @@ export async function primeSkillFiles(
|
|||
};
|
||||
return {
|
||||
skillId: skill._id,
|
||||
relativePath: f.filename.slice(f.filename.indexOf('/') + 1),
|
||||
relativePath: f.filename.startsWith(sandboxPrefix)
|
||||
? f.filename.slice(sandboxPrefix.length)
|
||||
: f.filename.slice(f.filename.indexOf('/') + 1),
|
||||
codeEnvRef: ref,
|
||||
};
|
||||
});
|
||||
|
|
@ -443,7 +455,7 @@ export async function primeInvokedSkills(
|
|||
const cachedFiles = resolvedWithRef.map(({ skillName, file, ref }) => ({
|
||||
id: ref!.file_id,
|
||||
resource_id: ref!.id,
|
||||
name: `${skillName}/${file.relativePath}`,
|
||||
name: `${SKILL_FILE_PREFIX}${skillName}/${file.relativePath}`,
|
||||
storage_session_id: ref!.storage_session_id,
|
||||
kind: ref!.kind,
|
||||
...(ref!.kind === 'skill' ? { version: ref!.version } : {}),
|
||||
|
|
|
|||
|
|
@ -51,6 +51,20 @@ export const MAX_PRIMED_SKILLS_PER_TURN = 30;
|
|||
*/
|
||||
export const MAX_SKILL_NAME_LENGTH = 200;
|
||||
|
||||
/**
|
||||
* Canonical namespace prefix for skill files. Single source of truth for
|
||||
* three layers that must agree:
|
||||
* - the `read_file`/`create_file`/`edit_file` authoring namespace shown to
|
||||
* the model (`skills/{skillName}/...`),
|
||||
* - the `handleReadFileCall` routing + bash-fallback paths in `handlers.ts`,
|
||||
* - the physical mount layout under `/mnt/data` (see `skillFiles.ts`, which
|
||||
* primes bundled files at `skills/{skillName}/...` so bash and the
|
||||
* model-facing namespace resolve to the same path on disk).
|
||||
*
|
||||
* Keep the trailing slash — call sites concatenate `${SKILL_FILE_PREFIX}${skillName}/...`.
|
||||
*/
|
||||
export const SKILL_FILE_PREFIX = 'skills/';
|
||||
|
||||
/**
|
||||
* Marker tagged onto every skill-primed message (as `additional_kwargs.source`
|
||||
* on a LangChain `HumanMessage`, or as `source` on the `InjectedMessage` that
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue