From 44ed7864fb545e8f6d9f9a032a46484e2263caf1 Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Thu, 4 Jun 2026 18:36:16 -0400 Subject: [PATCH] =?UTF-8?q?=F0=9F=93=9C=20feat:=20Improve=20Skill=20Author?= =?UTF-8?q?ing=20Guidance=20(#13517)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat: Improve skill authoring guidance * test: Guard tool description lengths * fix: Align skill template guidance * fix: Satisfy advisory limit test lint * fix: Transform LangGraph ESM in Jest --- api/jest.config.js | 20 +++++- api/package.json | 1 + package-lock.json | 1 + packages/api/jest.config.mjs | 8 +++ packages/api/src/agents/tools.spec.ts | 67 ++++++++++++++++++- packages/api/src/agents/tools.ts | 19 +++--- .../data-schemas/src/methods/skill.spec.ts | 2 + packages/data-schemas/src/methods/skill.ts | 2 + 8 files changed, 109 insertions(+), 11 deletions(-) diff --git a/api/jest.config.js b/api/jest.config.js index 47f8b7287b..07588ea455 100644 --- a/api/jest.config.js +++ b/api/jest.config.js @@ -1,3 +1,13 @@ +const esModules = [ + 'openid-client', + 'oauth4webapi', + 'jose', + '@langchain/langgraph', + '@langchain/langgraph-checkpoint', + '@langchain/langgraph-sdk', + 'uuid', +].join('|'); + module.exports = { testEnvironment: 'node', clearMocks: true, @@ -12,5 +22,13 @@ module.exports = { '^openid-client/passport$': '/test/__mocks__/openid-client-passport.js', '^openid-client$': '/test/__mocks__/openid-client.js', }, - transformIgnorePatterns: ['/node_modules/(?!(openid-client|oauth4webapi|jose)/).*/'], + transform: { + '\\.[jt]sx?$': [ + 'babel-jest', + { + presets: [['@babel/preset-env', { targets: { node: 'current' } }]], + }, + ], + }, + transformIgnorePatterns: [`/node_modules/(?!(${esModules})/).*/`], }; diff --git a/api/package.json b/api/package.json index 652cec9ccd..91c0b8e160 100644 --- a/api/package.json +++ b/api/package.json @@ -131,6 +131,7 @@ "zod": "^3.22.4" }, "devDependencies": { + "@babel/preset-env": "^7.29.5", "@types/sanitize-html": "^2.13.0", "jest": "^30.2.0", "mongodb-memory-server": "^11.0.1", diff --git a/package-lock.json b/package-lock.json index 8c9220c6b7..7931480bc6 100644 --- a/package-lock.json +++ b/package-lock.json @@ -146,6 +146,7 @@ "zod": "^3.22.4" }, "devDependencies": { + "@babel/preset-env": "^7.29.5", "@types/sanitize-html": "^2.13.0", "jest": "^30.2.0", "mongodb-memory-server": "^11.0.1", diff --git a/packages/api/jest.config.mjs b/packages/api/jest.config.mjs index b0029fe1a6..c49f4cc7cf 100644 --- a/packages/api/jest.config.mjs +++ b/packages/api/jest.config.mjs @@ -1,3 +1,10 @@ +const esModules = [ + '@langchain/langgraph', + '@langchain/langgraph-checkpoint', + '@langchain/langgraph-sdk', + 'uuid', +].join('|'); + export default { collectCoverageFrom: ['src/**/*.{js,jsx,ts,tsx}', '!/node_modules/'], coveragePathIgnorePatterns: ['/node_modules/', '/dist/'], @@ -23,6 +30,7 @@ export default { }, ], }, + transformIgnorePatterns: [`/node_modules/(?!(${esModules})/).*/`], moduleNameMapper: { '^@src/(.*)$': '/src/$1', '~/(.*)': '/src/$1', diff --git a/packages/api/src/agents/tools.spec.ts b/packages/api/src/agents/tools.spec.ts index 3a0306d0f5..570eba1708 100644 --- a/packages/api/src/agents/tools.spec.ts +++ b/packages/api/src/agents/tools.spec.ts @@ -5,7 +5,6 @@ * Mirrors the same pattern used in `__tests__/skills.test.ts`. */ jest.mock('@librechat/agents', () => ({ - ...jest.requireActual('@librechat/agents'), CODE_EXECUTION_TOOLS: new Set(['execute_code', 'bash_tool']), ReadFileToolDefinition: { name: 'read_file', @@ -51,6 +50,9 @@ import { isCodeSessionToolName, } from './tools'; +/** Portable ceiling for OpenAI-compatible tool description validators. */ +const TOOL_DESCRIPTION_ADVISORY_MAX_LENGTH = 1024; + function filePathDescription(tool?: LCTool): string { const parameters = tool?.parameters as | { properties?: { file_path?: { description?: string } } } @@ -58,6 +60,13 @@ function filePathDescription(tool?: LCTool): string { return parameters?.properties?.file_path?.description ?? ''; } +function maxToolDescriptionLength(definitions: LCTool[]): number { + return definitions.reduce((max, definition) => { + const length = definition.description?.length ?? Number.POSITIVE_INFINITY; + return Math.max(max, length); + }, 0); +} + describe('buildToolSet', () => { describe('event-driven mode (toolDefinitions)', () => { it('builds toolSet from toolDefinitions when available', () => { @@ -274,6 +283,30 @@ describe('registerCodeExecutionTools', () => { const names = result.toolDefinitions.map((d) => d.name); expect(names).toEqual(['calculator', 'read_file', 'bash_tool']); }); + + it('keeps code-execution tool descriptions within provider advisory limits', () => { + const skillAwareWithRefs = registerCodeExecutionTools({ + toolRegistry: makeRegistry(), + toolDefinitions: [], + includeBash: true, + includeSkillFileInstructions: true, + enableToolOutputReferences: true, + }); + const codeOnlyWithoutRefs = registerCodeExecutionTools({ + toolRegistry: makeRegistry(), + toolDefinitions: [], + includeBash: true, + includeSkillFileInstructions: false, + enableToolOutputReferences: false, + }); + + expect( + maxToolDescriptionLength([ + ...skillAwareWithRefs.toolDefinitions, + ...codeOnlyWithoutRefs.toolDefinitions, + ]), + ).toBeLessThanOrEqual(TOOL_DESCRIPTION_ADVISORY_MAX_LENGTH); + }); }); describe('idempotence (second call in same run)', () => { @@ -469,7 +502,12 @@ describe('registerFileAuthoringTools', () => { expect(result.toolDefinitions[0].responseFormat).toBe('content_and_artifact'); expect(result.toolDefinitions.map((d) => d.description).join('\n')).toContain('skills/'); expect(toolRegistry.get('create_file')?.description).toContain('frontmatter name must match'); + expect(toolRegistry.get('create_file')?.description).toContain('trigger-friendly'); + expect(toolRegistry.get('create_file')?.description).toContain('references/template.html'); + expect(toolRegistry.get('create_file')?.description).toContain('templates/{file}'); expect(toolRegistry.get('edit_file')?.description).toContain('edit_file cannot rename skills'); + expect(toolRegistry.get('edit_file')?.description).toContain('Keep SKILL.md concise'); + expect(toolRegistry.get('edit_file')?.description).toContain('templates/'); expect(filePathDescription(toolRegistry.get('create_file'))).toContain( 'frontmatter name must match', ); @@ -537,6 +575,33 @@ describe('registerFileAuthoringTools', () => { expect(toolRegistry.get('edit_file')?.description).toContain('skills/'); }); + it('keeps file-authoring tool descriptions within provider advisory limits', () => { + const skillAware = registerFileAuthoringTools({ + toolRegistry: makeRegistry(), + toolDefinitions: [], + includeSkillFileInstructions: true, + }); + const codeOnlyRegistry = makeRegistry(); + const codeOnly = registerFileAuthoringTools({ + toolRegistry: codeOnlyRegistry, + toolDefinitions: [], + includeSkillFileInstructions: false, + }); + const upgraded = registerFileAuthoringTools({ + toolRegistry: codeOnlyRegistry, + toolDefinitions: codeOnly.toolDefinitions, + includeSkillFileInstructions: true, + }); + + expect( + maxToolDescriptionLength([ + ...skillAware.toolDefinitions, + ...codeOnly.toolDefinitions, + ...upgraded.toolDefinitions, + ]), + ).toBeLessThanOrEqual(TOOL_DESCRIPTION_ADVISORY_MAX_LENGTH); + }); + it('distinguishes host file authoring definitions from user tools with matching names', () => { const result = registerFileAuthoringTools({ toolRegistry: makeRegistry(), diff --git a/packages/api/src/agents/tools.ts b/packages/api/src/agents/tools.ts index b86047cba7..f554dd3df6 100644 --- a/packages/api/src/agents/tools.ts +++ b/packages/api/src/agents/tools.ts @@ -263,17 +263,18 @@ const CODE_EDIT_FILE_PARAMETERS: LCTool['parameters'] = Object.freeze({ const SKILL_CREATE_FILE_DESCRIPTION = `Create a new file, or overwrite an existing file with explicit intent. -Use for new files and full rewrites where the change is larger than half the file. Requires overwrite: true to replace existing files. Refuses otherwise. +Use for new files and full rewrites. Requires overwrite: true to replace existing files. -Paths starting with "skills/" target the skill file system: -- skills/{skillName}/SKILL.md - the skill's main instruction file -- skills/{skillName}/references/{file} - supporting reference files -- skills/{skillName}/scripts/{file} - helper scripts -- skills/{skillName}/templates/{file} - output templates +Paths starting with "skills/" write skill files: +- skills/{skillName}/SKILL.md - main instructions; keep it lean with YAML frontmatter, trigger-friendly description, workflow steps, and short snippets. +- skills/{skillName}/references/{file} - long docs, schemas, examples, large templates, HTML/CSS/JS dashboards. +- skills/{skillName}/scripts/{file} - helper scripts. +- skills/{skillName}/assets/{file} - static assets. +- skills/{skillName}/templates/{file} - reusable output templates. -For skills/{skillName}/SKILL.md, YAML frontmatter name must match {skillName}. To use a different skill name, create a new skills/{newName}/SKILL.md instead. +For SKILL.md, frontmatter name must match {skillName}; create skills/{newName}/SKILL.md to rename. Put large runnable artifacts in bundled files such as references/template.html, and have SKILL.md tell the agent when to read or reuse them. -When code execution is enabled, non-skills paths target the code-execution sandbox. Prefer /mnt/data/{file} for files that should remain available to later sandbox calls.`; +Non-skills paths target the code-execution sandbox when enabled. Prefer /mnt/data/{file}.`; const CODE_CREATE_FILE_DESCRIPTION = `Create a new file, or overwrite an existing file with explicit intent. @@ -285,7 +286,7 @@ const SKILL_EDIT_FILE_DESCRIPTION = `Apply targeted text replacements to an exis Use for small, precise changes. Each old_text must match exactly one location. Tries exact match first; falls back to whitespace-tolerant matching if needed. Reports which matching strategy was used. Returns a unified diff. -For skills/{skillName}/SKILL.md, edit description, title, or body content, but keep YAML frontmatter name equal to {skillName}. edit_file cannot rename skills; create a new skills/{newName}/SKILL.md for a different skill name. +For skills/{skillName}/SKILL.md, edit description, title, or body content, but keep YAML frontmatter name equal to {skillName}. edit_file cannot rename skills; create a new skills/{newName}/SKILL.md for a different skill name. Keep SKILL.md concise; move large templates, HTML/CSS/JS dashboards, examples, schemas, and long docs into references/, scripts/, assets/, or templates/ files and point to them from SKILL.md. Paths starting with "skills/" target the skill file system. When code execution is enabled, non-skills paths target the code-execution sandbox.`; diff --git a/packages/data-schemas/src/methods/skill.spec.ts b/packages/data-schemas/src/methods/skill.spec.ts index bbd1d16530..c56ada418c 100644 --- a/packages/data-schemas/src/methods/skill.spec.ts +++ b/packages/data-schemas/src/methods/skill.spec.ts @@ -253,7 +253,9 @@ describe('skill validation helpers', () => { 'user-invocable': true, effort: 5, version: '1.0.0', + license: 'MIT', hooks: { 'pre-run': 'echo hi' }, + metadata: { owner: 'data-team' }, }), ).toEqual([]); }); diff --git a/packages/data-schemas/src/methods/skill.ts b/packages/data-schemas/src/methods/skill.ts index 85fc7d30a3..bb4b2d8786 100644 --- a/packages/data-schemas/src/methods/skill.ts +++ b/packages/data-schemas/src/methods/skill.ts @@ -251,6 +251,7 @@ const ALLOWED_FRONTMATTER_KEYS = new Set([ 'shell', 'hooks', 'version', + 'license', 'metadata', ]); @@ -277,6 +278,7 @@ const FRONTMATTER_KIND: Record = { paths: ['string', 'stringArray'], shell: 'string', version: 'string', + license: 'string', }; function isPlainObject(value: unknown): value is Record {