From 4fa86be424aff97c538598f888ea76574be0e883 Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Fri, 26 Jun 2026 12:22:06 -0400 Subject: [PATCH] =?UTF-8?q?=F0=9F=93=82=20fix:=20Mount=20Skill=20Files=20U?= =?UTF-8?q?nder=20`skills/`=20in=20Code=20Interpreter=20(#13961)=20(#13975?= =?UTF-8?q?)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- packages/api/src/agents/handlers.spec.ts | 100 +++++++++++++++++++++ packages/api/src/agents/handlers.ts | 23 +++-- packages/api/src/agents/skillFiles.spec.ts | 23 ++--- packages/api/src/agents/skillFiles.ts | 22 +++-- packages/api/src/agents/skills.ts | 14 +++ 5 files changed, 159 insertions(+), 23 deletions(-) diff --git a/packages/api/src/agents/handlers.spec.ts b/packages/api/src/agents/handlers.spec.ts index f195837735..9a7f7af995 100644 --- a/packages/api/src/agents/handlers.spec.ts +++ b/packages/api/src/agents/handlers.spec.ts @@ -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` diff --git a/packages/api/src/agents/handlers.ts b/packages/api/src/agents/handlers.ts index 8605ef498e..125a1a761d 100644 --- a/packages/api/src/agents/handlers.ts +++ b/packages/api/src/agents/handlers.ts @@ -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}`, }; } diff --git a/packages/api/src/agents/skillFiles.spec.ts b/packages/api/src/agents/skillFiles.spec.ts index a571e9a0af..95fb48bf9d 100644 --- a/packages/api/src/agents/skillFiles.spec.ts +++ b/packages/api/src/agents/skillFiles.spec.ts @@ -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, }, diff --git a/packages/api/src/agents/skillFiles.ts b/packages/api/src/agents/skillFiles.ts index b395f383b2..731c441a5f 100644 --- a/packages/api/src/agents/skillFiles.ts +++ b/packages/api/src/agents/skillFiles.ts @@ -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 } : {}), diff --git a/packages/api/src/agents/skills.ts b/packages/api/src/agents/skills.ts index b074daad5f..f6cc0ea9f1 100644 --- a/packages/api/src/agents/skills.ts +++ b/packages/api/src/agents/skills.ts @@ -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