🎛️ fix: Honor All Invocation-Mode Frontmatter Fields on Skill Import and Inline Edits

`POST /api/skills/import` silently discarded `user-invocable` and
`disable-model-invocation`, returning 201 with both columns at their schema
defaults. Those columns derive only from the structured `frontmatter` bag, and
import never passed one, so the flags had no channel to reach the document.
`always-apply` survived because it also travels an explicit column param and a
body-parse fallback.

Import now passes a sanitized bag, and the body-level cascade that previously
served only `always-apply` covers all three flags, so a flag declared inline is
honored on `POST`/`PATCH /api/skills` too — the create/edit forms send `body`
with no `frontmatter`, so that was the same defect on another endpoint, and
without it an imported restriction could never be released from the UI.

- parse.ts: one shared flag table drives parsing and the new `toCleanFrontmatter`,
  which rewrites each flag from its resolved value under the canonical key
- data-schemas: `checkFrontmatterEntry` shared by `validateSkillFrontmatter` and
  the new `pickValidFrontmatter`; body scanner generalized to all three flags
- sync/github.ts: local cleaner replaced by the shared one
- both frontmatter readers stop matching indented lines, and trust a resolved
  boolean when the key's line carries no inline text to contradict it

`allowedTools` stays bag-only: the body scan reads booleans, not YAML sequences,
so a body-only edit must not drop a list it cannot re-read.
This commit is contained in:
Danny Avila 2026-08-04 07:37:36 -04:00
parent 120ee2afa6
commit 1e1f751e92
10 changed files with 1423 additions and 240 deletions

View file

@ -0,0 +1,195 @@
import JSZip from 'jszip';
import mongoose, { Types } from 'mongoose';
import { MongoMemoryServer } from 'mongodb-memory-server';
import { logger, createModels, createMethods } from '@librechat/data-schemas';
import type { AllMethods, ISkill } from '@librechat/data-schemas';
import type { Response } from 'express';
import type { ImportSkillDeps } from '../import';
import { createImportHandler } from '../import';
logger.silent = true;
/**
* End-to-end coverage for #14208: the import handler wired to the REAL
* `createSkill`, asserting the persisted document rather than the arguments a
* mock received. `import.test.ts` pins the frontmatter bag the handler builds;
* this pins the columns that bag actually produces, so the two halves of the
* fix can't drift apart silently.
*/
type ImportRequest = Parameters<ReturnType<typeof createImportHandler>>[0];
type CapturedResponse = Response & { statusCode?: number; body?: unknown };
let mongoServer: MongoMemoryServer;
let methods: AllMethods;
let author: Types.ObjectId;
beforeAll(async () => {
mongoServer = await MongoMemoryServer.create();
await mongoose.connect(mongoServer.getUri());
createModels(mongoose);
methods = createMethods(mongoose);
});
afterAll(async () => {
await mongoose.disconnect();
await mongoServer.stop();
});
beforeEach(async () => {
author = new Types.ObjectId();
await mongoose.connection.collection('skills').deleteMany({});
});
function captureResponse(): CapturedResponse {
const res = {} as CapturedResponse;
res.status = jest.fn((statusCode: number) => {
res.statusCode = statusCode;
return res;
}) as CapturedResponse['status'];
res.json = jest.fn((body: unknown) => {
res.body = body;
return res;
}) as CapturedResponse['json'];
return res;
}
function importDeps(): ImportSkillDeps {
return {
createSkill: methods.createSkill,
getSkillById: methods.getSkillById,
deleteSkill: methods.deleteSkill,
upsertSkillFile: methods.upsertSkillFile,
saveBuffer: jest.fn(async () => ({ filepath: '/tmp/skill-file', source: 'local' })),
grantPermission: jest.fn(async () => undefined),
};
}
function request(content: string | Buffer, originalname: string): ImportRequest {
return {
user: { id: author.toString(), _id: author, username: 'importer' },
file: {
originalname,
buffer: typeof content === 'string' ? Buffer.from(content) : content,
},
} as unknown as ImportRequest;
}
async function zipped(skillMarkdown: string): Promise<Buffer> {
const zip = new JSZip();
zip.file('SKILL.md', skillMarkdown);
return zip.generateAsync({ type: 'nodebuffer', compression: 'DEFLATE' });
}
async function persisted(res: CapturedResponse): Promise<ISkill> {
const created = res.body as ISkill & { _id: Types.ObjectId };
const reloaded = await methods.getSkillById(created._id);
if (!reloaded) {
throw new Error('Imported skill was not persisted');
}
return reloaded;
}
/** Verbatim from the issue's reproduction steps. */
const REPORTED_SKILL_MD = [
'---',
'name: test-skill',
'description: test',
'always-apply: true',
'user-invocable: false',
'disable-model-invocation: true',
'---',
'Test body.',
].join('\n');
describe('POST /api/skills/import — invocation-mode frontmatter (#14208)', () => {
it('persists all three invocation-mode flags from a markdown upload', async () => {
const res = captureResponse();
await createImportHandler(importDeps())(request(REPORTED_SKILL_MD, 'test-skill.md'), res);
expect(res.status).toHaveBeenCalledWith(201);
const skill = await persisted(res);
expect(skill.name).toBe('test-skill');
expect(skill.alwaysApply).toBe(true);
expect(skill.userInvocable).toBe(false);
expect(skill.disableModelInvocation).toBe(true);
});
it('persists all three invocation-mode flags from an archive upload', async () => {
const res = captureResponse();
await createImportHandler(importDeps())(
request(await zipped(REPORTED_SKILL_MD), 'test-skill.skill'),
res,
);
expect(res.status).toHaveBeenCalledWith(201);
const skill = await persisted(res);
expect(skill.alwaysApply).toBe(true);
expect(skill.userInvocable).toBe(false);
expect(skill.disableModelInvocation).toBe(true);
});
it('leaves the schema defaults in place when the file declares no flags', async () => {
const res = captureResponse();
const markdown = '---\nname: plain-skill\ndescription: A skill with no flags.\n---\n\nbody';
await createImportHandler(importDeps())(request(markdown, 'plain-skill.md'), res);
expect(res.status).toHaveBeenCalledWith(201);
const skill = await persisted(res);
expect(skill.alwaysApply).toBe(false);
expect(skill.userInvocable).toBe(true);
expect(skill.disableModelInvocation).toBe(false);
expect(skill.allowedTools).toBeUndefined();
});
it('persists the allowedTools column declared by the uploaded file', async () => {
const res = captureResponse();
const markdown = [
'---',
'name: tooled-skill',
'description: A skill declaring extra tools.',
'allowed-tools:',
' - web_search',
' - file_search',
'---',
'body',
].join('\n');
await createImportHandler(importDeps())(request(markdown, 'tooled-skill.md'), res);
expect(res.status).toHaveBeenCalledWith(201);
const skill = await persisted(res);
expect(skill.allowedTools).toEqual(['web_search', 'file_search']);
});
it('imports a file whose extra frontmatter would fail strict validation', async () => {
const res = captureResponse();
const markdown = [
'---',
'name: ecosystem-skill',
'description: Authored for another skill ecosystem.',
'icon: rocket',
'version: 1.0',
'user-invocable: false',
'---',
'body',
].join('\n');
await createImportHandler(importDeps())(request(markdown, 'ecosystem-skill.md'), res);
expect(res.status).toHaveBeenCalledWith(201);
const skill = await persisted(res);
expect(skill.userInvocable).toBe(false);
expect(skill.frontmatter).toEqual({ 'user-invocable': false });
});
it('rejects a malformed flag value instead of persisting a skill at the default', async () => {
const res = captureResponse();
const markdown =
'---\nname: broken-skill\ndescription: A skill with a bad flag.\nuser-invocable: yes\n---\n\nbody';
await createImportHandler(importDeps())(request(markdown, 'broken-skill.md'), res);
expect(res.status).toHaveBeenCalledWith(400);
await expect(
mongoose.connection.collection('skills').countDocuments({ name: 'broken-skill' }),
).resolves.toBe(0);
});
});

View file

@ -110,6 +110,18 @@ async function zipWithSkillMarkdown(skillMarkdown: string): Promise<Buffer> {
return zip.generateAsync({ type: 'nodebuffer', compression: 'DEFLATE' });
}
/** The reporter's repro from #14208: all three invocation-mode flags off-default. */
const INVOCATION_MODE_SKILL_MD = [
'---',
'name: test-skill',
'description: A skill that restricts its invocation channels.',
'always-apply: true',
'user-invocable: false',
'disable-model-invocation: true',
'---',
'Test body.',
].join('\n');
describe('parseFrontmatter', () => {
it('extracts name + description from a minimal frontmatter block', () => {
const raw = `---\nname: demo\ndescription: A demo skill.\n---\n\n# Body`;
@ -117,6 +129,7 @@ describe('parseFrontmatter', () => {
name: 'demo',
description: 'A demo skill.',
alwaysApply: undefined,
frontmatter: {},
invalidBooleans: [],
});
});
@ -127,6 +140,7 @@ describe('parseFrontmatter', () => {
name: '123',
description: '2024',
alwaysApply: undefined,
frontmatter: {},
invalidBooleans: [],
});
});
@ -137,6 +151,7 @@ describe('parseFrontmatter', () => {
name: 'legal',
description: 'Legal rules.',
alwaysApply: true,
frontmatter: { 'always-apply': true },
invalidBooleans: [],
});
});
@ -147,16 +162,18 @@ describe('parseFrontmatter', () => {
name: 'optional',
description: 'Optional rules.',
alwaysApply: false,
frontmatter: { 'always-apply': false },
invalidBooleans: [],
});
});
it('extracts alwaysApply: true', () => {
it('extracts alwaysApply: true and canonicalizes the alias in the bag', () => {
const raw = `---\nname: legal\ndescription: Legal rules.\nalwaysApply: true\n---\n\n# Legal body`;
expect(parseFrontmatter(raw)).toEqual({
name: 'legal',
description: 'Legal rules.',
alwaysApply: true,
frontmatter: { 'always-apply': true },
invalidBooleans: [],
});
});
@ -233,6 +250,7 @@ describe('parseFrontmatter', () => {
name: 'quoted-name',
description: 'quoted desc',
alwaysApply: true,
frontmatter: { 'always-apply': true },
invalidBooleans: [],
});
});
@ -242,6 +260,7 @@ describe('parseFrontmatter', () => {
expect(parseFrontmatter(raw)).toEqual({
name: '',
description: '',
frontmatter: {},
invalidBooleans: [],
});
});
@ -252,6 +271,7 @@ describe('parseFrontmatter', () => {
name: 'prologue',
description: 'Has leading whitespace.',
alwaysApply: undefined,
frontmatter: {},
invalidBooleans: [],
});
});
@ -262,6 +282,7 @@ describe('parseFrontmatter', () => {
name: 'marker',
description: 'first ---not a closing fence last',
alwaysApply: false,
frontmatter: { 'always-apply': false },
invalidBooleans: [],
});
});
@ -271,6 +292,7 @@ describe('parseFrontmatter', () => {
expect(parseFrontmatter(raw)).toEqual({
name: '',
description: '',
frontmatter: {},
invalidBooleans: [],
});
});
@ -328,6 +350,212 @@ describe('parseFrontmatter', () => {
expect(result.alwaysApply).toBe(false);
expect(result.invalidBooleans).toEqual([]);
});
it('extracts user-invocable and disable-model-invocation alongside always-apply', () => {
const raw = [
'---',
'name: test-skill',
'description: test',
'always-apply: true',
'user-invocable: false',
'disable-model-invocation: true',
'---',
'Test body.',
].join('\n');
expect(parseFrontmatter(raw)).toEqual({
name: 'test-skill',
description: 'test',
alwaysApply: true,
userInvocable: false,
disableModelInvocation: true,
frontmatter: {
'always-apply': true,
'user-invocable': false,
'disable-model-invocation': true,
},
invalidBooleans: [],
});
});
it.each([
['user-invocable', 'userInvocable'],
['disable-model-invocation', 'disableModelInvocation'],
] as const)('extracts %s: false', (key, field) => {
const raw = `---\nname: n\ndescription: d\n${key}: false\n---\n\nbody`;
const result = parseFrontmatter(raw);
expect(result[field]).toBe(false);
expect(result.frontmatter).toEqual({ [key]: false });
expect(result.invalidBooleans).toEqual([]);
});
it.each(['user-invocable', 'disable-model-invocation'])(
'flags a non-boolean %s value instead of silently defaulting it',
(key) => {
const raw = `---\nname: n\ndescription: d\n${key}: yes\n---\n\nbody`;
const result = parseFrontmatter(raw);
expect(result.invalidBooleans).toEqual([key]);
expect(result.frontmatter).toEqual({});
},
);
it.each(['user-invocable', 'disable-model-invocation'])(
'treats an empty %s value as absent (mid-edit placeholder)',
(key) => {
const raw = `---\nname: n\ndescription: d\n${key}:\n---\n\nbody`;
const result = parseFrontmatter(raw);
expect(result.invalidBooleans).toEqual([]);
expect(result.frontmatter).toEqual({});
},
);
it('normalizes quoted and comment-trailed invocation booleans', () => {
const raw = [
'---',
'name: n',
'description: d',
'user-invocable: "false" # manual off',
"disable-model-invocation: 'true'",
'---',
'body',
].join('\n');
const result = parseFrontmatter(raw);
expect(result.userInvocable).toBe(false);
expect(result.disableModelInvocation).toBe(true);
expect(result.frontmatter).toEqual({
'user-invocable': false,
'disable-model-invocation': true,
});
});
it('is case-insensitive on the new flag keys', () => {
const raw = `---\nname: n\ndescription: d\nUser-Invocable: FALSE\nDISABLE-MODEL-INVOCATION: True\n---\n\nbody`;
const result = parseFrontmatter(raw);
expect(result.userInvocable).toBe(false);
expect(result.disableModelInvocation).toBe(true);
});
it('reports every malformed flag at once', () => {
const raw = [
'---',
'name: n',
'description: d',
'always-apply: tru',
'user-invocable: nope',
'disable-model-invocation: 1',
'---',
'body',
].join('\n');
expect(parseFrontmatter(raw).invalidBooleans).toEqual([
'always-apply',
'user-invocable',
'disable-model-invocation',
]);
});
it('keeps recognized non-flag frontmatter in the bag', () => {
const raw = [
'---',
'name: n',
'description: d',
'when-to-use: When demoing.',
'allowed-tools:',
' - web_search',
'license: MIT',
'---',
'body',
].join('\n');
expect(parseFrontmatter(raw).frontmatter).toEqual({
'when-to-use': 'When demoing.',
'allowed-tools': ['web_search'],
license: 'MIT',
});
});
it('drops unrecognized and malformed non-flag keys rather than failing the parse', () => {
const raw = [
'---',
'name: n',
'description: d',
'icon: rocket',
'version: 1.0',
'user-invocable: false',
'---',
'body',
].join('\n');
const result = parseFrontmatter(raw);
expect(result.frontmatter).toEqual({ 'user-invocable': false });
expect(result.invalidBooleans).toEqual([]);
expect(result.parseError).toBeUndefined();
});
it.each([
['always-apply', 'true', 'alwaysApply', true],
['user-invocable', 'false', 'userInvocable', false],
['disable-model-invocation', 'true', 'disableModelInvocation', true],
] as const)(
'resolves %s when YAML continues the value on the next line',
(key, text, field, expected) => {
const raw = `---\nname: n\ndescription: d\n${key}:\n ${text}\n---\n\nbody`;
const result = parseFrontmatter(raw);
expect(result[field]).toBe(expected);
expect(result.invalidBooleans).toEqual([]);
expect(result.frontmatter).toEqual({ [key]: expected });
},
);
it('resolves flags when the whole frontmatter mapping is indented', () => {
const raw = `---\n name: n\n description: d\n user-invocable: false\n---\n\nbody`;
const result = parseFrontmatter(raw);
expect(result.userInvocable).toBe(false);
expect(result.invalidBooleans).toEqual([]);
});
it('ignores a nested mapping key that reuses a flag name', () => {
const raw = [
'---',
'name: n',
'description: d',
'metadata:',
' user-invocable: nonsense',
'user-invocable: false',
'---',
'body',
].join('\n');
const result = parseFrontmatter(raw);
expect(result.userInvocable).toBe(false);
expect(result.invalidBooleans).toEqual([]);
});
it('does not read a flag from a nested mapping when the top level omits it', () => {
const raw = `---\nname: n\ndescription: d\nmetadata:\n user-invocable: nonsense\n---\n\nbody`;
const result = parseFrontmatter(raw);
expect(result.userInvocable).toBeUndefined();
expect(result.invalidBooleans).toEqual([]);
});
it('preserves frontmatter key order when rewriting flags', () => {
const raw = [
'---',
'name: n',
'description: d',
'user-invocable: "false"',
'license: MIT',
'---',
'body',
].join('\n');
expect(Object.keys(parseFrontmatter(raw).frontmatter)).toEqual(['user-invocable', 'license']);
});
});
describe('createImportHandler', () => {
@ -415,4 +643,139 @@ describe('createImportHandler', () => {
);
expect(deps.createSkill).not.toHaveBeenCalled();
});
it('forwards every invocation-mode flag from a markdown import', async () => {
const deps = mockImportDeps();
const handler = createImportHandler(deps);
const res = mockResponse();
await handler(mockMarkdownRequest(INVOCATION_MODE_SKILL_MD, 'test-skill.md'), res);
expect(res.status).toHaveBeenCalledWith(201);
expect(deps.createSkill).toHaveBeenCalledWith(
expect.objectContaining({
name: 'test-skill',
alwaysApply: true,
frontmatter: {
'always-apply': true,
'user-invocable': false,
'disable-model-invocation': true,
},
}),
);
});
it('forwards every invocation-mode flag from an archive import', async () => {
const deps = mockImportDeps();
const handler = createImportHandler(deps);
const res = mockResponse();
const buffer = await zipWithSkillMarkdown(INVOCATION_MODE_SKILL_MD);
await handler(mockZipRequest(buffer), res);
expect(res.status).toHaveBeenCalledWith(201);
expect(deps.createSkill).toHaveBeenCalledWith(
expect.objectContaining({
alwaysApply: true,
frontmatter: {
'always-apply': true,
'user-invocable': false,
'disable-model-invocation': true,
},
}),
);
});
it('forwards allowed-tools so the derived column is populated on import', async () => {
const deps = mockImportDeps();
const handler = createImportHandler(deps);
const res = mockResponse();
const markdown = [
'---',
'name: tooled-skill',
'description: A skill that declares extra tools.',
'allowed-tools:',
' - web_search',
' - file_search',
'---',
'body',
].join('\n');
await handler(mockMarkdownRequest(markdown, 'tooled-skill.md'), res);
expect(res.status).toHaveBeenCalledWith(201);
expect(deps.createSkill).toHaveBeenCalledWith(
expect.objectContaining({
frontmatter: { 'allowed-tools': ['web_search', 'file_search'] },
}),
);
});
it.each(['user-invocable', 'disable-model-invocation'])(
'rejects a malformed %s value instead of importing it at the schema default',
async (key) => {
const deps = mockImportDeps();
const handler = createImportHandler(deps);
const res = mockResponse();
const markdown = `---\nname: broken-skill\ndescription: A skill with a bad flag.\n${key}: yes\n---\n\nbody`;
await handler(mockMarkdownRequest(markdown, 'broken-skill.md'), res);
expect(res.status).toHaveBeenCalledWith(400);
expect(res.body).toEqual(
expect.objectContaining({
error: 'Validation failed',
issues: [
{
field: `frontmatter.${key}`,
code: 'INVALID_TYPE',
message: `"${key}" must be a boolean (true or false)`,
},
],
}),
);
expect(deps.createSkill).not.toHaveBeenCalled();
},
);
it('imports a file carrying unknown frontmatter keys, dropping only those keys', async () => {
const deps = mockImportDeps();
const handler = createImportHandler(deps);
const res = mockResponse();
const markdown = [
'---',
'name: ecosystem-skill',
'description: Authored for another skill ecosystem.',
'icon: rocket',
'version: 1.0',
'user-invocable: false',
'---',
'body',
].join('\n');
await handler(mockMarkdownRequest(markdown, 'ecosystem-skill.md'), res);
expect(res.status).toHaveBeenCalledWith(201);
expect(deps.createSkill).toHaveBeenCalledWith(
expect.objectContaining({
frontmatter: { 'user-invocable': false },
}),
);
});
it('sends an empty frontmatter bag when the file has no frontmatter block', async () => {
const deps = mockImportDeps();
const handler = createImportHandler(deps);
const res = mockResponse();
await handler(mockMarkdownRequest('# Just a body', 'bodyless-skill.md'), res);
expect(res.status).toHaveBeenCalledWith(201);
expect(deps.createSkill).toHaveBeenCalledWith(
expect.objectContaining({
name: 'bodyless-skill',
frontmatter: {},
}),
);
});
});

View file

@ -1,7 +1,7 @@
import path from 'path';
import JSZip from 'jszip';
import crypto from 'crypto';
import { logger } from '@librechat/data-schemas';
import { logger, pickValidFrontmatter } from '@librechat/data-schemas';
import { ResourceType, AccessRoleIds, PrincipalType } from 'librechat-data-provider';
import type {
ISkill,
@ -13,73 +13,101 @@ import type {
import type { Request, Response } from 'express';
import type { Types } from 'mongoose';
import type { ImportLimits } from './limits';
import { parseSkillMarkdown, toCleanFrontmatter } from './parse';
import { resolveRequestTenantId } from '~/middleware/tenant';
import { DEFAULT_SKILL_IMPORT_LIMITS } from './limits';
import { isSafeSkillFilePath } from './path';
import { parseSkillMarkdown } from './parse';
const SKILL_MD = 'SKILL.md';
export type { ImportLimits } from './limits';
/**
* YAML frontmatter parser extracts the first-class fields LibreChat
* persists as columns (`name`, `description`, `alwaysApply`) out of a
* SKILL.md file. Intentionally narrow: the full frontmatter validator in
* `packages/data-schemas/src/methods/skill.ts` covers the wire contract;
* this parser only needs to hand `createSkill` the columns it populates.
*
* When a known boolean field (currently `always-apply` plus the accepted
* `alwaysApply` alias) is present
* with a value that isn't recognizable as `true`/`false`, the parser
* records it on `invalidBooleans[]` so the import handler can surface
* a 400 instead of silently dropping the flag. Without this signal,
* authoring mistakes like `alwaysApply: yes` would be lossy-converted
* to "not always-applied" and the user would never learn their
* frontmatter was malformed.
*
* Exported for unit testing only prefer `createImportHandler` at runtime.
*/
export function parseFrontmatter(raw: string): {
type ParsedSkillFile = {
name: string;
description: string;
alwaysApply?: boolean;
userInvocable?: boolean;
disableModelInvocation?: boolean;
/**
* Frontmatter bag ready for `createSkill`: cleaned of the fields that live
* as their own columns, and narrowed to entries strict validation accepts.
*/
frontmatter: Record<string, unknown>;
/** Keys that carried non-boolean values for fields that must be boolean. */
invalidBooleans: string[];
parseError?: string;
} {
};
/**
* YAML frontmatter parser for an uploaded SKILL.md hands `createSkill` the
* first-class columns (`name`, `description`, `alwaysApply`) plus the
* frontmatter bag the remaining columns are derived from. Every
* invocation-mode flag (`always-apply` with its accepted `alwaysApply` alias,
* `user-invocable`, `disable-model-invocation`) reaches the document through
* that bag, so an uploaded file governs its own invocation channels the same
* way an explicit `frontmatter` payload to `POST /api/skills` does.
*
* When a boolean field carries a value that isn't recognizable as
* `true`/`false`, the parser records it on `invalidBooleans[]` so the import
* handler can surface a 400 instead of silently dropping the flag. Without
* this signal, authoring mistakes like `user-invocable: yes` would be
* lossy-converted to the schema default and the user would never learn their
* frontmatter was malformed.
*
* Entries outside the flags are filtered rather than rejected: an uploaded
* file is not something the uploader typed into a form, so a stray
* `version: 1.0` or a bespoke key from another skill ecosystem must not fail
* the upload. Admin-authored paths (GitHub sync, deployment skills) keep the
* strict validator's hard failure instead.
*
* Exported for unit testing only prefer `createImportHandler` at runtime.
*/
export function parseFrontmatter(raw: string): ParsedSkillFile {
const parsed = parseSkillMarkdown(raw);
const result: {
name: string;
description: string;
alwaysApply?: boolean;
invalidBooleans: string[];
parseError?: string;
} = {
const result: ParsedSkillFile = {
name: parsed.name,
description: parsed.description,
alwaysApply: parsed.alwaysApply,
userInvocable: parsed.userInvocable,
disableModelInvocation: parsed.disableModelInvocation,
frontmatter: pickValidFrontmatter(toCleanFrontmatter(parsed)),
invalidBooleans: parsed.invalidBooleans,
};
if (parsed.parseError) {
result.parseError = parsed.parseError;
}
if ('alwaysApply' in parsed) {
result.alwaysApply = parsed.alwaysApply;
}
return result;
}
function sendFrontmatterParseError(res: Response, parseError: string) {
return res.status(400).json({
error: 'Validation failed',
issues: [
{
field: 'frontmatter',
code: 'INVALID_YAML',
message: `Invalid YAML frontmatter: ${parseError}`,
},
],
});
/**
* Reject a SKILL.md whose frontmatter can't be trusted unparseable YAML, or
* a boolean invocation-mode flag with a value that is neither `true` nor
* `false`. Returns the sent response, or `null` when the parse is clean.
*/
function sendFrontmatterIssues(res: Response, parsed: ParsedSkillFile): Response | null {
if (parsed.parseError) {
return res.status(400).json({
error: 'Validation failed',
issues: [
{
field: 'frontmatter',
code: 'INVALID_YAML',
message: `Invalid YAML frontmatter: ${parsed.parseError}`,
},
],
});
}
if (parsed.invalidBooleans.length > 0) {
return res.status(400).json({
error: 'Validation failed',
issues: parsed.invalidBooleans.map((key) => ({
field: `frontmatter.${key}`,
code: 'INVALID_TYPE',
message: `"${key}" must be a boolean (true or false)`,
})),
});
}
return null;
}
/** Type guard for validation errors thrown by data-schemas. */
@ -247,20 +275,12 @@ async function handleMarkdown(
) {
const content = file.buffer.toString('utf-8');
const { name, description, alwaysApply, invalidBooleans, parseError } = parseFrontmatter(content);
if (parseError) {
return sendFrontmatterParseError(res, parseError);
}
if (invalidBooleans.length > 0) {
return res.status(400).json({
error: 'Validation failed',
issues: invalidBooleans.map((key) => ({
field: `frontmatter.${key}`,
code: 'INVALID_TYPE',
message: `"${key}" must be a boolean (true or false)`,
})),
});
const parsed = parseFrontmatter(content);
const frontmatterError = sendFrontmatterIssues(res, parsed);
if (frontmatterError) {
return frontmatterError;
}
const { name, description, alwaysApply, frontmatter } = parsed;
const inferredName =
name ||
file.originalname
@ -280,6 +300,7 @@ async function handleMarkdown(
name: inferredName,
description: description || inferredName,
body: content,
frontmatter,
author: authorId,
authorName,
alwaysApply,
@ -361,21 +382,12 @@ async function handleZip(
return res.status(400).json({ error: 'SKILL.md exceeds maximum file size' });
}
const { name, description, alwaysApply, invalidBooleans, parseError } =
parseFrontmatter(skillMdContent);
if (parseError) {
return sendFrontmatterParseError(res, parseError);
}
if (invalidBooleans.length > 0) {
return res.status(400).json({
error: 'Validation failed',
issues: invalidBooleans.map((key) => ({
field: `frontmatter.${key}`,
code: 'INVALID_TYPE',
message: `"${key}" must be a boolean (true or false)`,
})),
});
const parsed = parseFrontmatter(skillMdContent);
const frontmatterError = sendFrontmatterIssues(res, parsed);
if (frontmatterError) {
return frontmatterError;
}
const { name, description, alwaysApply, frontmatter } = parsed;
const inferredName =
name ||
file.originalname
@ -395,6 +407,7 @@ async function handleZip(
name: inferredName,
description: description || inferredName,
body: skillMdContent,
frontmatter,
author: authorId,
authorName,
alwaysApply,

View file

@ -1,14 +1,24 @@
import yaml from 'js-yaml';
import { SKILL_BOOLEAN_FLAGS } from '@librechat/data-schemas';
import type { SkillBooleanFlag, SkillBooleanColumn } from '@librechat/data-schemas';
export type ParsedSkillMarkdown = {
name: string;
description: string;
alwaysApply?: boolean;
userInvocable?: boolean;
disableModelInvocation?: boolean;
frontmatter?: Record<string, unknown>;
invalidBooleans: string[];
parseError?: string;
};
const SKILL_BOOLEAN_FLAG_BY_KEY = new Map<string, SkillBooleanFlag>(
SKILL_BOOLEAN_FLAGS.flatMap((flag) =>
[flag.key, ...flag.aliases].map((key) => [key, flag] as const),
),
);
function isPlainObject(value: unknown): value is Record<string, unknown> {
return typeof value === 'object' && value !== null && !Array.isArray(value);
}
@ -33,17 +43,29 @@ function extractFrontmatterBlock(raw: string): string | null {
}
function getCaseInsensitive(frontmatter: Record<string, unknown>, key: string): unknown {
const entry = Object.entries(frontmatter).find(([candidate]) => candidate.toLowerCase() === key);
const target = key.toLowerCase();
const entry = Object.entries(frontmatter).find(
([candidate]) => candidate.toLowerCase() === target,
);
return entry?.[1];
}
function hasCaseInsensitive(frontmatter: Record<string, unknown>, key: string): boolean {
return Object.keys(frontmatter).some((candidate) => candidate.toLowerCase() === key);
const target = key.toLowerCase();
return Object.keys(frontmatter).some((candidate) => candidate.toLowerCase() === target);
}
/**
* Recover the text a key carried on its own line, used to cross-check the
* value the YAML parser resolved. Anchored at column zero so a nested mapping
* that reuses a flag name (`metadata:` ` user-invocable: ...`) can't shadow
* the top-level key; nested keys never reach the top-level bag, so matching one
* here would attribute an unrelated value to the flag. Returns `undefined` when
* no line matches, which callers treat as "nothing to cross-check".
*/
function getRawFrontmatterValue(block: string, key: string): string | undefined {
const escapedKey = key.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
const pattern = new RegExp(`^\\s*${escapedKey}\\s*:\\s*(.*)$`, 'i');
const pattern = new RegExp(`^${escapedKey}\\s*:\\s*(.*)$`, 'i');
const line = block.split('\n').find((candidate) => pattern.test(candidate));
const match = line?.match(pattern);
return match?.[1];
@ -75,6 +97,15 @@ function normalizeFrontmatterKeys(frontmatter: Record<string, unknown>): Record<
function parseBoolean(value: unknown, rawValue?: string): boolean | undefined {
const raw = rawValue === undefined ? undefined : stripInlineComment(rawValue).toLowerCase();
if (typeof value === 'boolean') {
/* The raw line exists only to reject inline text that contradicts the
resolved value (an explicit `!!bool` tag, say). When the key's line holds
no value of its own because YAML continues it on the following line, or
the whole mapping is indented and the line scan finds nothing there is
nothing to contradict, so trust the boolean the parser resolved. Treating
it as absent instead would silently discard a declared flag. */
if (raw === undefined || raw.length === 0) {
return value;
}
return raw === 'true' || raw === 'false' ? value : undefined;
}
if (typeof value !== 'string') {
@ -94,6 +125,43 @@ function hasBooleanPlaceholder(rawValue?: string): boolean {
return rawValue !== undefined && stripInlineComment(rawValue).length === 0;
}
type ResolvedBooleanFlag = { value?: boolean; invalidKey?: string };
/**
* Read a flag key already known to be present. A value that is neither
* `true` nor `false` is reported on `invalidKey` under the key's authored
* spelling rather than silently collapsing to "off", except for an empty
* value (`user-invocable:` with nothing after it), which is a mid-edit
* placeholder and treated as absent.
*/
function readPresentBooleanFlag(
frontmatter: Record<string, unknown>,
block: string,
key: string,
): ResolvedBooleanFlag {
const rawValue = getRawFrontmatterValue(block, key);
const value = parseBoolean(getCaseInsensitive(frontmatter, key), rawValue);
if (value === undefined && !hasBooleanPlaceholder(rawValue)) {
return { invalidKey: key };
}
return { value };
}
function resolveBooleanFlag(
frontmatter: Record<string, unknown>,
block: string,
flag: SkillBooleanFlag,
): ResolvedBooleanFlag {
if (hasCaseInsensitive(frontmatter, flag.key)) {
return readPresentBooleanFlag(frontmatter, block, flag.key);
}
const alias = flag.aliases.find((candidate) => hasCaseInsensitive(frontmatter, candidate));
if (alias !== undefined) {
return readPresentBooleanFlag(frontmatter, block, alias);
}
return {};
}
function toScalarString(value: unknown): string {
if (typeof value === 'string') {
return value;
@ -124,12 +192,6 @@ export function parseSkillMarkdown(raw: string): ParsedSkillMarkdown {
const nameValue = getCaseInsensitive(frontmatter, 'name');
const descriptionValue = getCaseInsensitive(frontmatter, 'description');
const whenToUseValue = getCaseInsensitive(frontmatter, 'when-to-use');
const hasCanonicalAlwaysApply = hasCaseInsensitive(frontmatter, 'always-apply');
const hasAliasAlwaysApply = hasCaseInsensitive(frontmatter, 'alwaysapply');
const canonicalAlwaysApplyValue = getCaseInsensitive(frontmatter, 'always-apply');
const aliasAlwaysApplyValue = getCaseInsensitive(frontmatter, 'alwaysapply');
const rawCanonicalAlwaysApplyValue = getRawFrontmatterValue(block, 'always-apply');
const rawAliasAlwaysApplyValue = getRawFrontmatterValue(block, 'alwaysApply');
const name = toScalarString(nameValue);
let description = '';
if (descriptionValue !== undefined) {
@ -137,24 +199,60 @@ export function parseSkillMarkdown(raw: string): ParsedSkillMarkdown {
} else if (whenToUseValue !== undefined) {
description = toScalarString(whenToUseValue);
}
let alwaysApply: boolean | undefined;
const flagValues: Partial<Record<SkillBooleanColumn, boolean>> = {};
const invalidBooleans: string[] = [];
if (hasCanonicalAlwaysApply) {
alwaysApply = parseBoolean(canonicalAlwaysApplyValue, rawCanonicalAlwaysApplyValue);
if (alwaysApply === undefined && !hasBooleanPlaceholder(rawCanonicalAlwaysApplyValue)) {
invalidBooleans.push('always-apply');
for (const flag of SKILL_BOOLEAN_FLAGS) {
const { value, invalidKey } = resolveBooleanFlag(frontmatter, block, flag);
if (invalidKey !== undefined) {
invalidBooleans.push(invalidKey);
continue;
}
} else if (hasAliasAlwaysApply) {
alwaysApply = parseBoolean(aliasAlwaysApplyValue, rawAliasAlwaysApplyValue);
if (alwaysApply === undefined && !hasBooleanPlaceholder(rawAliasAlwaysApplyValue)) {
invalidBooleans.push('alwaysApply');
if (value !== undefined) {
flagValues[flag.column] = value;
}
}
return {
name,
description,
alwaysApply,
alwaysApply: flagValues.alwaysApply,
userInvocable: flagValues.userInvocable,
disableModelInvocation: flagValues.disableModelInvocation,
frontmatter,
invalidBooleans,
};
}
/**
* Reduce a parsed frontmatter bag to what a skill document should persist:
* `name`/`description` live in their own columns, and every boolean flag is
* rewritten from the parser's resolved value under its canonical key.
*
* That rewrite is what keeps the bag honest. Values the parser could not read
* as booleans a mid-edit `user-invocable:` placeholder, a
* `disable-model-invocation: yes` typo, a quoted `"true"` never reach the
* document as-is, so the bag both passes strict frontmatter validation and
* agrees with the columns `deriveStructuredFrontmatterFields` derives from it.
* The legacy `alwaysApply` spelling is folded into `always-apply`.
*
* Key order is preserved: a flag is rewritten in place rather than appended,
* so re-cleaning an unchanged SKILL.md is byte-identical for callers that
* detect drift by comparing serialized bags.
*/
export function toCleanFrontmatter(parsed: ParsedSkillMarkdown): Record<string, unknown> {
const clean: Record<string, unknown> = {};
for (const [key, value] of Object.entries(parsed.frontmatter ?? {})) {
if (key === 'name' || key === 'description') {
continue;
}
const flag = SKILL_BOOLEAN_FLAG_BY_KEY.get(key);
if (!flag) {
clean[key] = value;
continue;
}
const resolved = parsed[flag.column];
if (resolved !== undefined && !(flag.key in clean)) {
clean[flag.key] = resolved;
}
}
return clean;
}

View file

@ -308,6 +308,94 @@ describe('createGitHubSkillSyncRunner', () => {
);
});
it('mirrors user-invocable and disable-model-invocation into the synced frontmatter', async () => {
const deps = createDeps({
fetchFn: githubFetch(
'---\nname: research\ndescription: Research things\nuser-invocable: false\ndisable-model-invocation: true\n---\nBody',
),
});
const runner = createGitHubSkillSyncRunner(deps);
const result = await runner.runOnce();
expect(result.status).toBe('completed');
expect(deps.createSkill).toHaveBeenCalledWith(
expect.objectContaining({
frontmatter: { 'user-invocable': false, 'disable-model-invocation': true },
}),
);
});
it('syncs quoted invocation booleans instead of failing strict frontmatter validation', async () => {
const deps = createDeps({
fetchFn: githubFetch(
'---\nname: research\ndescription: Research things\nuser-invocable: "false"\n---\nBody',
),
});
const runner = createGitHubSkillSyncRunner(deps);
const result = await runner.runOnce();
expect(result.status).toBe('completed');
expect(deps.createSkill).toHaveBeenCalledWith(
expect.objectContaining({
frontmatter: { 'user-invocable': false },
}),
);
});
it('drops a mid-edit invocation-flag placeholder rather than failing the source', async () => {
const deps = createDeps({
fetchFn: githubFetch(
'---\nname: research\ndescription: Research things\nuser-invocable:\n---\nBody',
),
});
const runner = createGitHubSkillSyncRunner(deps);
const result = await runner.runOnce();
expect(result.status).toBe('completed');
expect(deps.createSkill).toHaveBeenCalledWith(
expect.objectContaining({
frontmatter: {},
}),
);
});
it('keeps a flag whose YAML value continues on the next line', async () => {
const deps = createDeps({
fetchFn: githubFetch(
'---\nname: research\ndescription: Research things\nalways-apply:\n true\nuser-invocable:\n false\n---\nBody',
),
});
const runner = createGitHubSkillSyncRunner(deps);
const result = await runner.runOnce();
expect(result.status).toBe('completed');
expect(deps.createSkill).toHaveBeenCalledWith(
expect.objectContaining({
alwaysApply: true,
frontmatter: { 'always-apply': true, 'user-invocable': false },
}),
);
});
it('marks a source failed when an invocation flag carries a non-boolean value', async () => {
const deps = createDeps({
fetchFn: githubFetch(
'---\nname: research\ndescription: Research things\ndisable-model-invocation: yes\n---\nBody',
),
});
const runner = createGitHubSkillSyncRunner(deps);
const result = await runner.runOnce();
expect(result.status).toBe('failed');
expect(deps.createSkill).not.toHaveBeenCalled();
expect(deps.upsertStatus).toHaveBeenLastCalledWith(
expect.objectContaining({
status: 'failed',
errorCode: 'SKILL_PARSE_FAILED',
}),
);
});
it('fails duplicate discovered skill names before publishing partial mirrors', async () => {
const duplicateFetch = jest.fn(async (input: RequestInfo | URL) => {
const url = input.toString();

View file

@ -22,8 +22,8 @@ import type {
SkillSyncStatusInput,
} from '@librechat/data-schemas';
import type { SkillSyncConfig, SkillSyncGitHubSourceConfig } from 'librechat-data-provider';
import { parseSkillMarkdown, toCleanFrontmatter } from '../parse';
import { DEFAULT_SKILL_IMPORT_LIMITS } from '../limits';
import { parseSkillMarkdown } from '../parse';
const GITHUB_API_BASE = 'https://api.github.com';
const SYSTEM_AUTHOR_ID = new Types.ObjectId('000000000000000000000000');
@ -331,31 +331,6 @@ function guessMimeType(filename: string): string {
return mimeMap[ext] ?? 'application/octet-stream';
}
function toCleanFrontmatter(
frontmatter: Record<string, unknown> | undefined,
): Record<string, unknown> {
if (!frontmatter) {
return {};
}
const clean = { ...frontmatter };
delete clean.name;
delete clean.description;
// Drop a placeholder/non-boolean always-apply (e.g. `always-apply:` or
// `always-apply: # TODO`, which js-yaml yields as null). Apply the same
// cleanup to the accepted `alwaysApply` alias so a malformed alias does not
// survive after the canonical key has already supplied the effective flag.
// The boolean is already captured in the dedicated alwaysApply field, and
// persisting a null here would leave ambiguous/invalid frontmatter on the
// synced skill.
if ('always-apply' in clean && typeof clean['always-apply'] !== 'boolean') {
delete clean['always-apply'];
}
if ('alwaysApply' in clean && typeof clean.alwaysApply !== 'boolean') {
delete clean.alwaysApply;
}
return clean;
}
function getLimitMegabytes(bytes: number): number {
return Math.round(bytes / 1024 / 1024);
}
@ -845,7 +820,7 @@ async function prepareRemoteSkill(params: {
name: parsed.name || fallbackName,
description: parsed.description || parsed.name || fallbackName,
body: skillMdContent,
frontmatter: toCleanFrontmatter(parsed.frontmatter),
frontmatter: toCleanFrontmatter(parsed),
alwaysApply: parsed.alwaysApply,
source: PROVIDER,
sourceMetadata,

View file

@ -16,10 +16,12 @@ export {
defaultRate,
createTxMethods,
permissionBitSupersets,
SKILL_BOOLEAN_FLAGS,
partitionIssues,
validateSkillName,
validateSkillBody,
validateRelativePath,
pickValidFrontmatter,
inferSkillFileCategory,
validateSkillFrontmatter,
validateSkillDescription,

View file

@ -73,9 +73,11 @@ import { createPromptMethods, type PromptMethods, type PromptDeps } from './prom
import {
createSkillMethods,
partitionIssues,
SKILL_BOOLEAN_FLAGS,
validateSkillName,
validateSkillBody,
validateRelativePath,
pickValidFrontmatter,
validateSkillFrontmatter,
validateSkillDescription,
deriveStructuredFrontmatterFields,
@ -90,6 +92,8 @@ import {
type ListSkillsByAccessResult,
type UpdateSkillResult,
type ValidationIssue,
type SkillBooleanFlag,
type SkillBooleanColumn,
} from './skill';
import { createSkillSyncMethods, type SkillSyncMethods } from './skillSync';
import type {
@ -106,10 +110,12 @@ export { RoleConflictError, DEFAULT_REFRESH_TOKEN_EXPIRY, DEFAULT_SESSION_EXPIRY
export { tokenValues, cacheTokenValues, premiumTokenValues, defaultRate, createTxMethods };
export { permissionBitSupersets };
export {
SKILL_BOOLEAN_FLAGS,
partitionIssues,
validateSkillName,
validateSkillBody,
validateRelativePath,
pickValidFrontmatter,
validateSkillFrontmatter,
validateSkillDescription,
deriveStructuredFrontmatterFields,
@ -338,6 +344,8 @@ export type {
ListSkillsByAccessResult,
UpdateSkillResult,
ValidationIssue,
SkillBooleanFlag,
SkillBooleanColumn,
SkillSyncStatusInput,
SkillSyncCredentialSummary,
UpsertSkillSyncCredentialInput,

View file

@ -14,6 +14,7 @@ import {
validateSkillFrontmatter,
validateAlwaysApply,
validateRelativePath,
pickValidFrontmatter,
inferSkillFileCategory,
filterExistingSkillIds,
deriveStructuredFrontmatterFields,
@ -353,6 +354,71 @@ describe('skill validation helpers', () => {
});
});
describe('pickValidFrontmatter', () => {
it('keeps every entry the strict validator accepts', () => {
const frontmatter = {
'when-to-use': 'When the user needs a demo.',
'allowed-tools': ['read', 'write'],
'user-invocable': false,
'disable-model-invocation': true,
'always-apply': true,
metadata: { owner: 'data-team' },
};
expect(pickValidFrontmatter(frontmatter)).toEqual(frontmatter);
expect(validateSkillFrontmatter(pickValidFrontmatter(frontmatter))).toEqual([]);
});
it('drops unknown keys instead of rejecting the whole bag', () => {
expect(pickValidFrontmatter({ 'user-invocable': false, icon: '🚀', tags: ['a'] })).toEqual({
'user-invocable': false,
});
});
it('drops recognized keys whose value fails its declared kind', () => {
expect(
pickValidFrontmatter({
'user-invocable': false,
version: 1.0,
effort: ['high'],
'allowed-tools': [1, 2],
}),
).toEqual({ 'user-invocable': false });
});
it('drops hooks / metadata that are not plain JSON-safe objects', () => {
expect(pickValidFrontmatter({ hooks: 'echo hi' })).toEqual({});
expect(
pickValidFrontmatter({ metadata: { a: { b: { c: { d: { e: { f: 'deep' } } } } } } }),
).toEqual({});
});
it('returns an empty bag for non-object input', () => {
expect(pickValidFrontmatter(undefined)).toEqual({});
expect(pickValidFrontmatter(null)).toEqual({});
expect(pickValidFrontmatter([])).toEqual({});
expect(pickValidFrontmatter('name: demo')).toEqual({});
});
it('never yields a bag the strict validator would reject', () => {
const messy = {
name: 'demo-skill',
'user-invocable': 'yes',
'disable-model-invocation': null,
unknown: 1,
hooks: [],
license: 'MIT',
};
expect(validateSkillFrontmatter(pickValidFrontmatter(messy))).toEqual([]);
expect(pickValidFrontmatter(messy)).toEqual({ name: 'demo-skill', license: 'MIT' });
});
it('preserves key order so serialized-bag drift checks stay stable', () => {
expect(
Object.keys(pickValidFrontmatter({ license: 'MIT', icon: 'x', 'user-invocable': true })),
).toEqual(['license', 'user-invocable']);
});
});
describe('validateAlwaysApply', () => {
it('accepts undefined and booleans (undefined = no change)', () => {
expect(validateAlwaysApply(undefined)).toEqual([]);
@ -779,6 +845,186 @@ describe('Skill CRUD methods', () => {
expect(updated.skill.allowedTools).toBeUndefined();
});
/**
* The shipping create/edit forms send `body` with no structured
* `frontmatter`, so the SKILL.md text is the only place a user can declare
* these flags. Before this cascade existed only `always-apply` was read out
* of the body and the other two silently kept their defaults.
*/
describe('invocation-mode columns derived from the SKILL.md body', () => {
const bodyWithFlags = [
'---',
'name: body-flags',
'description: A skill that restricts its invocation channels.',
'always-apply: true',
'user-invocable: false',
'disable-model-invocation: true',
'---',
'Body.',
].join('\n');
it('createSkill honors flags declared only in the body', async () => {
const { skill } = await methods.createSkill(
makeSkillInput({ name: 'body-flags', body: bodyWithFlags, frontmatter: undefined }),
);
expect(skill.alwaysApply).toBe(true);
expect(skill.userInvocable).toBe(false);
expect(skill.disableModelInvocation).toBe(true);
const reloaded = await methods.getSkillById(skill._id);
expect(reloaded?.userInvocable).toBe(false);
expect(reloaded?.disableModelInvocation).toBe(true);
});
it('createSkill lets an explicit frontmatter bag win over the body', async () => {
const { skill } = await methods.createSkill(
makeSkillInput({
name: 'bag-wins',
body: bodyWithFlags,
frontmatter: { 'user-invocable': true },
}),
);
expect(skill.userInvocable).toBe(true);
/* Not declared in the bag, so the body still supplies it. */
expect(skill.disableModelInvocation).toBe(true);
});
it('createSkill rejects a non-boolean flag in the body', async () => {
await expect(
methods.createSkill(
makeSkillInput({
name: 'body-typo',
body: '---\nname: body-typo\ndescription: A demo skill.\nuser-invocable: yes\n---\n\nBody.',
frontmatter: undefined,
}),
),
).rejects.toMatchObject({
code: 'SKILL_VALIDATION_FAILED',
issues: expect.arrayContaining([
expect.objectContaining({ field: 'body.frontmatter.user-invocable' }),
]),
});
});
it('createSkill accepts a body typo the frontmatter bag overrides', async () => {
const { skill } = await methods.createSkill(
makeSkillInput({
name: 'body-typo-overridden',
body: '---\nname: body-typo-overridden\ndescription: A demo skill.\nuser-invocable: yes\n---\n\nBody.',
frontmatter: { 'user-invocable': false },
}),
);
expect(skill.userInvocable).toBe(false);
});
it('createSkill ignores a flag nested under another frontmatter key', async () => {
const { skill } = await methods.createSkill(
makeSkillInput({
name: 'nested-flag',
body: '---\nname: nested-flag\ndescription: A demo skill.\nmetadata:\n user-invocable: nonsense\n---\n\nBody.',
frontmatter: undefined,
}),
);
expect(skill.userInvocable).toBe(true);
});
it('updateSkill picks up flags added to the body', async () => {
const { skill } = await methods.createSkill(makeSkillInput({ name: 'body-add-flags' }));
expect(skill.userInvocable).toBe(true);
const updated = await methods.updateSkill({
id: skill._id.toString(),
expectedVersion: skill.version,
update: {
body: '---\nname: body-add-flags\ndescription: A demo skill.\nuser-invocable: false\n---\n\nBody.',
},
});
expect(updated.status).toBe('updated');
if (updated.status !== 'updated') return;
expect(updated.skill.userInvocable).toBe(false);
});
it('updateSkill releases a restriction when the body drops the flag line', async () => {
/* Regression for the sticky-column trap: an imported skill restricted to
manual-only had no way back the edit UI sends `body` only, so the
column stayed `false` while the SKILL.md text no longer said so. */
const { skill } = await methods.createSkill(
makeSkillInput({ name: 'body-release', body: bodyWithFlags, frontmatter: undefined }),
);
expect(skill.userInvocable).toBe(false);
expect(skill.disableModelInvocation).toBe(true);
const updated = await methods.updateSkill({
id: skill._id.toString(),
expectedVersion: skill.version,
update: {
body: '---\nname: body-release\ndescription: A demo skill.\n---\n\nBody.',
},
});
expect(updated.status).toBe('updated');
if (updated.status !== 'updated') return;
/* Unset, which every runtime gate reads as the schema default. */
expect(updated.skill.userInvocable).toBeUndefined();
expect(updated.skill.disableModelInvocation).toBeUndefined();
});
it('updateSkill leaves the columns alone when the update touches neither body nor frontmatter', async () => {
const { skill } = await methods.createSkill(
makeSkillInput({ name: 'untouched-columns', body: bodyWithFlags, frontmatter: undefined }),
);
const updated = await methods.updateSkill({
id: skill._id.toString(),
expectedVersion: skill.version,
update: { category: 'research' },
});
expect(updated.status).toBe('updated');
if (updated.status !== 'updated') return;
expect(updated.skill.userInvocable).toBe(false);
expect(updated.skill.disableModelInvocation).toBe(true);
});
it('updateSkill keeps allowedTools when only the body changes', async () => {
/* The body scan reads booleans, not YAML sequences, so a body-only update
must not drop a tool list it cannot re-read. */
const { skill } = await methods.createSkill(
makeSkillInput({
name: 'keeps-tools',
frontmatter: { 'allowed-tools': ['execute_code'] },
}),
);
expect(skill.allowedTools).toEqual(['execute_code']);
const updated = await methods.updateSkill({
id: skill._id.toString(),
expectedVersion: skill.version,
update: { body: '---\nname: keeps-tools\ndescription: A demo skill.\n---\n\nBody.' },
});
expect(updated.status).toBe('updated');
if (updated.status !== 'updated') return;
expect(updated.skill.allowedTools).toEqual(['execute_code']);
});
it('updateSkill rejects a non-boolean flag introduced by a body edit', async () => {
const { skill } = await methods.createSkill(makeSkillInput({ name: 'body-edit-typo' }));
await expect(
methods.updateSkill({
id: skill._id.toString(),
expectedVersion: skill.version,
update: {
body: '---\nname: body-edit-typo\ndescription: A demo skill.\ndisable-model-invocation: 1\n---\n\nBody.',
},
}),
).rejects.toMatchObject({
code: 'SKILL_VALIDATION_FAILED',
issues: expect.arrayContaining([
expect.objectContaining({ field: 'body.frontmatter.disable-model-invocation' }),
]),
});
});
});
it('backfills legacy skills from frontmatter when columns are unset (getSkillByName)', async () => {
/* Simulate a pre-Phase-6 skill: it has `user-invocable` /
`disable-model-invocation` / `allowed-tools` set in frontmatter

View file

@ -234,6 +234,33 @@ export function validateAlwaysApply(alwaysApply: unknown): ValidationIssue[] {
return [];
}
/** Column on a skill document that mirrors a boolean frontmatter flag. */
export type SkillBooleanColumn = 'alwaysApply' | 'userInvocable' | 'disableModelInvocation';
export type SkillBooleanFlag = {
/** Column the flag is mirrored onto. */
column: SkillBooleanColumn;
/** Canonical kebab-case frontmatter key. */
key: string;
/**
* Legacy spellings accepted on read and normalized to `key` on write.
* Consulted only when the canonical key is absent.
*/
aliases: readonly string[];
};
/**
* Boolean frontmatter flags mirrored onto first-class columns. Shared with the
* SKILL.md parser in `@librechat/api` so the parser, the body extractor, and
* the column derivation can't disagree about which keys exist or which column
* each one feeds.
*/
export const SKILL_BOOLEAN_FLAGS: readonly SkillBooleanFlag[] = [
{ column: 'alwaysApply', key: 'always-apply', aliases: ['alwaysApply'] },
{ column: 'userInvocable', key: 'user-invocable', aliases: [] },
{ column: 'disableModelInvocation', key: 'disable-model-invocation', aliases: [] },
];
/**
* Known fields allowed inside a skill's YAML frontmatter. Anything else is
* rejected in strict mode. The list is derived from Anthropic's Agent Skills
@ -343,6 +370,48 @@ function isJsonSafe(value: unknown, depth: number): boolean {
return false;
}
/**
* Evaluate one frontmatter entry against the strict allowlist and its declared
* kind, returning `null` when the entry is acceptable. Shared by
* `validateSkillFrontmatter` (which reports issues) and
* `pickValidFrontmatter` (which drops them) so the two can never disagree on
* what a valid entry is.
*/
function checkFrontmatterEntry(key: string, value: unknown): ValidationIssue | null {
if (!ALLOWED_FRONTMATTER_KEYS.has(key)) {
return {
field: `frontmatter.${key}`,
code: 'UNKNOWN_KEY',
message: `"${key}" is not a recognized frontmatter key`,
};
}
if (key === 'hooks' || key === 'metadata') {
if (!isPlainObject(value) || !isJsonSafe(value, 0)) {
return {
field: `frontmatter.${key}`,
code: 'INVALID_SHAPE',
message: `"${key}" must be a plain JSON-safe object (max depth ${FRONTMATTER_MAX_DEPTH}, max string ${FRONTMATTER_MAX_STRING})`,
};
}
return null;
}
const expected = FRONTMATTER_KIND[key];
if (!expected) {
return null;
}
const kinds = Array.isArray(expected) ? expected : [expected];
if (!kinds.some((kind) => matchesKind(value, kind))) {
return {
field: `frontmatter.${key}`,
code: 'INVALID_TYPE',
message: `"${key}" must be ${kinds.join(' or ')}`,
};
}
return null;
}
/**
* Validate a skill's structured YAML frontmatter. Strict mode: unknown keys
* are rejected so any expansion of the allowed set is an intentional code
@ -366,42 +435,44 @@ export function validateSkillFrontmatter(frontmatter: unknown): ValidationIssue[
const issues: ValidationIssue[] = [];
for (const [key, value] of Object.entries(frontmatter)) {
if (!ALLOWED_FRONTMATTER_KEYS.has(key)) {
issues.push({
field: `frontmatter.${key}`,
code: 'UNKNOWN_KEY',
message: `"${key}" is not a recognized frontmatter key`,
});
continue;
}
if (key === 'hooks' || key === 'metadata') {
if (!isPlainObject(value) || !isJsonSafe(value, 0)) {
issues.push({
field: `frontmatter.${key}`,
code: 'INVALID_SHAPE',
message: `"${key}" must be a plain JSON-safe object (max depth ${FRONTMATTER_MAX_DEPTH}, max string ${FRONTMATTER_MAX_STRING})`,
});
}
continue;
}
const expected = FRONTMATTER_KIND[key];
if (!expected) {
continue;
}
const kinds = Array.isArray(expected) ? expected : [expected];
if (!kinds.some((kind) => matchesKind(value, kind))) {
issues.push({
field: `frontmatter.${key}`,
code: 'INVALID_TYPE',
message: `"${key}" must be ${kinds.join(' or ')}`,
});
const issue = checkFrontmatterEntry(key, value);
if (issue) {
issues.push(issue);
}
}
return issues;
}
/**
* Narrow a frontmatter bag to exactly the entries `validateSkillFrontmatter`
* accepts, dropping unknown keys and values that fail their declared kind.
*
* For ingestion paths that accept files authored outside LibreChat (skill
* import), where the bag is a byproduct of the upload rather than something
* the uploader typed: a stray `version: 1.0` or a bespoke `icon:` key must
* not fail an otherwise valid import, and neither may it reach `createSkill`
* and trip strict validation there. Fields whose value is load-bearing for
* behavior (the invocation-mode booleans) are resolved and reported by the
* caller's own parser before this filter runs, so a malformed one still
* surfaces as an error rather than being quietly dropped here.
*
* Admin-authored paths (GitHub sync, deployment skills) deliberately skip
* this and keep the strict validator's hard failure a typo in a curated
* source should be loud.
*/
export function pickValidFrontmatter(frontmatter: unknown): Record<string, unknown> {
if (!isPlainObject(frontmatter)) {
return {};
}
const picked: Record<string, unknown> = {};
for (const [key, value] of Object.entries(frontmatter)) {
if (checkFrontmatterEntry(key, value) === null) {
picked[key] = value;
}
}
return picked;
}
export function validateRelativePath(relativePath: unknown): ValidationIssue[] {
const issues: ValidationIssue[] = [];
if (typeof relativePath !== 'string' || relativePath.length === 0) {
@ -700,97 +771,184 @@ type BodyAlwaysApplyResult =
| { status: 'valid'; value: boolean }
| { status: 'invalid' };
/**
* Extractor for the `always-apply` / `alwaysApply` flag sitting inside a SKILL.md body's
* YAML frontmatter block. The REST edit flow lets users rewrite the full
* SKILL.md text via `update.body` without a structured `frontmatter`
* object, so this is the only signal we have for "user flipped
* `always-apply:` or `alwaysApply:` inline in their editor".
*
* Returns a discriminated union so callers can tell:
* - `absent` no always-apply key (leave column alone; could be
* "user removed the flag" or "user hasn't written it yet" both
* resolve to no-op). An empty value (`always-apply:` with nothing
* after the colon) is also treated as absent to allow mid-edit
* placeholder states without rejecting a save.
* - `valid` parsed cleanly as `true` / `false` (case-insensitive,
* quote-tolerant, YAML inline-comment-tolerant).
* - `invalid` key is present with a non-empty value that isn't a
* recognizable boolean (e.g. `tru`, `yes`, `1`). Validation rejects
* this rather than silently ignoring so `always-apply: tru` typos
* surface as 400s instead of drifting the column from what the
* saved SKILL.md text says. When both forms are present, the canonical
* `always-apply` form wins because existing files may already rely on it.
*/
function extractAlwaysApplyFromBody(body: string | undefined): BodyAlwaysApplyResult {
/** Body-derived state for every boolean flag mirrored onto a column. */
type BodyFlagResults = Record<SkillBooleanColumn, BodyAlwaysApplyResult>;
const BODY_FLAG_BY_KEY = new Map<string, SkillBooleanFlag>(
SKILL_BOOLEAN_FLAGS.flatMap((flag) =>
[flag.key, ...flag.aliases].map((key) => [key.toLowerCase(), flag] as const),
),
);
/** Isolate a SKILL.md body's leading YAML frontmatter block, or `null`. */
function extractBodyFrontmatterBlock(body: string | undefined): string | null {
if (typeof body !== 'string' || body.length === 0) {
return { status: 'absent' };
return null;
}
const trimmed = body.trim();
if (!trimmed.startsWith('---')) {
return { status: 'absent' };
return null;
}
const after = trimmed.slice(3);
const closingIdx = after.indexOf('\n---');
if (closingIdx === -1) {
return null;
}
return after.slice(0, closingIdx);
}
function readBodyFlagValue(rawValue: string): BodyAlwaysApplyResult {
/* Strip the YAML inline comment BEFORE unquoting a line like
`always-apply: "true" # note` has both, and handling whole-line quoting
first would leave `"true"` behind, which parses as invalid. */
let value = stripYamlTrailingComment(rawValue.trim()).trim();
if (value === '') {
return { status: 'absent' };
}
const block = after.slice(0, closingIdx);
let aliasResult: BodyAlwaysApplyResult | undefined;
if (
value.length >= 2 &&
((value[0] === '"' && value[value.length - 1] === '"') ||
(value[0] === "'" && value[value.length - 1] === "'"))
) {
value = value.slice(1, -1).trim();
}
if (value === '') {
return { status: 'absent' };
}
const lowered = value.toLowerCase();
if (lowered === 'true') {
return { status: 'valid', value: true };
}
if (lowered === 'false') {
return { status: 'valid', value: false };
}
return { status: 'invalid' };
}
/**
* Extractor for the boolean invocation-mode flags sitting inside a SKILL.md
* body's YAML frontmatter block. The REST edit flow lets users rewrite the
* full SKILL.md text via `update.body` without a structured `frontmatter`
* object, so this is the only signal we have for "user flipped
* `user-invocable:` inline in their editor".
*
* Each flag resolves to a discriminated union so callers can tell:
* - `absent` key not present (or present with an empty value, a mid-edit
* placeholder that must not reject a save). Treated as a declaration that
* the flag is off, so removing a line returns the column to its default.
* - `valid` parsed cleanly as `true` / `false` (case-insensitive,
* quote-tolerant, YAML inline-comment-tolerant).
* - `invalid` present with a non-empty value that isn't a recognizable
* boolean (`tru`, `yes`, `1`). Validation rejects this rather than silently
* ignoring it, so typos surface as 400s instead of drifting the column away
* from what the saved SKILL.md text says.
*
* The first canonical spelling wins; a legacy alias (`alwaysApply`) is only
* consulted when the canonical key never appears. Indented lines are skipped:
* a nested mapping that reuses a flag name (`metadata:` ` user-invocable:`)
* is not a top-level declaration and must not be read as one.
*/
function extractBooleanFlagsFromBody(body: string | undefined): BodyFlagResults {
const results: BodyFlagResults = {
alwaysApply: { status: 'absent' },
userInvocable: { status: 'absent' },
disableModelInvocation: { status: 'absent' },
};
const block = extractBodyFrontmatterBlock(body);
if (block === null) {
return results;
}
const canonical = new Map<SkillBooleanColumn, BodyAlwaysApplyResult>();
const aliased = new Map<SkillBooleanColumn, BodyAlwaysApplyResult>();
for (const line of block.split('\n')) {
if (line.length === 0 || line[0] === ' ' || line[0] === '\t') {
continue;
}
const colon = line.indexOf(':');
if (colon === -1) {
continue;
}
const key = line.slice(0, colon).trim();
const normalizedKey = key.toLowerCase();
if (normalizedKey !== 'always-apply' && normalizedKey !== 'alwaysapply') {
const key = line.slice(0, colon).trim().toLowerCase();
const flag = BODY_FLAG_BY_KEY.get(key);
if (!flag) {
continue;
}
// Strip the YAML inline comment BEFORE unquoting — a line like
// `always-apply: "true" # note` has both, and if we only handled
// whole-line quoting first, the quoted branch wouldn't match and
// the comment-strip would leave `"true"` which parses as invalid.
let value = stripYamlTrailingComment(line.slice(colon + 1).trim()).trim();
if (value === '') {
const result: BodyAlwaysApplyResult = { status: 'absent' };
if (normalizedKey === 'always-apply') {
return result;
const result = readBodyFlagValue(line.slice(colon + 1));
if (key === flag.key) {
if (!canonical.has(flag.column)) {
canonical.set(flag.column, result);
}
aliasResult = result;
continue;
}
if (
value.length >= 2 &&
((value[0] === '"' && value[value.length - 1] === '"') ||
(value[0] === "'" && value[value.length - 1] === "'"))
) {
value = value.slice(1, -1);
}
value = value.trim();
if (value === '') {
const result: BodyAlwaysApplyResult = { status: 'absent' };
if (normalizedKey === 'always-apply') {
return result;
}
aliasResult = result;
continue;
}
const lowered = value.toLowerCase();
let result: BodyAlwaysApplyResult;
if (lowered === 'true') {
result = { status: 'valid', value: true };
} else if (lowered === 'false') {
result = { status: 'valid', value: false };
} else {
result = { status: 'invalid' };
}
if (normalizedKey === 'always-apply') {
return result;
}
aliasResult = result;
aliased.set(flag.column, result);
}
return aliasResult ?? { status: 'absent' };
for (const flag of SKILL_BOOLEAN_FLAGS) {
const resolved = canonical.get(flag.column) ?? aliased.get(flag.column);
if (resolved) {
results[flag.column] = resolved;
}
}
return results;
}
function extractAlwaysApplyFromBody(body: string | undefined): BodyAlwaysApplyResult {
return extractBooleanFlagsFromBody(body).alwaysApply;
}
/**
* Columns whose only inputs are the frontmatter bag and the body's own
* frontmatter. `alwaysApply` is excluded: it additionally accepts an explicit
* top-level input and is a non-nullable column, so it runs its own cascade.
*/
const BODY_DERIVED_COLUMNS = ['userInvocable', 'disableModelInvocation'] as const;
/**
* Resolve one boolean column from the two sources that can carry it, in
* precedence order: an explicit key in the structured `frontmatter` bag, then
* the SKILL.md body's own frontmatter. `undefined` means "declared nowhere",
* which callers turn into the schema default.
*/
function resolveBodyDerivedColumn(
column: (typeof BODY_DERIVED_COLUMNS)[number],
bagDerived: { userInvocable?: boolean; disableModelInvocation?: boolean } | undefined,
bodyFlags: BodyFlagResults | undefined,
): boolean | undefined {
const fromBag = bagDerived?.[column];
if (typeof fromBag === 'boolean') {
return fromBag;
}
const fromBody = bodyFlags?.[column];
return fromBody?.status === 'valid' ? fromBody.value : undefined;
}
/**
* Report body-declared flags whose value isn't a boolean, skipping any the
* caller is already overriding through the structured `frontmatter` bag.
*/
function validateBodyDerivedColumns(
frontmatter: Record<string, unknown> | undefined,
bodyFlags: BodyFlagResults | undefined,
): ValidationIssue[] {
if (!bodyFlags) {
return [];
}
const bagDerived = deriveStructuredFrontmatterFields(frontmatter);
const issues: ValidationIssue[] = [];
for (const flag of SKILL_BOOLEAN_FLAGS) {
if (flag.column === 'alwaysApply') {
continue;
}
const column = flag.column as (typeof BODY_DERIVED_COLUMNS)[number];
if (bodyFlags[column].status !== 'invalid' || typeof bagDerived[column] === 'boolean') {
continue;
}
issues.push({
field: `body.frontmatter.${flag.key}`,
code: 'INVALID_TYPE',
message: `"${flag.key}" in SKILL.md frontmatter must be a boolean (true or false)`,
});
}
return issues;
}
/**
@ -1024,11 +1182,11 @@ export function createSkillMethods(
}
async function createSkill(data: CreateSkillInput): Promise<CreateSkillResult> {
/* Parse body's always-apply status once reused for validation
(below) and derivation in `resolveAlwaysApplyFromInput`. Avoids
parsing the same YAML frontmatter block twice per create. */
const bodyAlwaysApply =
data.body !== undefined ? extractAlwaysApplyFromBody(data.body) : undefined;
/* Parse the body's flag declarations once reused for validation (below)
and for the derivation cascades. Avoids parsing the same YAML
frontmatter block twice per create. */
const bodyFlags = data.body !== undefined ? extractBooleanFlagsFromBody(data.body) : undefined;
const bodyAlwaysApply = bodyFlags?.alwaysApply;
const issues: ValidationIssue[] = [
...validateSkillName(data.name),
...validateSkillDescription(data.description),
@ -1036,6 +1194,7 @@ export function createSkillMethods(
...validateSkillDisplayTitle(data.displayTitle),
...validateSkillFrontmatter(data.frontmatter),
...validateAlwaysApply(data.alwaysApply),
...validateBodyDerivedColumns(data.frontmatter, bodyFlags),
];
/* Body-level `always-apply:` only needs to be well-formed when a
higher-precedence source won't override it (see
@ -1084,6 +1243,20 @@ export function createSkillMethods(
}
const derived = deriveStructuredFrontmatterFields(data.frontmatter);
/**
* A caller may declare the invocation-mode flags in the structured bag, in
* the SKILL.md body's own frontmatter, or both — the UI's create form sends
* only `body`. Resolve each column from whichever source carries it so a
* flag written inline is honored the same way `always-apply` already is.
* Keys the bag declares still win, so `derived` is spread last.
*/
const bodyDerived: { userInvocable?: boolean; disableModelInvocation?: boolean } = {};
for (const column of BODY_DERIVED_COLUMNS) {
const resolved = resolveBodyDerivedColumn(column, derived, bodyFlags);
if (resolved !== undefined) {
bodyDerived[column] = resolved;
}
}
const doc = await Skill.create({
name: data.name,
displayTitle: data.displayTitle,
@ -1105,6 +1278,7 @@ export function createSkillMethods(
bodyAlwaysApply,
),
tenantId: data.tenantId,
...bodyDerived,
...derived,
});
return {
@ -1346,12 +1520,12 @@ export function createSkillMethods(
return { status: 'not_found' };
}
/* Parse body's always-apply status once reused for validation
(precedence-aware, below) and the derivation cascade further
down. Avoids parsing the same YAML frontmatter block twice per
update. */
const bodyAlwaysApply =
update.body !== undefined ? extractAlwaysApplyFromBody(update.body) : undefined;
/* Parse the body's flag declarations once reused for validation
(precedence-aware, below) and the derivation cascades further down.
Avoids parsing the same YAML frontmatter block twice per update. */
const bodyFlags =
update.body !== undefined ? extractBooleanFlagsFromBody(update.body) : undefined;
const bodyAlwaysApply = bodyFlags?.alwaysApply;
const issues: ValidationIssue[] = [];
if (update.name !== undefined) issues.push(...validateSkillName(update.name));
if (update.description !== undefined)
@ -1362,6 +1536,7 @@ export function createSkillMethods(
if (update.frontmatter !== undefined)
issues.push(...validateSkillFrontmatter(update.frontmatter));
if (update.alwaysApply !== undefined) issues.push(...validateAlwaysApply(update.alwaysApply));
issues.push(...validateBodyDerivedColumns(update.frontmatter, bodyFlags));
/* Body-level `always-apply:` only needs to be well-formed when a
higher-precedence source won't override it (see
`resolveAlwaysApplyFromInput` for precedence). Rejecting a typo
@ -1397,21 +1572,41 @@ export function createSkillMethods(
if (update.body !== undefined) setPayload.body = update.body;
if (update.source !== undefined) setPayload.source = update.source;
if (update.sourceMetadata !== undefined) setPayload.sourceMetadata = update.sourceMetadata;
const bagDerived =
update.frontmatter !== undefined
? deriveStructuredFrontmatterFields(update.frontmatter)
: undefined;
if (update.frontmatter !== undefined) {
setPayload.frontmatter = update.frontmatter;
/**
* Derived columns track frontmatter when frontmatter changes, the
* derived view must follow. Fields the new frontmatter omits are
* unset (back to schema default) so removing `disable-model-invocation`
* from a SKILL.md re-enables model invocation on the next save.
* `allowedTools` tracks the frontmatter bag alone the body scan reads
* boolean flags, not YAML sequences so a bag that omits `allowed-tools`
* unsets the column, while a body-only update leaves it untouched rather
* than dropping a list it cannot re-read.
*/
const derived = deriveStructuredFrontmatterFields(update.frontmatter);
for (const key of ['disableModelInvocation', 'userInvocable', 'allowedTools'] as const) {
if (derived[key] !== undefined) {
setPayload[key] = derived[key];
} else {
unsetPayload[key] = '';
}
if (bagDerived?.allowedTools !== undefined) {
setPayload.allowedTools = bagDerived.allowedTools;
} else {
unsetPayload.allowedTools = '';
}
}
/**
* Boolean invocation-mode columns follow whichever source the update
* carries: a key in the structured bag wins, then the SKILL.md body's own
* frontmatter (the only signal the UI edit flow sends). When neither
* declares the flag but the update did supply one of those sources, the
* column is unset back to its schema default removing
* `disable-model-invocation:` from a SKILL.md re-enables model invocation,
* mirroring how a removed `always-apply:` line stops auto-priming. Updates
* touching neither `frontmatter` nor `body` leave the columns alone.
*/
const declaresColumns = update.frontmatter !== undefined || update.body !== undefined;
for (const column of BODY_DERIVED_COLUMNS) {
const resolved = resolveBodyDerivedColumn(column, bagDerived, bodyFlags);
if (resolved !== undefined) {
setPayload[column] = resolved;
} else if (declaresColumns) {
unsetPayload[column] = '';
}
}
if (update.category !== undefined) setPayload.category = update.category;