diff --git a/api/server/controllers/UserController.js b/api/server/controllers/UserController.js index 16b68968d9..939a637563 100644 --- a/api/server/controllers/UserController.js +++ b/api/server/controllers/UserController.js @@ -333,6 +333,7 @@ const deleteUserController = async (req, res) => { await db.deleteConversationTags({ user: user.id }); await db.deleteAllUserMemories(user.id); await db.deleteUserPrompts(user.id); + await db.deleteUserSkills(user.id); await deleteUserMcpServers(user.id); await db.deleteActions({ user: user.id }); await db.deleteTokens({ userId: user.id }); diff --git a/api/server/controllers/UserController.spec.js b/api/server/controllers/UserController.spec.js index 4a96072062..30e6190e28 100644 --- a/api/server/controllers/UserController.spec.js +++ b/api/server/controllers/UserController.spec.js @@ -28,6 +28,7 @@ jest.mock('~/models', () => { deleteAssistants: jest.fn().mockResolvedValue(undefined), deleteUserById: jest.fn().mockResolvedValue(undefined), deleteUserPrompts: jest.fn().mockResolvedValue(undefined), + deleteUserSkills: jest.fn().mockResolvedValue(undefined), deleteMessages: jest.fn().mockResolvedValue(undefined), deleteBalances: jest.fn().mockResolvedValue(undefined), deleteActions: jest.fn().mockResolvedValue(undefined), diff --git a/api/server/controllers/__tests__/deleteUser.spec.js b/api/server/controllers/__tests__/deleteUser.spec.js index a382a6cdc7..bc6acde53d 100644 --- a/api/server/controllers/__tests__/deleteUser.spec.js +++ b/api/server/controllers/__tests__/deleteUser.spec.js @@ -17,6 +17,7 @@ const mockProcessDeleteRequest = jest.fn(); const mockDeleteToolCalls = jest.fn(); const mockDeleteUserAgents = jest.fn(); const mockDeleteUserPrompts = jest.fn(); +const mockDeleteUserSkills = jest.fn(); jest.mock('@librechat/data-schemas', () => ({ logger: { error: jest.fn(), info: jest.fn() }, @@ -56,6 +57,7 @@ jest.mock('~/models', () => ({ deleteToolCalls: (...args) => mockDeleteToolCalls(...args), deleteUserAgents: (...args) => mockDeleteUserAgents(...args), deleteUserPrompts: (...args) => mockDeleteUserPrompts(...args), + deleteUserSkills: (...args) => mockDeleteUserSkills(...args), deleteTransactions: jest.fn(), deleteBalances: jest.fn(), deleteAllAgentApiKeys: jest.fn(), @@ -130,6 +132,7 @@ function stubDeletionMocks() { mockDeleteToolCalls.mockResolvedValue(); mockDeleteUserAgents.mockResolvedValue(); mockDeleteUserPrompts.mockResolvedValue(); + mockDeleteUserSkills.mockResolvedValue(0); } beforeEach(() => { @@ -148,6 +151,9 @@ describe('deleteUserController - 2FA enforcement', () => { expect(res.status).toHaveBeenCalledWith(200); expect(res.send).toHaveBeenCalledWith({ message: 'User deleted' }); expect(mockDeleteMessages).toHaveBeenCalled(); + expect(mockDeleteUserAgents).toHaveBeenCalledWith('user1'); + expect(mockDeleteUserPrompts).toHaveBeenCalledWith('user1'); + expect(mockDeleteUserSkills).toHaveBeenCalledWith('user1'); expect(mockVerifyOTPOrBackupCode).not.toHaveBeenCalled(); }); diff --git a/api/server/controllers/__tests__/deleteUserResourceCoverage.spec.js b/api/server/controllers/__tests__/deleteUserResourceCoverage.spec.js index b08e502800..78fcfa16b0 100644 --- a/api/server/controllers/__tests__/deleteUserResourceCoverage.spec.js +++ b/api/server/controllers/__tests__/deleteUserResourceCoverage.spec.js @@ -15,6 +15,7 @@ const HANDLED_RESOURCE_TYPES = { [ResourceType.REMOTE_AGENT]: 'deleteUserAgents', [ResourceType.PROMPTGROUP]: 'deleteUserPrompts', [ResourceType.MCPSERVER]: 'deleteUserMcpServers', + [ResourceType.SKILL]: 'deleteUserSkills', }; /** diff --git a/api/server/experimental.js b/api/server/experimental.js index 3dfd5aee3f..cd594c1696 100644 --- a/api/server/experimental.js +++ b/api/server/experimental.js @@ -310,6 +310,7 @@ if (cluster.isMaster) { app.use('/api/convos', routes.convos); app.use('/api/presets', routes.presets); app.use('/api/prompts', routes.prompts); + app.use('/api/skills', routes.skills); app.use('/api/categories', routes.categories); app.use('/api/endpoints', routes.endpoints); app.use('/api/balance', routes.balance); diff --git a/api/server/index.js b/api/server/index.js index adeaacdfcc..d798f1a166 100644 --- a/api/server/index.js +++ b/api/server/index.js @@ -168,6 +168,7 @@ const startServer = async () => { app.use('/api/convos', routes.convos); app.use('/api/presets', routes.presets); app.use('/api/prompts', routes.prompts); + app.use('/api/skills', routes.skills); app.use('/api/categories', routes.categories); app.use('/api/endpoints', routes.endpoints); app.use('/api/balance', routes.balance); diff --git a/api/server/middleware/accessResources/canAccessSkillResource.js b/api/server/middleware/accessResources/canAccessSkillResource.js new file mode 100644 index 0000000000..1010ca8998 --- /dev/null +++ b/api/server/middleware/accessResources/canAccessSkillResource.js @@ -0,0 +1,32 @@ +const { ResourceType } = require('librechat-data-provider'); +const { canAccessResource } = require('./canAccessResource'); +const { getSkillById } = require('~/models'); + +/** + * Skill-specific middleware factory that checks skill access permissions. + * Wraps the generic `canAccessResource` with the SKILL resource type and + * `getSkillById` as the ID resolver. + * + * @param {Object} options + * @param {number} options.requiredPermission - Permission bit required (1=view, 2=edit, 4=delete, 8=share) + * @param {string} [options.resourceIdParam='id'] - Route parameter name holding the skill id + * @returns {Function} Express middleware + */ +const canAccessSkillResource = (options) => { + const { requiredPermission, resourceIdParam = 'id' } = options || {}; + + if (!requiredPermission || typeof requiredPermission !== 'number') { + throw new Error('canAccessSkillResource: requiredPermission is required and must be a number'); + } + + return canAccessResource({ + resourceType: ResourceType.SKILL, + requiredPermission, + resourceIdParam, + idResolver: getSkillById, + }); +}; + +module.exports = { + canAccessSkillResource, +}; diff --git a/api/server/middleware/accessResources/index.js b/api/server/middleware/accessResources/index.js index 838834919a..31548411ba 100644 --- a/api/server/middleware/accessResources/index.js +++ b/api/server/middleware/accessResources/index.js @@ -4,6 +4,7 @@ const { canAccessAgentFromBody } = require('./canAccessAgentFromBody'); const { canAccessPromptViaGroup } = require('./canAccessPromptViaGroup'); const { canAccessPromptGroupResource } = require('./canAccessPromptGroupResource'); const { canAccessMCPServerResource } = require('./canAccessMCPServerResource'); +const { canAccessSkillResource } = require('./canAccessSkillResource'); module.exports = { canAccessResource, @@ -12,4 +13,5 @@ module.exports = { canAccessPromptViaGroup, canAccessPromptGroupResource, canAccessMCPServerResource, + canAccessSkillResource, }; diff --git a/api/server/middleware/checkSharePublicAccess.js b/api/server/middleware/checkSharePublicAccess.js index c7b65a077e..945d5521b7 100644 --- a/api/server/middleware/checkSharePublicAccess.js +++ b/api/server/middleware/checkSharePublicAccess.js @@ -10,6 +10,7 @@ const resourceToPermissionType = { [ResourceType.PROMPTGROUP]: PermissionTypes.PROMPTS, [ResourceType.MCPSERVER]: PermissionTypes.MCP_SERVERS, [ResourceType.REMOTE_AGENT]: PermissionTypes.REMOTE_AGENTS, + [ResourceType.SKILL]: PermissionTypes.SKILLS, }; /** diff --git a/api/server/routes/accessPermissions.js b/api/server/routes/accessPermissions.js index 45afec133b..01e3196b8f 100644 --- a/api/server/routes/accessPermissions.js +++ b/api/server/routes/accessPermissions.js @@ -11,7 +11,7 @@ const { const { requireJwtAuth, checkBan, uaParser, canAccessResource } = require('~/server/middleware'); const { checkPeoplePickerAccess } = require('~/server/middleware/checkPeoplePickerAccess'); const { checkSharePublicAccess } = require('~/server/middleware/checkSharePublicAccess'); -const { findMCPServerByObjectId } = require('~/models'); +const { findMCPServerByObjectId, getSkillById } = require('~/models'); const router = express.Router(); @@ -72,6 +72,13 @@ const checkResourcePermissionAccess = (requiredPermission) => (req, res, next) = resourceIdParam: 'resourceId', idResolver: findMCPServerByObjectId, }); + } else if (resourceType === ResourceType.SKILL) { + middleware = canAccessResource({ + resourceType: ResourceType.SKILL, + requiredPermission, + resourceIdParam: 'resourceId', + idResolver: getSkillById, + }); } else { return res.status(400).json({ error: 'Bad Request', diff --git a/api/server/routes/index.js b/api/server/routes/index.js index 1feaf63fdb..cab2f92ed1 100644 --- a/api/server/routes/index.js +++ b/api/server/routes/index.js @@ -13,6 +13,7 @@ const messages = require('./messages'); const memories = require('./memories'); const presets = require('./presets'); const prompts = require('./prompts'); +const skills = require('./skills'); const balance = require('./balance'); const actions = require('./actions'); const apiKeys = require('./apiKeys'); @@ -56,6 +57,7 @@ module.exports = { config, models, prompts, + skills, actions, presets, balance, diff --git a/api/server/routes/skills.js b/api/server/routes/skills.js new file mode 100644 index 0000000000..38d4b38ca5 --- /dev/null +++ b/api/server/routes/skills.js @@ -0,0 +1,102 @@ +const express = require('express'); +const { createSkillsHandlers, generateCheckAccess } = require('@librechat/api'); +const { isValidObjectIdString } = require('@librechat/data-schemas'); +const { PermissionBits, PermissionTypes, Permissions } = require('librechat-data-provider'); +const { + createSkill, + getSkillById, + listSkillsByAccess, + updateSkill, + deleteSkill, + listSkillFiles, + deleteSkillFile, + getRoleByName, +} = require('~/models'); +const { requireJwtAuth, canAccessSkillResource } = require('~/server/middleware'); +const { + findAccessibleResources, + findPubliclyAccessibleResources, + grantPermission, +} = require('~/server/services/PermissionService'); + +const router = express.Router(); + +// Role-based capability gates. Mirrors prompts.js — the ACL middleware on each +// route handles per-resource permissions; these check that the caller's role +// is even allowed to use / create skills at all. +const checkSkillAccess = generateCheckAccess({ + permissionType: PermissionTypes.SKILLS, + permissions: [Permissions.USE], + getRoleByName, +}); +const checkSkillCreate = generateCheckAccess({ + permissionType: PermissionTypes.SKILLS, + permissions: [Permissions.USE, Permissions.CREATE], + getRoleByName, +}); + +router.use(requireJwtAuth); +router.use(checkSkillAccess); + +const handlers = createSkillsHandlers({ + createSkill, + getSkillById, + listSkillsByAccess, + updateSkill, + deleteSkill, + listSkillFiles, + deleteSkillFile, + findAccessibleResources, + findPubliclyAccessibleResources, + grantPermission, + isValidObjectIdString, +}); + +router.get('/', handlers.list); +router.post('/', checkSkillCreate, handlers.create); + +router.get( + '/:id', + canAccessSkillResource({ requiredPermission: PermissionBits.VIEW }), + handlers.get, +); + +router.patch( + '/:id', + checkSkillCreate, + canAccessSkillResource({ requiredPermission: PermissionBits.EDIT }), + handlers.patch, +); + +router.delete( + '/:id', + checkSkillCreate, + canAccessSkillResource({ requiredPermission: PermissionBits.DELETE }), + handlers.delete, +); + +router.get( + '/:id/files', + canAccessSkillResource({ requiredPermission: PermissionBits.VIEW }), + handlers.listFiles, +); + +router.post( + '/:id/files', + canAccessSkillResource({ requiredPermission: PermissionBits.EDIT }), + handlers.uploadFileStub, +); + +router.get( + '/:id/files/:relativePath', + canAccessSkillResource({ requiredPermission: PermissionBits.VIEW }), + handlers.downloadFileStub, +); + +router.delete( + '/:id/files/:relativePath', + canAccessSkillResource({ requiredPermission: PermissionBits.EDIT }), + handlers.deleteFile, +); + +module.exports = router; diff --git a/api/server/routes/skills.test.js b/api/server/routes/skills.test.js new file mode 100644 index 0000000000..c4b0c6b8c9 --- /dev/null +++ b/api/server/routes/skills.test.js @@ -0,0 +1,480 @@ +const express = require('express'); +const request = require('supertest'); +const mongoose = require('mongoose'); +const { MongoMemoryServer } = require('mongodb-memory-server'); +const { + SystemRoles, + ResourceType, + AccessRoleIds, + PrincipalType, + PermissionBits, +} = require('librechat-data-provider'); + +jest.mock('~/server/services/Config', () => ({ + getCachedTools: jest.fn().mockResolvedValue({}), +})); + +jest.mock('~/models', () => { + const mongoose = require('mongoose'); + const { createMethods } = require('@librechat/data-schemas'); + const methods = createMethods(mongoose, { + removeAllPermissions: async ({ resourceType, resourceId }) => { + const AclEntry = mongoose.models.AclEntry; + if (AclEntry) { + await AclEntry.deleteMany({ resourceType, resourceId }); + } + }, + }); + // Override getRoleByName to return a permissive SKILLS capability block for all + // test users. The real role seeding relies on `initializeRoles` which this + // suite intentionally skips to keep setup minimal. + return { + ...methods, + getRoleByName: jest.fn(), + }; +}); + +jest.mock('~/server/middleware', () => ({ + requireJwtAuth: (req, res, next) => next(), + canAccessSkillResource: jest.requireActual('~/server/middleware').canAccessSkillResource, +})); + +let app; +let mongoServer; +let skillRoutes; +let Skill; +let SkillFile; +let AclEntry; +let AccessRole; +let User; +let testUsers; +let testRoles; +let grantPermission; +let currentTestUser; + +function setTestUser(user) { + currentTestUser = user; +} + +beforeAll(async () => { + mongoServer = await MongoMemoryServer.create(); + await mongoose.connect(mongoServer.getUri()); + + const dbModels = require('~/db/models'); + Skill = dbModels.Skill; + SkillFile = dbModels.SkillFile; + AclEntry = dbModels.AclEntry; + AccessRole = dbModels.AccessRole; + User = dbModels.User; + + const permissionService = require('~/server/services/PermissionService'); + grantPermission = permissionService.grantPermission; + + await setupTestData(); + + app = express(); + app.use(express.json()); + app.use((req, res, next) => { + if (currentTestUser) { + req.user = { + ...(currentTestUser.toObject ? currentTestUser.toObject() : currentTestUser), + id: currentTestUser._id.toString(), + _id: currentTestUser._id, + name: currentTestUser.name, + role: currentTestUser.role, + }; + } + next(); + }); + + currentTestUser = testUsers.owner; + skillRoutes = require('./skills'); + app.use('/api/skills', skillRoutes); +}); + +afterEach(async () => { + await Skill.deleteMany({}); + await SkillFile.deleteMany({}); + await AclEntry.deleteMany({}); + currentTestUser = testUsers.owner; +}); + +afterAll(async () => { + await mongoose.disconnect(); + await mongoServer.stop(); + jest.clearAllMocks(); +}); + +async function setupTestData() { + testRoles = { + viewer: await AccessRole.create({ + accessRoleId: AccessRoleIds.SKILL_VIEWER, + name: 'Viewer', + resourceType: ResourceType.SKILL, + permBits: PermissionBits.VIEW, + }), + editor: await AccessRole.create({ + accessRoleId: AccessRoleIds.SKILL_EDITOR, + name: 'Editor', + resourceType: ResourceType.SKILL, + permBits: PermissionBits.VIEW | PermissionBits.EDIT, + }), + owner: await AccessRole.create({ + accessRoleId: AccessRoleIds.SKILL_OWNER, + name: 'Owner', + resourceType: ResourceType.SKILL, + permBits: + PermissionBits.VIEW | PermissionBits.EDIT | PermissionBits.DELETE | PermissionBits.SHARE, + }), + }; + + testUsers = { + owner: await User.create({ + name: 'Skill Owner', + email: 'skill-owner@test.com', + role: SystemRoles.USER, + }), + editor: await User.create({ + name: 'Skill Editor', + email: 'skill-editor@test.com', + role: SystemRoles.USER, + }), + noAccess: await User.create({ + name: 'No Access', + email: 'no-access@test.com', + role: SystemRoles.USER, + }), + }; + + const { getRoleByName } = require('~/models'); + getRoleByName.mockImplementation((roleName) => { + if (roleName === SystemRoles.USER || roleName === SystemRoles.ADMIN) { + return { + permissions: { + SKILLS: { + USE: true, + CREATE: true, + SHARE: true, + SHARE_PUBLIC: true, + }, + }, + }; + } + return null; + }); +} + +async function createSkillAsOwner(overrides = {}) { + // Description is deliberately kept above the 20-char short-description + // warning threshold so existing tests don't trip the coaching warning. + const res = await request(app) + .post('/api/skills') + .send({ + name: 'demo-skill', + description: 'A small demo skill used in routing integration tests.', + body: '# Demo', + ...overrides, + }); + return res; +} + +describe('Skill routes', () => { + let errSpy; + beforeEach(() => { + errSpy = jest.spyOn(console, 'error').mockImplementation(); + }); + afterEach(() => errSpy.mockRestore()); + + describe('POST /api/skills', () => { + it('creates a skill and grants SKILL_OWNER ACL', async () => { + const res = await createSkillAsOwner(); + expect(res.status).toBe(201); + expect(res.body._id).toBeDefined(); + expect(res.body.version).toBe(1); + expect(res.body.name).toBe('demo-skill'); + // No warnings on a description that comfortably clears the threshold. + expect(res.body.warnings).toBeUndefined(); + + const acl = await AclEntry.findOne({ + resourceType: ResourceType.SKILL, + resourceId: res.body._id, + principalType: PrincipalType.USER, + principalId: testUsers.owner._id, + }); + expect(acl).toBeTruthy(); + expect(acl.roleId.toString()).toBe(testRoles.owner._id.toString()); + }); + + it('attaches a TOO_SHORT warning on create when description is under 20 chars', async () => { + const res = await createSkillAsOwner({ + name: 'short-desc-skill', + description: 'Too short.', + }); + expect(res.status).toBe(201); + expect(res.body._id).toBeDefined(); + expect(Array.isArray(res.body.warnings)).toBe(true); + expect(res.body.warnings).toEqual([ + expect.objectContaining({ + field: 'description', + code: 'TOO_SHORT', + severity: 'warning', + }), + ]); + }); + + it('rejects names starting with reserved brand prefixes', async () => { + const anthropic = await createSkillAsOwner({ name: 'anthropic-helper' }); + expect(anthropic.status).toBe(400); + const claude = await createSkillAsOwner({ name: 'claude-helper' }); + expect(claude.status).toBe(400); + }); + + it('allows names that merely contain reserved brand words as substrings', async () => { + const res = await createSkillAsOwner({ name: 'research-anthropic-helper' }); + expect(res.status).toBe(201); + }); + + it('rejects reserved CLI command names', async () => { + const res = await createSkillAsOwner({ name: 'settings' }); + expect(res.status).toBe(400); + }); + + it('rejects frontmatter with unknown keys', async () => { + const res = await createSkillAsOwner({ + name: 'bad-frontmatter-skill', + frontmatter: { 'not-a-real-key': 'value' }, + }); + expect(res.status).toBe(400); + expect(res.body.issues).toEqual( + expect.arrayContaining([expect.objectContaining({ code: 'UNKNOWN_KEY' })]), + ); + }); + + it('rejects missing description with 400', async () => { + const res = await request(app).post('/api/skills').send({ name: 'x-skill', body: '' }); + expect(res.status).toBe(400); + }); + + it('rejects invalid name with 400 validation failure', async () => { + const res = await createSkillAsOwner({ name: 'BAD NAME' }); + expect(res.status).toBe(400); + expect(res.body.issues).toBeDefined(); + }); + + it('rejects duplicate names with 409', async () => { + const a = await createSkillAsOwner(); + expect(a.status).toBe(201); + const b = await createSkillAsOwner(); + expect(b.status).toBe(409); + }); + }); + + describe('GET /api/skills', () => { + it('returns only skills the caller can access', async () => { + const mine = await createSkillAsOwner({ name: 'mine-skill' }); + expect(mine.status).toBe(201); + + setTestUser(testUsers.noAccess); + const other = await createSkillAsOwner({ name: 'other-skill' }); + expect(other.status).toBe(201); + // Note: the user middleware grants owner perms to whichever user created, so both + // users see their own skill only. + + setTestUser(testUsers.owner); + const res = await request(app).get('/api/skills'); + expect(res.status).toBe(200); + expect(res.body.skills.length).toBe(1); + expect(res.body.skills[0].name).toBe('mine-skill'); + }); + }); + + describe('GET /api/skills/:id', () => { + it('returns 403 when the user has no access', async () => { + const created = await createSkillAsOwner(); + expect(created.status).toBe(201); + setTestUser(testUsers.noAccess); + const res = await request(app).get(`/api/skills/${created.body._id}`); + expect(res.status).toBe(403); + }); + + it('returns the skill to the owner with isPublic flag', async () => { + const created = await createSkillAsOwner(); + const res = await request(app).get(`/api/skills/${created.body._id}`); + expect(res.status).toBe(200); + expect(res.body.name).toBe('demo-skill'); + expect(res.body.isPublic).toBe(false); + }); + }); + + describe('PATCH /api/skills/:id (optimistic concurrency)', () => { + it('updates with correct expectedVersion and bumps version', async () => { + const created = await createSkillAsOwner(); + const res = await request(app) + .patch(`/api/skills/${created.body._id}`) + .send({ expectedVersion: 1, description: 'Updated description' }); + expect(res.status).toBe(200); + expect(res.body.version).toBe(2); + expect(res.body.description).toBe('Updated description'); + }); + + it('returns 409 on stale expectedVersion', async () => { + const created = await createSkillAsOwner(); + const first = await request(app) + .patch(`/api/skills/${created.body._id}`) + .send({ expectedVersion: 1, description: 'First' }); + expect(first.status).toBe(200); + + const stale = await request(app) + .patch(`/api/skills/${created.body._id}`) + .send({ expectedVersion: 1, description: 'Stale' }); + expect(stale.status).toBe(409); + expect(stale.body.error).toBe('skill_version_conflict'); + expect(stale.body.current.version).toBe(2); + }); + + it('rejects updates without expectedVersion', async () => { + const created = await createSkillAsOwner(); + const res = await request(app) + .patch(`/api/skills/${created.body._id}`) + .send({ description: 'no version' }); + expect(res.status).toBe(400); + }); + + it('returns 403 for a user without EDIT permission', async () => { + const created = await createSkillAsOwner(); + setTestUser(testUsers.noAccess); + const res = await request(app) + .patch(`/api/skills/${created.body._id}`) + .send({ expectedVersion: 1, description: 'nope' }); + expect(res.status).toBe(403); + }); + }); + + describe('DELETE /api/skills/:id', () => { + it('deletes and cascades ACL entries', async () => { + const created = await createSkillAsOwner(); + const res = await request(app).delete(`/api/skills/${created.body._id}`); + expect(res.status).toBe(200); + expect(res.body.deleted).toBe(true); + + const remainingAcl = await AclEntry.countDocuments({ + resourceType: ResourceType.SKILL, + resourceId: created.body._id, + }); + expect(remainingAcl).toBe(0); + }); + + it('returns 403 for a non-owner', async () => { + const created = await createSkillAsOwner(); + setTestUser(testUsers.noAccess); + const res = await request(app).delete(`/api/skills/${created.body._id}`); + expect(res.status).toBe(403); + }); + }); + + describe('GET /api/skills/:id/files', () => { + it('returns an empty list for a skill with no files', async () => { + const created = await createSkillAsOwner(); + const res = await request(app).get(`/api/skills/${created.body._id}/files`); + expect(res.status).toBe(200); + expect(res.body.files).toEqual([]); + }); + }); + + describe('POST /api/skills/:id/files (stub)', () => { + it('returns 501 with phase marker', async () => { + const created = await createSkillAsOwner(); + const res = await request(app).post(`/api/skills/${created.body._id}/files`); + expect(res.status).toBe(501); + expect(res.body.phase).toBe(2); + }); + }); + + describe('GET /api/skills/:id/files/:relativePath (stub)', () => { + it('returns 501 stub', async () => { + const created = await createSkillAsOwner(); + const res = await request(app).get( + `/api/skills/${created.body._id}/files/scripts%2Fparse.sh`, + ); + expect(res.status).toBe(501); + }); + }); + + describe('DELETE /api/skills/:id/files/:relativePath', () => { + const { upsertSkillFile } = require('~/models'); + + it('deletes an existing skill file, bumps skill version, and returns 200', async () => { + const created = await createSkillAsOwner(); + await upsertSkillFile({ + skillId: created.body._id, + relativePath: 'scripts/parse.sh', + file_id: 'file-1', + filename: 'parse.sh', + filepath: '/tmp/parse.sh', + source: 'local', + mimeType: 'text/x-shellscript', + bytes: 42, + author: testUsers.owner._id, + }); + + const beforeSkill = await request(app).get(`/api/skills/${created.body._id}`); + expect(beforeSkill.body.fileCount).toBe(1); + expect(beforeSkill.body.version).toBe(2); + + const res = await request(app).delete( + `/api/skills/${created.body._id}/files/scripts%2Fparse.sh`, + ); + expect(res.status).toBe(200); + expect(res.body).toEqual({ + skillId: created.body._id, + relativePath: 'scripts/parse.sh', + deleted: true, + }); + + const afterSkill = await request(app).get(`/api/skills/${created.body._id}`); + expect(afterSkill.body.fileCount).toBe(0); + expect(afterSkill.body.version).toBe(3); + }); + + it('returns 404 when the file does not exist', async () => { + const created = await createSkillAsOwner(); + const res = await request(app).delete( + `/api/skills/${created.body._id}/files/scripts%2Fmissing.sh`, + ); + expect(res.status).toBe(404); + }); + + it('returns 403 for a non-owner', async () => { + const created = await createSkillAsOwner(); + setTestUser(testUsers.noAccess); + const res = await request(app).delete( + `/api/skills/${created.body._id}/files/scripts%2Fparse.sh`, + ); + expect(res.status).toBe(403); + }); + }); + + describe('Sharing via ACL (editor grant)', () => { + it('allows an editor to patch a shared skill', async () => { + const created = await createSkillAsOwner(); + await grantPermission({ + principalType: PrincipalType.USER, + principalId: testUsers.editor._id, + resourceType: ResourceType.SKILL, + resourceId: created.body._id, + accessRoleId: AccessRoleIds.SKILL_EDITOR, + grantedBy: testUsers.owner._id, + }); + + setTestUser(testUsers.editor); + const res = await request(app) + .patch(`/api/skills/${created.body._id}`) + .send({ expectedVersion: 1, description: 'Edited by editor' }); + expect(res.status).toBe(200); + + // Editor should NOT be able to delete + const del = await request(app).delete(`/api/skills/${created.body._id}`); + expect(del.status).toBe(403); + }); + }); +}); diff --git a/client/src/data-provider/Skills/index.ts b/client/src/data-provider/Skills/index.ts new file mode 100644 index 0000000000..d0720956a0 --- /dev/null +++ b/client/src/data-provider/Skills/index.ts @@ -0,0 +1,2 @@ +export * from './queries'; +export * from './mutations'; diff --git a/client/src/data-provider/Skills/mutations.ts b/client/src/data-provider/Skills/mutations.ts new file mode 100644 index 0000000000..473d13ff54 --- /dev/null +++ b/client/src/data-provider/Skills/mutations.ts @@ -0,0 +1,264 @@ +import { useMutation, useQueryClient } from '@tanstack/react-query'; +import { QueryKeys, dataService } from 'librechat-data-provider'; +import type { InfiniteData, QueryKey, UseMutationResult } from '@tanstack/react-query'; +import type { + TSkill, + TSkillFile, + TCreateSkill, + TUpdateSkillVariables, + TUpdateSkillResponse, + TDeleteSkillResponse, + TSkillListResponse, + TSkillCacheEntry, + TUploadSkillFileVariables, + TDeleteSkillFileVariables, + TDeleteSkillFileResponse, + TListSkillFilesResponse, + CreateSkillOptions, + UpdateSkillOptions, + DeleteSkillOptions, + UploadSkillFileOptions, + DeleteSkillFileOptions, +} from 'librechat-data-provider'; + +function isInfiniteSkillData( + data: TSkillListResponse | InfiniteData, +): data is InfiniteData { + return Array.isArray((data as InfiniteData).pages); +} + +/** + * Prepend a newly-created skill into every cached skill list. For infinite + * queries the new skill is inserted at the top of page 0 only (it belongs + * strictly "before" every other page by the cursor contract). For flat + * queries it's prepended to the single page. + * + * This mirrors the established prompts pattern (`useUpdatePromptGroup`): we + * intentionally write across every `[QueryKeys.skills]` entry regardless of + * each entry's embedded filter. In exchange for instant visual feedback on + * every open list view, a filtered view may briefly show a non-matching skill + * until the next explicit refetch. Trying to match filters from a mutation + * callback would couple the hook to every query-key variant's internals, + * which is exactly what React Query's `setQueriesData` is designed to avoid. + */ +function addSkillToCachedLists( + queryClient: ReturnType, + skill: TSkill, +): void { + queryClient.setQueriesData([QueryKeys.skills], (data) => { + if (!data) return data; + if (isInfiniteSkillData(data)) { + return { + ...data, + pages: data.pages.map((page, i) => + i === 0 ? { ...page, skills: [skill, ...page.skills] } : page, + ), + }; + } + return { ...data, skills: [skill, ...data.skills] }; + }); +} + +/** Replace a skill by id in every cached page that contains it. */ +function replaceSkillInCachedLists( + queryClient: ReturnType, + skill: TSkill, +): void { + queryClient.setQueriesData([QueryKeys.skills], (data) => { + if (!data) return data; + if (isInfiniteSkillData(data)) { + return { + ...data, + pages: data.pages.map((page) => ({ + ...page, + skills: page.skills.map((existing) => (existing._id === skill._id ? skill : existing)), + })), + }; + } + return { + ...data, + skills: data.skills.map((existing) => (existing._id === skill._id ? skill : existing)), + }; + }); +} + +/** + * Remove a deleted skill id from every cached page that contains it. Id-based + * removal is always safe regardless of filter — a deleted skill never belongs + * in any list. + */ +function removeSkillFromCachedLists( + queryClient: ReturnType, + id: string, +): void { + queryClient.setQueriesData([QueryKeys.skills], (data) => { + if (!data) return data; + if (isInfiniteSkillData(data)) { + return { + ...data, + pages: data.pages.map((page) => ({ + ...page, + skills: page.skills.filter((s) => s._id !== id), + })), + }; + } + return { ...data, skills: data.skills.filter((s) => s._id !== id) }; + }); +} + +/** + * Create a new skill. On success, writes the new skill into both the detail and + * list caches so any open listing UI updates immediately. + */ +export const useCreateSkillMutation = ( + options?: CreateSkillOptions, +): UseMutationResult => { + const queryClient = useQueryClient(); + const { onSuccess, ...rest } = options ?? {}; + return useMutation({ + mutationFn: (payload: TCreateSkill) => dataService.createSkill(payload), + ...rest, + onSuccess: (skill, variables, context) => { + queryClient.setQueryData([QueryKeys.skill, skill._id], skill); + addSkillToCachedLists(queryClient, skill); + if (onSuccess) onSuccess(skill, variables, context); + }, + }); +}; + +/** + * Update a skill. Uses optimistic updates mirroring `useUpdatePromptGroup`: + * - cancel in-flight queries so a late refetch can't clobber the optimistic state + * - snapshot the previous skill + list data in `onMutate` + * - apply the patch locally + * - roll back on error + * - replace with server state on success (captures the new version) + */ +export const useUpdateSkillMutation = ( + options?: UpdateSkillOptions, +): UseMutationResult => { + const queryClient = useQueryClient(); + const { onMutate, onError, onSuccess, ...rest } = options ?? {}; + return useMutation({ + mutationFn: (vars: TUpdateSkillVariables) => dataService.updateSkill(vars), + ...rest, + onMutate: async (variables) => { + // Prevent in-flight refetches from overwriting the optimistic update. + await queryClient.cancelQueries([QueryKeys.skill, variables.id]); + await queryClient.cancelQueries([QueryKeys.skills]); + + const previousSkill = queryClient.getQueryData([QueryKeys.skill, variables.id]); + const previousListSnapshots = queryClient.getQueriesData([ + QueryKeys.skills, + ]) as Array<[QueryKey, TSkillCacheEntry]>; + + if (previousSkill) { + const optimistic: TSkill = { + ...previousSkill, + ...variables.payload, + frontmatter: { + ...(previousSkill.frontmatter ?? {}), + ...(variables.payload.frontmatter ?? {}), + } as TSkill['frontmatter'], + }; + queryClient.setQueryData([QueryKeys.skill, variables.id], optimistic); + replaceSkillInCachedLists(queryClient, optimistic); + } + + const userContext = await onMutate?.(variables); + return { previousSkill, previousListSnapshots, userContext }; + }, + onError: (error, variables, context) => { + if (context?.previousSkill) { + queryClient.setQueryData([QueryKeys.skill, variables.id], context.previousSkill); + } + if (context?.previousListSnapshots) { + for (const [key, value] of context.previousListSnapshots) { + queryClient.setQueryData(key, value); + } + } + if (onError) onError(error, variables, context); + }, + onSuccess: (skill, variables, context) => { + queryClient.setQueryData([QueryKeys.skill, skill._id], skill); + replaceSkillInCachedLists(queryClient, skill); + if (onSuccess) onSuccess(skill, variables, context); + }, + }); +}; + +/** + * Delete a skill. Removes it from caches and invalidates its file list cache. + */ +export const useDeleteSkillMutation = ( + options?: DeleteSkillOptions, +): UseMutationResult => { + const queryClient = useQueryClient(); + const { onSuccess, ...rest } = options ?? {}; + return useMutation({ + mutationFn: ({ id }: { id: string }) => dataService.deleteSkill(id), + ...rest, + onSuccess: (response, variables, context) => { + queryClient.removeQueries([QueryKeys.skill, variables.id]); + queryClient.removeQueries([QueryKeys.skillFiles, variables.id]); + removeSkillFromCachedLists(queryClient, variables.id); + if (onSuccess) onSuccess(response, variables, context); + }, + }); +}; + +/** + * Upload a file into a skill. Stubbed in phase 1 — the backend responds 501. + * The hook is wired now so the frontend can call it once the backend is ready. + */ +export const useUploadSkillFileMutation = ( + options?: UploadSkillFileOptions, +): UseMutationResult => { + const queryClient = useQueryClient(); + const { onSuccess, ...rest } = options ?? {}; + return useMutation({ + mutationFn: ({ skillId, formData }: TUploadSkillFileVariables) => + dataService.uploadSkillFile(skillId, formData), + ...rest, + onSuccess: (skillFile, variables, context) => { + queryClient.setQueryData( + [QueryKeys.skillFiles, variables.skillId], + (prev) => { + if (!prev) return { files: [skillFile] }; + const filtered = prev.files.filter((f) => f.relativePath !== skillFile.relativePath); + return { files: [...filtered, skillFile] }; + }, + ); + queryClient.invalidateQueries([QueryKeys.skill, variables.skillId]); + if (onSuccess) onSuccess(skillFile, variables, context); + }, + }); +}; + +/** + * Delete a file from a skill. Works in phase 1 — only the SkillFile row is removed. + */ +export const useDeleteSkillFileMutation = ( + options?: DeleteSkillFileOptions, +): UseMutationResult => { + const queryClient = useQueryClient(); + const { onSuccess, ...rest } = options ?? {}; + return useMutation({ + mutationFn: ({ skillId, relativePath }: TDeleteSkillFileVariables) => + dataService.deleteSkillFile(skillId, relativePath), + ...rest, + onSuccess: (response, variables, context) => { + queryClient.setQueryData( + [QueryKeys.skillFiles, variables.skillId], + (prev) => { + if (!prev) return prev; + return { + files: prev.files.filter((f) => f.relativePath !== variables.relativePath), + }; + }, + ); + queryClient.invalidateQueries([QueryKeys.skill, variables.skillId]); + if (onSuccess) onSuccess(response, variables, context); + }, + }); +}; diff --git a/client/src/data-provider/Skills/queries.ts b/client/src/data-provider/Skills/queries.ts new file mode 100644 index 0000000000..ecbad1a342 --- /dev/null +++ b/client/src/data-provider/Skills/queries.ts @@ -0,0 +1,117 @@ +import { useQuery, useInfiniteQuery } from '@tanstack/react-query'; +import { QueryKeys, dataService } from 'librechat-data-provider'; +import type { + QueryObserverResult, + UseQueryOptions, + UseInfiniteQueryOptions, +} from '@tanstack/react-query'; +import type { + TSkill, + TSkillListRequest, + TSkillListResponse, + TListSkillFilesResponse, +} from 'librechat-data-provider'; + +/** + * Paginated skill list (single page) — use this for small lists or when you want to + * control pagination manually. + */ +export const useListSkillsQuery = ( + params?: TSkillListRequest, + config?: UseQueryOptions, +): QueryObserverResult => { + return useQuery( + [ + QueryKeys.skills, + params?.category ?? '', + params?.search ?? '', + params?.limit ?? 20, + params?.cursor ?? '', + ], + () => dataService.listSkills(params), + { + refetchOnWindowFocus: false, + refetchOnReconnect: false, + refetchOnMount: false, + ...config, + }, + ); +}; + +/** + * Cursor-paginated infinite query for skills. Use this for the main skills listing UI. + */ +export const useSkillsInfiniteQuery = ( + params?: Omit, + config?: UseInfiniteQueryOptions, +) => { + return useInfiniteQuery( + [ + QueryKeys.skills, + 'infinite', + params?.category ?? '', + params?.search ?? '', + params?.limit ?? 20, + ], + ({ pageParam }) => { + const request: TSkillListRequest = { + category: params?.category, + search: params?.search, + limit: params?.limit, + }; + if (typeof pageParam === 'string' && pageParam.length > 0) { + request.cursor = pageParam; + } + return dataService.listSkills(request); + }, + { + getNextPageParam: (lastPage) => + lastPage.has_more && lastPage.after ? lastPage.after : undefined, + refetchOnWindowFocus: false, + refetchOnReconnect: false, + refetchOnMount: false, + ...config, + }, + ); +}; + +/** + * Fetch a single skill by id (includes full body + frontmatter). + */ +export const useGetSkillQuery = ( + id: string | null | undefined, + config?: UseQueryOptions, +): QueryObserverResult => { + const enabled = !!id; + return useQuery([QueryKeys.skill, id], () => dataService.getSkill(id as string), { + refetchOnWindowFocus: false, + refetchOnReconnect: false, + refetchOnMount: false, + retry: false, + ...config, + enabled: enabled && (config?.enabled ?? true), + }); +}; + +/** + * List file metadata for a single skill. In phase 1 this returns an empty array for + * skills that have only an inline `SKILL.md`; multi-file skills arrive in phase 2. + */ +export const useListSkillFilesQuery = ( + skillId: string | null | undefined, + config?: UseQueryOptions, +): QueryObserverResult => { + const enabled = !!skillId; + return useQuery( + [QueryKeys.skillFiles, skillId], + () => dataService.listSkillFiles(skillId as string), + { + refetchOnWindowFocus: false, + refetchOnReconnect: false, + refetchOnMount: false, + retry: false, + ...config, + enabled: enabled && (config?.enabled ?? true), + }, + ); +}; diff --git a/client/src/data-provider/index.ts b/client/src/data-provider/index.ts index bfc87bb232..8d1e38f3a5 100644 --- a/client/src/data-provider/index.ts +++ b/client/src/data-provider/index.ts @@ -12,6 +12,7 @@ export * from './Favorites'; export * from './mutations'; export * from './prompts'; export * from './queries'; +export * from './Skills'; export * from './roles'; export * from './tags'; export * from './MCP'; diff --git a/client/src/hooks/Sharing/useCanSharePublic.ts b/client/src/hooks/Sharing/useCanSharePublic.ts index 54355dd9a8..e93a3dcc82 100644 --- a/client/src/hooks/Sharing/useCanSharePublic.ts +++ b/client/src/hooks/Sharing/useCanSharePublic.ts @@ -6,6 +6,7 @@ const resourceToPermissionMap: Partial> = [ResourceType.PROMPTGROUP]: PermissionTypes.PROMPTS, [ResourceType.MCPSERVER]: PermissionTypes.MCP_SERVERS, [ResourceType.REMOTE_AGENT]: PermissionTypes.REMOTE_AGENTS, + [ResourceType.SKILL]: PermissionTypes.SKILLS, }; /** diff --git a/packages/api/src/index.ts b/packages/api/src/index.ts index d4b6ac9542..6369ce6ed4 100644 --- a/packages/api/src/index.ts +++ b/packages/api/src/index.ts @@ -37,6 +37,8 @@ export * from './memory'; export * from './agents'; /* Prompts */ export * from './prompts'; +/* Skills */ +export * from './skills'; /* Endpoints */ export * from './endpoints'; /* Files */ diff --git a/packages/api/src/skills/handlers.ts b/packages/api/src/skills/handlers.ts new file mode 100644 index 0000000000..3b044a8a9d --- /dev/null +++ b/packages/api/src/skills/handlers.ts @@ -0,0 +1,572 @@ +import { logger } from '@librechat/data-schemas'; +import { + ResourceType, + AccessRoleIds, + PrincipalType, + PermissionBits, +} from 'librechat-data-provider'; +import type { Response } from 'express'; +import type { Types } from 'mongoose'; +import type { + ISkill, + ISkillFile, + ISkillSummary, + CreateSkillInput, + CreateSkillResult, + UpdateSkillInput, + ListSkillsByAccessResult, + UpdateSkillResult, + ValidationIssue, +} from '@librechat/data-schemas'; +import type { + TSkill, + TSkillFile, + TSkillSummary, + TSkillWarning, + TCreateSkill, + TUpdateSkillPayload, + TListSkillFilesResponse, + TDeleteSkillResponse, + TDeleteSkillFileResponse, + TSkillConflictResponse, +} from 'librechat-data-provider'; +import type { ServerRequest } from '~/types/http'; + +/** Thin error shape the skill methods throw on validation failure. */ +type SkillValidationError = Error & { code?: string; issues?: ValidationIssue[] }; + +/** Mongo duplicate-key shape. */ +type DuplicateKeyError = Error & { code?: number | string }; + +/** + * All dependencies required to serve skill HTTP requests. Every dep is resolved + * from the legacy api layer (`~/models`, `PermissionService`) so the TS handlers + * stay pure — no direct imports of mongoose, no direct filesystem I/O. + */ +export interface SkillsHandlersDeps { + /** Skill CRUD — from `@librechat/data-schemas` `createMethods` output. */ + createSkill: (data: CreateSkillInput) => Promise; + getSkillById: (id: string | Types.ObjectId) => Promise<(ISkill & { _id: Types.ObjectId }) | null>; + listSkillsByAccess: (params: { + accessibleIds: Types.ObjectId[]; + category?: string; + search?: string; + limit: number; + cursor?: string | null; + }) => Promise; + updateSkill: (params: { + id: string; + expectedVersion: number; + update: UpdateSkillInput; + }) => Promise; + deleteSkill: (id: string) => Promise<{ deleted: boolean }>; + listSkillFiles: ( + skillId: string | Types.ObjectId, + ) => Promise>; + deleteSkillFile: ( + skillId: string | Types.ObjectId, + relativePath: string, + ) => Promise<{ deleted: boolean }>; + + /** Access-control primitives from PermissionService. */ + findAccessibleResources: (params: { + userId: string; + role?: string | null; + resourceType: string; + requiredPermissions: number; + }) => Promise; + findPubliclyAccessibleResources: (params: { + resourceType: string; + requiredPermissions: number; + }) => Promise; + grantPermission: (params: { + principalType: string; + principalId: string | Types.ObjectId; + resourceType: string; + resourceId: string | Types.ObjectId; + accessRoleId: string; + grantedBy: string | Types.ObjectId; + }) => Promise; + + /** ObjectId validation helper from data-schemas. */ + isValidObjectIdString: (value: unknown) => boolean; +} + +/** + * Narrow an opaque `Record` frontmatter (as stored in Mongoose + * `Mixed`) to the wire type. Returns `undefined` for empty/missing frontmatter + * so clients see a clear absence instead of an empty `{}`. + */ +function serializeFrontmatter( + frontmatter: Record | undefined, +): TSkill['frontmatter'] { + if (!frontmatter || typeof frontmatter !== 'object' || Object.keys(frontmatter).length === 0) { + return undefined; + } + return frontmatter as TSkill['frontmatter']; +} + +function serializeSourceMetadata( + metadata: Record | undefined, +): TSkill['sourceMetadata'] { + if (!metadata || typeof metadata !== 'object' || Object.keys(metadata).length === 0) { + return undefined; + } + return metadata as TSkill['sourceMetadata']; +} + +/** Converts a skill document to the wire format returned by the API. */ +function serializeSkill(skill: ISkill & { _id: Types.ObjectId }, publicSet: Set): TSkill { + return { + _id: skill._id.toString(), + name: skill.name, + displayTitle: skill.displayTitle, + description: skill.description, + body: skill.body, + frontmatter: serializeFrontmatter(skill.frontmatter), + category: skill.category, + author: skill.author.toString(), + authorName: skill.authorName, + version: skill.version, + source: skill.source, + sourceMetadata: serializeSourceMetadata(skill.sourceMetadata), + fileCount: skill.fileCount, + isPublic: publicSet.has(skill._id.toString()), + tenantId: skill.tenantId, + createdAt: (skill.createdAt ?? new Date()).toISOString(), + updatedAt: (skill.updatedAt ?? new Date()).toISOString(), + }; +} + +function serializeSkillSummary( + skill: ISkillSummary & { _id: Types.ObjectId }, + publicSet: Set, +): TSkillSummary { + return { + _id: skill._id.toString(), + name: skill.name, + displayTitle: skill.displayTitle, + description: skill.description, + category: skill.category, + author: skill.author.toString(), + authorName: skill.authorName, + version: skill.version, + source: skill.source, + sourceMetadata: serializeSourceMetadata(skill.sourceMetadata), + fileCount: skill.fileCount, + isPublic: publicSet.has(skill._id.toString()), + tenantId: skill.tenantId, + createdAt: (skill.createdAt ?? new Date()).toISOString(), + updatedAt: (skill.updatedAt ?? new Date()).toISOString(), + }; +} + +function serializeSkillFile(file: ISkillFile & { _id: Types.ObjectId }): TSkillFile { + return { + _id: file._id.toString(), + skillId: file.skillId.toString(), + relativePath: file.relativePath, + file_id: file.file_id, + filename: file.filename, + filepath: file.filepath, + source: file.source as TSkillFile['source'], + mimeType: file.mimeType, + bytes: file.bytes, + category: file.category, + isExecutable: file.isExecutable, + author: file.author.toString(), + tenantId: file.tenantId, + createdAt: (file.createdAt ?? new Date()).toISOString(), + updatedAt: (file.updatedAt ?? new Date()).toISOString(), + }; +} + +/** + * Attach non-blocking coaching warnings to a serialized skill response. + * Called from `createHandler` and `patchHandler` so clients can show inline + * feedback (e.g. "description too short") without the write being rejected. + * Only warning-severity issues come through here — errors are thrown by + * `createSkill`/`updateSkill` before we reach this point. + */ +function attachWarnings(skill: TSkill, warnings: ValidationIssue[]): TSkill { + if (!warnings || warnings.length === 0) { + return skill; + } + return { + ...skill, + warnings: warnings.map((w) => ({ + field: w.field, + code: w.code, + message: w.message, + severity: 'warning' as const, + })) satisfies TSkillWarning[], + }; +} + +function isValidationError(error: unknown): error is SkillValidationError { + if (!error || typeof error !== 'object') { + return false; + } + const code = (error as { code?: unknown }).code; + return code === 'SKILL_VALIDATION_FAILED' || code === 'SKILL_FILE_VALIDATION_FAILED'; +} + +function isDuplicateKeyError(error: unknown): error is DuplicateKeyError { + if (!error || typeof error !== 'object') { + return false; + } + return (error as { code?: unknown }).code === 11000; +} + +function parseLimit(raw: unknown): number { + const parsed = parseInt(String(raw ?? '20'), 10); + if (Number.isNaN(parsed)) { + return 20; + } + return Math.min(Math.max(1, parsed), 100); +} + +/** + * Factory for the typed Express handlers served at `/api/skills`. + * The legacy `api/server/routes/skills.js` imports this, passes in concrete + * deps from `~/models` + `PermissionService`, and wires the returned handlers + * onto the Express router. + */ +export function createSkillsHandlers(deps: SkillsHandlersDeps) { + const { + createSkill, + getSkillById, + listSkillsByAccess, + updateSkill, + deleteSkill, + listSkillFiles, + deleteSkillFile, + findAccessibleResources, + findPubliclyAccessibleResources, + grantPermission, + isValidObjectIdString, + } = deps; + + async function getPublicSkillIdSet(): Promise> { + try { + const publicIds = await findPubliclyAccessibleResources({ + resourceType: ResourceType.SKILL, + requiredPermissions: PermissionBits.VIEW, + }); + return new Set(publicIds.map((id) => id.toString())); + } catch (error) { + logger.error('[skills] Failed to fetch public skill IDs', error); + return new Set(); + } + } + + async function listHandler(req: ServerRequest, res: Response) { + try { + const user = req.user; + if (!user || !user.id) { + return res.status(401).json({ error: 'Authentication required' }); + } + const { category, search, limit, cursor } = req.query as { + category?: string; + search?: string; + limit?: string; + cursor?: string; + }; + const parsedLimit = parseLimit(limit); + + const [accessibleIds, publicIds] = await Promise.all([ + findAccessibleResources({ + userId: user.id, + role: user.role, + resourceType: ResourceType.SKILL, + requiredPermissions: PermissionBits.VIEW, + }), + findPubliclyAccessibleResources({ + resourceType: ResourceType.SKILL, + requiredPermissions: PermissionBits.VIEW, + }), + ]); + + const mergedIds = Array.from( + new Map([...accessibleIds, ...publicIds].map((id) => [id.toString(), id])).values(), + ); + + const result = await listSkillsByAccess({ + accessibleIds: mergedIds, + category: typeof category === 'string' && category.length > 0 ? category : undefined, + search: typeof search === 'string' && search.length > 0 ? search : undefined, + limit: parsedLimit, + cursor: typeof cursor === 'string' && cursor.length > 0 ? cursor : null, + }); + + const publicSet = new Set(publicIds.map((id) => id.toString())); + const skills = result.skills.map((s) => serializeSkillSummary(s, publicSet)); + + return res.status(200).json({ + skills, + has_more: result.has_more, + after: result.after, + }); + } catch (error) { + logger.error('[GET /skills] Error listing skills', error); + return res.status(500).json({ error: 'Error listing skills' }); + } + } + + async function createHandler(req: ServerRequest, res: Response) { + try { + const user = req.user; + if (!user || !user.id) { + return res.status(401).json({ error: 'Authentication required' }); + } + + const body = (req.body ?? {}) as TCreateSkill; + + if (!body.name || typeof body.name !== 'string') { + return res.status(400).json({ error: 'Skill name is required' }); + } + if (!body.description || typeof body.description !== 'string') { + return res.status(400).json({ error: 'Skill description is required' }); + } + + const authorId = (user._id ?? user.id) as unknown as Types.ObjectId; + const authorName = user.name ?? user.username ?? 'Unknown'; + + let createResult: CreateSkillResult; + try { + createResult = await createSkill({ + name: body.name, + displayTitle: body.displayTitle, + description: body.description, + body: body.body, + frontmatter: body.frontmatter as Record | undefined, + category: body.category, + author: authorId, + authorName, + tenantId: user.tenantId, + }); + } catch (error) { + if (isValidationError(error)) { + return res.status(400).json({ error: 'Validation failed', issues: error.issues }); + } + if (isDuplicateKeyError(error)) { + return res.status(409).json({ error: 'A skill with this name already exists' }); + } + throw error; + } + + const { skill, warnings } = createResult; + + try { + await grantPermission({ + principalType: PrincipalType.USER, + principalId: user.id, + resourceType: ResourceType.SKILL, + resourceId: skill._id, + accessRoleId: AccessRoleIds.SKILL_OWNER, + grantedBy: user.id, + }); + } catch (permissionError) { + logger.error( + `[POST /skills] Failed to grant owner permission for skill ${skill._id.toString()}, rolling back:`, + permissionError, + ); + try { + await deleteSkill(skill._id.toString()); + } catch (rollbackError) { + logger.error( + `[POST /skills] Compensating delete failed for orphaned skill ${skill._id.toString()}:`, + rollbackError, + ); + } + return res.status(500).json({ error: 'Failed to initialize skill permissions' }); + } + + // A freshly created skill has no PUBLIC ACL entry, so `isPublic` is + // always false. Skip the DB round-trip. + return res + .status(201) + .json(attachWarnings(serializeSkill(skill, new Set()), warnings)); + } catch (error) { + logger.error('[POST /skills] Error creating skill', error); + return res.status(500).json({ error: 'Error creating skill' }); + } + } + + async function getHandler(req: ServerRequest, res: Response) { + try { + const { id } = req.params as { id: string }; + // The canAccessSkillResource middleware already resolved the skill via + // getSkillById as its idResolver and stashed it on req.resourceAccess.resourceInfo. + // Reuse it to avoid a second DB round-trip. + const resolved = ( + req as ServerRequest & { + resourceAccess?: { resourceInfo?: ISkill & { _id: Types.ObjectId } }; + } + ).resourceAccess?.resourceInfo; + const skill = resolved ?? (await getSkillById(id)); + if (!skill) { + return res.status(404).json({ error: 'Skill not found' }); + } + const publicSet = await getPublicSkillIdSet(); + return res.status(200).json(serializeSkill(skill, publicSet)); + } catch (error) { + logger.error('[GET /skills/:id] Error fetching skill', error); + return res.status(500).json({ error: 'Error fetching skill' }); + } + } + + async function patchHandler(req: ServerRequest, res: Response) { + try { + const { id } = req.params as { id: string }; + const body = (req.body ?? {}) as TUpdateSkillPayload & { expectedVersion?: number }; + const { expectedVersion, ...rest } = body; + // `typeof NaN === 'number'` is true, so we need the stricter isFinite/isInteger + // checks below to avoid NaN passing through and triggering a misleading 409 + // (MongoDB's `{ version: NaN }` never matches, so the handler would fall + // through to the conflict branch and leak the current skill state). + if ( + typeof expectedVersion !== 'number' || + !Number.isFinite(expectedVersion) || + !Number.isInteger(expectedVersion) || + expectedVersion < 1 + ) { + return res + .status(400) + .json({ error: 'expectedVersion is required and must be a positive integer' }); + } + + const update: UpdateSkillInput = {}; + if (rest.name !== undefined) update.name = rest.name; + if (rest.displayTitle !== undefined) update.displayTitle = rest.displayTitle; + if (rest.description !== undefined) update.description = rest.description; + if (rest.body !== undefined) update.body = rest.body; + if (rest.frontmatter !== undefined) { + update.frontmatter = rest.frontmatter as Record; + } + if (rest.category !== undefined) update.category = rest.category; + + if (Object.keys(update).length === 0) { + return res.status(400).json({ error: 'At least one field must be provided for update' }); + } + + let result: UpdateSkillResult; + try { + result = await updateSkill({ id, expectedVersion, update }); + } catch (error) { + if (isValidationError(error)) { + return res.status(400).json({ error: 'Validation failed', issues: error.issues }); + } + if (isDuplicateKeyError(error)) { + return res.status(409).json({ error: 'A skill with this name already exists' }); + } + throw error; + } + + if (result.status === 'not_found') { + return res.status(404).json({ error: 'Skill not found' }); + } + const publicSet = await getPublicSkillIdSet(); + if (result.status === 'conflict') { + const conflict: TSkillConflictResponse = { + error: 'skill_version_conflict', + current: serializeSkill(result.current, publicSet), + }; + return res.status(409).json(conflict); + } + return res + .status(200) + .json(attachWarnings(serializeSkill(result.skill, publicSet), result.warnings)); + } catch (error) { + logger.error('[PATCH /skills/:id] Error updating skill', error); + return res.status(500).json({ error: 'Error updating skill' }); + } + } + + async function deleteHandler(req: ServerRequest, res: Response) { + try { + const { id } = req.params as { id: string }; + if (!isValidObjectIdString(id)) { + return res.status(400).json({ error: 'Invalid skill id' }); + } + const result = await deleteSkill(id); + if (!result.deleted) { + return res.status(404).json({ error: 'Skill not found' }); + } + const response: TDeleteSkillResponse = { id, deleted: true }; + return res.status(200).json(response); + } catch (error) { + logger.error('[DELETE /skills/:id] Error deleting skill', error); + return res.status(500).json({ error: 'Error deleting skill' }); + } + } + + async function listFilesHandler(req: ServerRequest, res: Response) { + try { + const { id } = req.params as { id: string }; + const rows = await listSkillFiles(id); + const response: TListSkillFilesResponse = { files: rows.map(serializeSkillFile) }; + return res.status(200).json(response); + } catch (error) { + logger.error('[GET /skills/:id/files] Error listing skill files', error); + return res.status(500).json({ error: 'Error listing skill files' }); + } + } + + function uploadFileStubHandler(_req: ServerRequest, res: Response) { + return res.status(501).json({ + error: 'skill_file_upload_not_implemented', + phase: 2, + message: + 'Skill file upload is not yet wired up. This endpoint is a stub reserved for phase 2.', + }); + } + + function downloadFileStubHandler(_req: ServerRequest, res: Response) { + return res.status(501).json({ + error: 'skill_file_download_not_implemented', + phase: 2, + message: + 'Skill file download is not yet wired up. This endpoint is a stub reserved for phase 2.', + }); + } + + async function deleteFileHandler(req: ServerRequest, res: Response) { + try { + const { id, relativePath } = req.params as { id: string; relativePath: string }; + let decodedPath: string; + try { + decodedPath = decodeURIComponent(relativePath); + } catch { + return res.status(400).json({ error: 'Invalid file path encoding' }); + } + const result = await deleteSkillFile(id, decodedPath); + if (!result.deleted) { + return res.status(404).json({ error: 'Skill file not found' }); + } + const response: TDeleteSkillFileResponse = { + skillId: id, + relativePath: decodedPath, + deleted: true, + }; + return res.status(200).json(response); + } catch (error) { + logger.error('[DELETE /skills/:id/files/:relativePath] Error', error); + return res.status(500).json({ error: 'Error deleting skill file' }); + } + } + + return { + list: listHandler, + create: createHandler, + get: getHandler, + patch: patchHandler, + delete: deleteHandler, + listFiles: listFilesHandler, + uploadFileStub: uploadFileStubHandler, + downloadFileStub: downloadFileStubHandler, + deleteFile: deleteFileHandler, + }; +} + +export type SkillsHandlers = ReturnType; diff --git a/packages/api/src/skills/index.ts b/packages/api/src/skills/index.ts new file mode 100644 index 0000000000..6c6f862d01 --- /dev/null +++ b/packages/api/src/skills/index.ts @@ -0,0 +1 @@ +export * from './handlers'; diff --git a/packages/data-provider/src/accessPermissions.ts b/packages/data-provider/src/accessPermissions.ts index bc97458076..8047f14b79 100644 --- a/packages/data-provider/src/accessPermissions.ts +++ b/packages/data-provider/src/accessPermissions.ts @@ -47,6 +47,7 @@ export enum ResourceType { PROMPTGROUP = 'promptGroup', MCPSERVER = 'mcpServer', REMOTE_AGENT = 'remoteAgent', + SKILL = 'skill', } /** @@ -79,6 +80,9 @@ export enum AccessRoleIds { REMOTE_AGENT_VIEWER = 'remoteAgent_viewer', REMOTE_AGENT_EDITOR = 'remoteAgent_editor', REMOTE_AGENT_OWNER = 'remoteAgent_owner', + SKILL_VIEWER = 'skill_viewer', + SKILL_EDITOR = 'skill_editor', + SKILL_OWNER = 'skill_owner', } // ===== ZOD SCHEMAS ===== @@ -317,16 +321,19 @@ export function accessRoleToPermBits(accessRoleId: string): number { case AccessRoleIds.PROMPTGROUP_VIEWER: case AccessRoleIds.MCPSERVER_VIEWER: case AccessRoleIds.REMOTE_AGENT_VIEWER: + case AccessRoleIds.SKILL_VIEWER: return PermissionBits.VIEW; case AccessRoleIds.AGENT_EDITOR: case AccessRoleIds.PROMPTGROUP_EDITOR: case AccessRoleIds.MCPSERVER_EDITOR: case AccessRoleIds.REMOTE_AGENT_EDITOR: + case AccessRoleIds.SKILL_EDITOR: return PermissionBits.VIEW | PermissionBits.EDIT; case AccessRoleIds.AGENT_OWNER: case AccessRoleIds.PROMPTGROUP_OWNER: case AccessRoleIds.MCPSERVER_OWNER: case AccessRoleIds.REMOTE_AGENT_OWNER: + case AccessRoleIds.SKILL_OWNER: return ( PermissionBits.VIEW | PermissionBits.EDIT | PermissionBits.DELETE | PermissionBits.SHARE ); diff --git a/packages/data-provider/src/api-endpoints.ts b/packages/data-provider/src/api-endpoints.ts index 21f9cdc6a3..69867a14ef 100644 --- a/packages/data-provider/src/api-endpoints.ts +++ b/packages/data-provider/src/api-endpoints.ts @@ -359,6 +359,33 @@ export const getCategories = () => `${BASE_URL}/api/categories`; export const getAllPromptGroups = () => `${prompts()}/all`; +/* Skills */ +export const skills = () => `${BASE_URL}/api/skills`; + +export const getSkill = (id: string) => `${skills()}/${encodeURIComponent(id)}`; + +export const listSkillsWithFilters = ( + filter: Record, +) => { + const cleaned = Object.entries(filter).reduce( + (acc, [key, value]) => { + if (value !== undefined && value !== null && value !== '') { + acc[key] = String(value); + } + return acc; + }, + {} as Record, + ); + const query = + Object.keys(cleaned).length > 0 ? `?${new URLSearchParams(cleaned).toString()}` : ''; + return `${skills()}${query}`; +}; + +export const skillFiles = (id: string) => `${getSkill(id)}/files`; + +export const skillFile = (id: string, relativePath: string) => + `${skillFiles(id)}/${encodeURIComponent(relativePath)}`; + /* Roles */ export const roles = () => `${BASE_URL}/api/roles`; export const adminRoles = () => `${BASE_URL}/api/admin/roles`; diff --git a/packages/data-provider/src/data-service.ts b/packages/data-provider/src/data-service.ts index d21089db82..e67fbedbf2 100644 --- a/packages/data-provider/src/data-service.ts +++ b/packages/data-provider/src/data-service.ts @@ -6,6 +6,7 @@ import * as ag from './types/agents'; import * as m from './types/mutations'; import * as q from './types/queries'; import * as f from './types/files'; +import * as sk from './types/skills'; import * as mcp from './types/mcpServers'; import * as config from './config'; import request from './request'; @@ -859,6 +860,46 @@ export function getRandomPrompts( return request.get(endpoints.getRandomPrompts(variables.limit, variables.skip)); } +/* Skills */ + +export function listSkills(params?: sk.TSkillListRequest): Promise { + return request.get(endpoints.listSkillsWithFilters(params ?? {})); +} + +export function getSkill(id: string): Promise { + return request.get(endpoints.getSkill(id)); +} + +export function createSkill(payload: sk.TCreateSkill): Promise { + return request.post(endpoints.skills(), payload); +} + +export function updateSkill(variables: sk.TUpdateSkillVariables): Promise { + return request.patch(endpoints.getSkill(variables.id), { + expectedVersion: variables.expectedVersion, + ...variables.payload, + }); +} + +export function deleteSkill(id: string): Promise { + return request.delete(endpoints.getSkill(id)); +} + +export function listSkillFiles(skillId: string): Promise { + return request.get(endpoints.skillFiles(skillId)); +} + +export function uploadSkillFile(skillId: string, formData: FormData): Promise { + return request.postMultiPart(endpoints.skillFiles(skillId), formData); +} + +export function deleteSkillFile( + skillId: string, + relativePath: string, +): Promise { + return request.delete(endpoints.skillFile(skillId, relativePath)); +} + /* Roles */ export function listRoles(): Promise { return request.get(`${endpoints.adminRoles()}?limit=200`); diff --git a/packages/data-provider/src/index.ts b/packages/data-provider/src/index.ts index 2867bd1ec5..08ab674849 100644 --- a/packages/data-provider/src/index.ts +++ b/packages/data-provider/src/index.ts @@ -25,6 +25,7 @@ export * from './types/files'; export * from './types/mcpServers'; export * from './types/mutations'; export * from './types/queries'; +export * from './types/skills'; export * from './types/runs'; export * from './types/web'; export * from './types/graph'; diff --git a/packages/data-provider/src/keys.ts b/packages/data-provider/src/keys.ts index 52208d3717..28a568b87e 100644 --- a/packages/data-provider/src/keys.ts +++ b/packages/data-provider/src/keys.ts @@ -65,6 +65,10 @@ export enum QueryKeys { activeJobs = 'activeJobs', /* Agent API Keys */ agentApiKeys = 'agentApiKeys', + /* Skills */ + skills = 'skills', + skill = 'skill', + skillFiles = 'skillFiles', } // Dynamic query keys that require parameters diff --git a/packages/data-provider/src/permissions.ts b/packages/data-provider/src/permissions.ts index 58cf0df15d..3f4590af3a 100644 --- a/packages/data-provider/src/permissions.ts +++ b/packages/data-provider/src/permissions.ts @@ -60,6 +60,10 @@ export enum PermissionTypes { * Type for Remote Agent (API) Permissions */ REMOTE_AGENTS = 'REMOTE_AGENTS', + /** + * Type for Skill Permissions + */ + SKILLS = 'SKILLS', } /** @@ -82,6 +86,7 @@ export const PERMISSION_TYPE_INTERFACE_FIELDS: Record = [PermissionTypes.MARKETPLACE]: 'marketplace', [PermissionTypes.MCP_SERVERS]: 'mcpServers', [PermissionTypes.REMOTE_AGENTS]: 'remoteAgents', + [PermissionTypes.SKILLS]: 'skills', }; /** Set of interface config field names that correspond to role permissions. */ @@ -218,6 +223,14 @@ export const remoteAgentsPermissionsSchema = z.object({ }); export type TRemoteAgentsPermissions = z.infer; +export const skillPermissionsSchema = z.object({ + [Permissions.USE]: z.boolean().default(true), + [Permissions.CREATE]: z.boolean().default(true), + [Permissions.SHARE]: z.boolean().default(false), + [Permissions.SHARE_PUBLIC]: z.boolean().default(false), +}); +export type TSkillPermissions = z.infer; + // Define a single permissions schema that holds all permission types. export const permissionsSchema = z.object({ [PermissionTypes.PROMPTS]: promptPermissionsSchema, @@ -234,4 +247,5 @@ export const permissionsSchema = z.object({ [PermissionTypes.FILE_CITATIONS]: fileCitationsPermissionsSchema, [PermissionTypes.MCP_SERVERS]: mcpServersPermissionsSchema, [PermissionTypes.REMOTE_AGENTS]: remoteAgentsPermissionsSchema, + [PermissionTypes.SKILLS]: skillPermissionsSchema, }); diff --git a/packages/data-provider/src/roles.spec.ts b/packages/data-provider/src/roles.spec.ts index 60dac5ab50..503bb7cb1e 100644 --- a/packages/data-provider/src/roles.spec.ts +++ b/packages/data-provider/src/roles.spec.ts @@ -31,8 +31,7 @@ describe('roleDefaults', () => { continue; } - const userValues = - userPerms[permType as PermissionTypes] as Record; + const userValues = userPerms[permType as PermissionTypes] as Record; for (const field of fieldNames) { expect({ @@ -78,7 +77,9 @@ describe('roleDefaults', () => { for (const [permType, subSchema] of Object.entries(schemaShape)) { const fieldNames = Object.keys(subSchema.shape); - const hasResourceFields = fieldNames.some((f) => RESOURCE_MANAGEMENT_FIELDS.includes(f as Permissions)); + const hasResourceFields = fieldNames.some((f) => + RESOURCE_MANAGEMENT_FIELDS.includes(f as Permissions), + ); if (!hasResourceFields) { continue; } @@ -87,7 +88,8 @@ describe('roleDefaults', () => { restrictedSet.has(permType) || permType === PermissionTypes.MEMORIES || permType === PermissionTypes.PROMPTS || - permType === PermissionTypes.AGENTS; + permType === PermissionTypes.AGENTS || + permType === PermissionTypes.SKILLS; expect({ permType, @@ -110,8 +112,7 @@ describe('roleDefaults', () => { for (const [permType, subSchema] of Object.entries(schemaShape)) { const fieldNames = Object.keys(subSchema.shape); - const adminValues = - adminPerms[permType as PermissionTypes] as Record; + const adminValues = adminPerms[permType as PermissionTypes] as Record; for (const field of fieldNames) { expect({ diff --git a/packages/data-provider/src/roles.ts b/packages/data-provider/src/roles.ts index 0d81c55304..600c017402 100644 --- a/packages/data-provider/src/roles.ts +++ b/packages/data-provider/src/roles.ts @@ -5,6 +5,7 @@ import { permissionsSchema, agentPermissionsSchema, promptPermissionsSchema, + skillPermissionsSchema, memoryPermissionsSchema, runCodePermissionsSchema, bookmarkPermissionsSchema, @@ -103,6 +104,12 @@ const defaultRolesSchema = z.object({ [Permissions.SHARE]: z.boolean().default(true), [Permissions.SHARE_PUBLIC]: z.boolean().default(true), }), + [PermissionTypes.SKILLS]: skillPermissionsSchema.extend({ + [Permissions.USE]: z.boolean().default(true), + [Permissions.CREATE]: z.boolean().default(true), + [Permissions.SHARE]: z.boolean().default(true), + [Permissions.SHARE_PUBLIC]: z.boolean().default(true), + }), }), }), [SystemRoles.USER]: roleSchema.extend({ @@ -185,6 +192,12 @@ export const roleDefaults = defaultRolesSchema.parse({ [Permissions.SHARE]: true, [Permissions.SHARE_PUBLIC]: true, }, + [PermissionTypes.SKILLS]: { + [Permissions.USE]: true, + [Permissions.CREATE]: true, + [Permissions.SHARE]: true, + [Permissions.SHARE_PUBLIC]: true, + }, }, }, [SystemRoles.USER]: { @@ -230,6 +243,12 @@ export const roleDefaults = defaultRolesSchema.parse({ [Permissions.SHARE]: false, [Permissions.SHARE_PUBLIC]: false, }, + [PermissionTypes.SKILLS]: { + [Permissions.USE]: true, + [Permissions.CREATE]: true, + [Permissions.SHARE]: false, + [Permissions.SHARE_PUBLIC]: false, + }, }, }, }); diff --git a/packages/data-provider/src/types/mutations.ts b/packages/data-provider/src/types/mutations.ts index 15f0cd2329..4b2c470821 100644 --- a/packages/data-provider/src/types/mutations.ts +++ b/packages/data-provider/src/types/mutations.ts @@ -13,6 +13,19 @@ import { AgentUpdateParams, } from './assistants'; import { Action, ActionMetadata } from './agents'; +import type { InfiniteData, QueryKey } from '@tanstack/react-query'; +import type { + TSkill, + TSkillFile, + TCreateSkill, + TUpdateSkillVariables, + TUpdateSkillResponse, + TDeleteSkillResponse, + TUploadSkillFileVariables, + TDeleteSkillFileVariables, + TDeleteSkillFileResponse, + TSkillListResponse, +} from './skills'; export type MutationOptions< Response, @@ -275,9 +288,44 @@ export type UpdateMemoryPermVars = UpdatePermVars; export type UpdateAgentPermVars = UpdatePermVars; export type UpdatePeoplePickerPermVars = UpdatePermVars; export type UpdateMCPServersPermVars = UpdatePermVars; +export type UpdateSkillPermVars = UpdatePermVars; export type UpdatePermResponse = r.TRole; +/* Skill mutations */ + +/** + * Cache entries that can appear under the `[QueryKeys.skills, ...]` key prefix. + * Flat responses come from `useListSkillsQuery`; infinite responses come from + * `useSkillsInfiniteQuery`. The context carries both shapes for rollback. + */ +export type TSkillCacheEntry = TSkillListResponse | InfiniteData | undefined; + +export type TUpdateSkillContext = + | { + previousSkill?: TSkill; + previousListSnapshots?: Array<[QueryKey, TSkillCacheEntry]>; + userContext?: unknown; + } + | undefined; + +export type CreateSkillOptions = MutationOptions; + +export type UpdateSkillOptions = MutationOptions< + TUpdateSkillResponse, + TUpdateSkillVariables, + TUpdateSkillContext +>; + +export type DeleteSkillOptions = MutationOptions; + +export type UploadSkillFileOptions = MutationOptions; + +export type DeleteSkillFileOptions = MutationOptions< + TDeleteSkillFileResponse, + TDeleteSkillFileVariables +>; + export type UpdatePromptPermOptions = MutationOptions< UpdatePermResponse, UpdatePromptPermVars, diff --git a/packages/data-provider/src/types/skills.ts b/packages/data-provider/src/types/skills.ts new file mode 100644 index 0000000000..8e445d34b3 --- /dev/null +++ b/packages/data-provider/src/types/skills.ts @@ -0,0 +1,219 @@ +import type { FileSources } from './files'; + +/** + * Source of a skill — where its canonical definition came from. + * `inline` means the skill was authored directly in LibreChat. + * `github` / `notion` are reserved for future sync integrations. + */ +export type SkillSource = 'inline' | 'github' | 'notion'; + +/** + * Category inferred from a skill file's top-level directory prefix. + * `script` for `scripts/...`, `reference` for `references/...`, `asset` for `assets/...`, + * everything else (including root-level files) is `other`. + */ +export type SkillFileCategory = 'script' | 'reference' | 'asset' | 'other'; + +/** + * Allowed value types inside a skill's YAML frontmatter. + * Kept strict so callers cannot slip arbitrary `unknown` payloads through the API. + */ +export type SkillFrontmatterValue = string | number | boolean | string[] | null; + +/** + * Structured YAML frontmatter for a skill. All keys are optional on the wire + * because not every skill document carries a complete frontmatter block — + * `name` and `description` live as first-class columns on `TSkill` itself, + * and frontmatter is an extension bag for additional fields like `when-to-use`, + * `allowed-tools`, etc. + */ +export type SkillFrontmatter = { + name?: string; + description?: string; +} & Record; + +/** + * Provenance metadata for skills that originated from an external source + * (e.g. a GitHub commit SHA or a Notion page id). + * + * Reserved for phase 2+ external sync — no code path currently populates this + * in phase 1, but the column exists so a future sync worker can use it + * without a schema migration. + */ +export type SkillSourceMetadata = Record; + +/** + * A non-blocking coaching hint surfaced alongside a successful create/update + * response. Unlike validation errors (which return 400 and block the write), + * warnings ride on the 2xx response so the UI can show inline feedback + * without rejecting the user's input. Example: "description is too short, + * Claude may undertrigger this skill". + */ +export type TSkillWarning = { + field: string; + code: string; + message: string; + severity: 'warning'; +}; + +/** + * API shape for a full skill (returned by GET `/api/skills/:id`). + * + * Field semantics: + * - `name` is the machine-readable kebab-case identifier Claude sees in its + * skill manifest. It's what drives triggering and must be stable across + * edits. Unique per author+tenant. + * - `displayTitle` is the human-readable UI label only. NOT sent to Claude, + * NOT part of the trigger path — purely cosmetic. + * - `description` is the "when to use this skill" sentence. Highest-leverage + * field for trigger accuracy; a short/vague one causes undertriggering. + * - `frontmatter` is the structured YAML bag minus `name`/`description` + * (those live as top-level columns). Validated strictly against a known + * key set server-side. + * - `source`/`sourceMetadata` are reserved for phase 2+ external sync and + * always `'inline'` / absent in phase 1. + */ +export type TSkill = { + _id: string; + name: string; + displayTitle?: string; + description: string; + body: string; + frontmatter?: SkillFrontmatter; + category?: string; + author: string; + authorName: string; + version: number; + source: SkillSource; + sourceMetadata?: SkillSourceMetadata; + fileCount: number; + isPublic?: boolean; + tenantId?: string; + createdAt: string; + updatedAt: string; + /** + * Present on POST/PATCH responses when the server emitted non-blocking + * coaching warnings (e.g. description too short). Never present on GET + * responses. + */ + warnings?: TSkillWarning[]; +}; + +/** + * Summary shape used in list endpoints — omits `body` and `frontmatter` to keep + * list payloads small. Callers that need the full body/frontmatter must fetch + * the detail via `GET /api/skills/:id`. + */ +export type TSkillSummary = Omit; + +/** + * Metadata for a single file bundled inside a skill. + * File content itself is fetched separately via the file download endpoint. + */ +export type TSkillFile = { + _id: string; + skillId: string; + relativePath: string; + file_id: string; + filename: string; + filepath: string; + source: FileSources; + mimeType: string; + bytes: number; + category: SkillFileCategory; + isExecutable: boolean; + author: string; + tenantId?: string; + createdAt: string; + updatedAt: string; +}; + +/** Request body for POST `/api/skills`. */ +export type TCreateSkill = { + name: string; + displayTitle?: string; + description: string; + body: string; + frontmatter?: Partial; + category?: string; +}; + +/** Partial payload for PATCH `/api/skills/:id` — all fields optional. */ +export type TUpdateSkillPayload = { + name?: string; + displayTitle?: string; + description?: string; + body?: string; + frontmatter?: Partial; + category?: string; +}; + +/** Variables passed into the update mutation: id + expectedVersion + partial payload. */ +export type TUpdateSkillVariables = { + id: string; + expectedVersion: number; + payload: TUpdateSkillPayload; +}; + +/** Response from a successful PATCH — includes the bumped version. */ +export type TUpdateSkillResponse = TSkill; + +/** Response from a 409 concurrency conflict — includes the current authoritative state. */ +export type TSkillConflictResponse = { + error: 'skill_version_conflict'; + current: TSkill; +}; + +/** Query params for GET `/api/skills` (list). */ +export type TSkillListRequest = { + category?: string; + search?: string; + limit?: number; + cursor?: string; +}; + +/** Paginated list response. `after` is the cursor to pass for the next page. */ +export type TSkillListResponse = { + skills: TSkillSummary[]; + has_more: boolean; + after: string | null; +}; + +/** Response from DELETE `/api/skills/:id`. */ +export type TDeleteSkillResponse = { + id: string; + deleted: true; +}; + +/** Response from GET `/api/skills/:id/files`. */ +export type TListSkillFilesResponse = { + files: TSkillFile[]; +}; + +/** + * Upload body for POST `/api/skills/:id/files`. + * In phase 1 the backend responds with 501; the client contract is still defined here + * so hooks are stable when the upload pipeline is wired up in phase 2. + */ +export type TUploadSkillFilePayload = { + relativePath: string; +}; + +/** Response from DELETE `/api/skills/:id/files/:relativePath`. */ +export type TDeleteSkillFileResponse = { + skillId: string; + relativePath: string; + deleted: true; +}; + +/** Variables passed into the skill file upload mutation. */ +export type TUploadSkillFileVariables = { + skillId: string; + formData: FormData; +}; + +/** Variables passed into the skill file delete mutation. */ +export type TDeleteSkillFileVariables = { + skillId: string; + relativePath: string; +}; diff --git a/packages/data-schemas/src/admin/capabilities.ts b/packages/data-schemas/src/admin/capabilities.ts index 44b9eaab4c..0284fb5586 100644 --- a/packages/data-schemas/src/admin/capabilities.ts +++ b/packages/data-schemas/src/admin/capabilities.ts @@ -35,6 +35,8 @@ export const SystemCapabilities = { MANAGE_MCP_SERVERS: 'manage:mcpservers', READ_PROMPTS: 'read:prompts', MANAGE_PROMPTS: 'manage:prompts', + READ_SKILLS: 'read:skills', + MANAGE_SKILLS: 'manage:skills', /** Reserved — not yet enforced by any middleware. */ READ_ASSISTANTS: 'read:assistants', MANAGE_ASSISTANTS: 'manage:assistants', @@ -52,6 +54,7 @@ export const CapabilityImplications: Partial = { [ResourceType.PROMPTGROUP]: SystemCapabilities.MANAGE_PROMPTS, [ResourceType.MCPSERVER]: SystemCapabilities.MANAGE_MCP_SERVERS, [ResourceType.REMOTE_AGENT]: SystemCapabilities.MANAGE_AGENTS, + [ResourceType.SKILL]: SystemCapabilities.MANAGE_SKILLS, }; /** @@ -204,6 +208,8 @@ export const CAPABILITY_CATEGORIES: CapabilityCategory[] = [ SystemCapabilities.READ_AGENTS, SystemCapabilities.MANAGE_PROMPTS, SystemCapabilities.READ_PROMPTS, + SystemCapabilities.MANAGE_SKILLS, + SystemCapabilities.READ_SKILLS, SystemCapabilities.MANAGE_ASSISTANTS, SystemCapabilities.READ_ASSISTANTS, SystemCapabilities.MANAGE_MCP_SERVERS, diff --git a/packages/data-schemas/src/methods/accessRole.spec.ts b/packages/data-schemas/src/methods/accessRole.spec.ts index 6919e6c7df..7f62c2f8cf 100644 --- a/packages/data-schemas/src/methods/accessRole.spec.ts +++ b/packages/data-schemas/src/methods/accessRole.spec.ts @@ -206,6 +206,9 @@ describe('AccessRole Model Tests', () => { AccessRoleIds.REMOTE_AGENT_EDITOR, AccessRoleIds.REMOTE_AGENT_OWNER, AccessRoleIds.REMOTE_AGENT_VIEWER, + AccessRoleIds.SKILL_EDITOR, + AccessRoleIds.SKILL_OWNER, + AccessRoleIds.SKILL_VIEWER, ].sort(), ); diff --git a/packages/data-schemas/src/methods/accessRole.ts b/packages/data-schemas/src/methods/accessRole.ts index ab2cffbad8..71cbba0c76 100644 --- a/packages/data-schemas/src/methods/accessRole.ts +++ b/packages/data-schemas/src/methods/accessRole.ts @@ -188,6 +188,27 @@ export function createAccessRoleMethods(mongoose: typeof import('mongoose')) { resourceType: ResourceType.REMOTE_AGENT, permBits: RoleBits.OWNER, }, + { + accessRoleId: AccessRoleIds.SKILL_VIEWER, + name: 'com_ui_role_viewer', + description: 'com_ui_role_viewer_desc', + resourceType: ResourceType.SKILL, + permBits: RoleBits.VIEWER, + }, + { + accessRoleId: AccessRoleIds.SKILL_EDITOR, + name: 'com_ui_role_editor', + description: 'com_ui_role_editor_desc', + resourceType: ResourceType.SKILL, + permBits: RoleBits.EDITOR, + }, + { + accessRoleId: AccessRoleIds.SKILL_OWNER, + name: 'com_ui_role_owner', + description: 'com_ui_role_owner_desc', + resourceType: ResourceType.SKILL, + permBits: RoleBits.OWNER, + }, ]; const result: Record = {}; diff --git a/packages/data-schemas/src/methods/index.ts b/packages/data-schemas/src/methods/index.ts index c2f6f7cb82..28d3effab7 100644 --- a/packages/data-schemas/src/methods/index.ts +++ b/packages/data-schemas/src/methods/index.ts @@ -45,6 +45,19 @@ import { import { createTransactionMethods, type TransactionMethods } from './transaction'; import { createSpendTokensMethods, type SpendTokensMethods } from './spendTokens'; import { createPromptMethods, type PromptMethods, type PromptDeps } from './prompt'; +import { + createSkillMethods, + type SkillMethods, + type SkillDeps, + type CreateSkillInput, + type CreateSkillResult, + type UpdateSkillInput, + type UpsertSkillFileInput, + type ListSkillsByAccessParams, + type ListSkillsByAccessResult, + type UpdateSkillResult, + type ValidationIssue, +} from './skill'; /* Tier 5 — Agent */ import { createAgentMethods, type AgentMethods, type AgentDeps } from './agent'; /* Config */ @@ -83,6 +96,7 @@ export type AllMethods = UserMethods & TransactionMethods & SpendTokensMethods & PromptMethods & + SkillMethods & AgentMethods & ConfigMethods; @@ -155,6 +169,12 @@ export function createMethods( }; const promptMethods = createPromptMethods(mongoose, promptDeps); + const skillDeps: SkillDeps = { + removeAllPermissions, + getSoleOwnedResourceIds: aclEntryMethods.getSoleOwnedResourceIds, + }; + const skillMethods = createSkillMethods(mongoose, skillDeps); + // Role methods with optional cache injection const roleDeps: RoleDeps = { getCache: deps.getCache }; const roleMethods = createRoleMethods(mongoose, roleDeps); @@ -203,6 +223,7 @@ export function createMethods( ...transactionMethods, ...spendTokensMethods, ...promptMethods, + ...skillMethods, /* Tier 5 */ ...agentMethods, /* Config */ @@ -240,6 +261,16 @@ export type { TransactionMethods, SpendTokensMethods, PromptMethods, + SkillMethods, + SkillDeps, + CreateSkillInput, + CreateSkillResult, + UpdateSkillInput, + UpsertSkillFileInput, + ListSkillsByAccessParams, + ListSkillsByAccessResult, + UpdateSkillResult, + ValidationIssue, AgentMethods, ConfigMethods, }; diff --git a/packages/data-schemas/src/methods/skill.spec.ts b/packages/data-schemas/src/methods/skill.spec.ts new file mode 100644 index 0000000000..332a7ecd5e --- /dev/null +++ b/packages/data-schemas/src/methods/skill.spec.ts @@ -0,0 +1,617 @@ +import mongoose from 'mongoose'; +import { MongoMemoryServer } from 'mongodb-memory-server'; +import { + SystemRoles, + ResourceType, + AccessRoleIds, + PrincipalType, + PermissionBits, +} from 'librechat-data-provider'; +import { createAclEntryMethods } from './aclEntry'; +import { + validateSkillName, + validateSkillDescription, + validateSkillFrontmatter, + validateRelativePath, + inferSkillFileCategory, +} from './skill'; +import { logger, createModels } from '..'; +import { createMethods } from './index'; + +logger.silent = true; + +type LeanUser = { + _id: mongoose.Types.ObjectId; + name?: string; + email: string; + role?: string; +}; + +let Skill: mongoose.Model; +let SkillFile: mongoose.Model; +let AclEntry: mongoose.Model; +let AccessRole: mongoose.Model; +let User: mongoose.Model; +let methods: ReturnType; +let aclMethods: ReturnType; +let owner: LeanUser; +let other: LeanUser; + +let mongoServer: MongoMemoryServer; + +beforeAll(async () => { + mongoServer = await MongoMemoryServer.create(); + await mongoose.connect(mongoServer.getUri()); + + createModels(mongoose); + Skill = mongoose.models.Skill; + SkillFile = mongoose.models.SkillFile; + AclEntry = mongoose.models.AclEntry; + AccessRole = mongoose.models.AccessRole; + User = mongoose.models.User; + + methods = createMethods(mongoose, { + removeAllPermissions: async ({ resourceType, resourceId }) => { + await AclEntry.deleteMany({ resourceType, resourceId }); + }, + }); + aclMethods = createAclEntryMethods(mongoose); + + await AccessRole.create({ + accessRoleId: AccessRoleIds.SKILL_OWNER, + name: 'Owner', + description: 'Full control over skills', + resourceType: ResourceType.SKILL, + permBits: + PermissionBits.VIEW | PermissionBits.EDIT | PermissionBits.DELETE | PermissionBits.SHARE, + }); + await AccessRole.create({ + accessRoleId: AccessRoleIds.SKILL_EDITOR, + name: 'Editor', + description: 'Can view and edit skills', + resourceType: ResourceType.SKILL, + permBits: PermissionBits.VIEW | PermissionBits.EDIT, + }); + await AccessRole.create({ + accessRoleId: AccessRoleIds.SKILL_VIEWER, + name: 'Viewer', + description: 'Can view skills', + resourceType: ResourceType.SKILL, + permBits: PermissionBits.VIEW, + }); + + owner = ( + await User.create({ + name: 'Skill Owner', + email: 'skill-owner@example.com', + role: SystemRoles.USER, + }) + ).toObject() as unknown as LeanUser; + other = ( + await User.create({ + name: 'Other User', + email: 'skill-other@example.com', + role: SystemRoles.USER, + }) + ).toObject() as unknown as LeanUser; +}); + +afterAll(async () => { + await mongoose.disconnect(); + await mongoServer.stop(); +}); + +afterEach(async () => { + await Skill.deleteMany({}); + await SkillFile.deleteMany({}); + await AclEntry.deleteMany({}); +}); + +async function grantOwner(resourceId: mongoose.Types.ObjectId | string) { + const role = (await AccessRole.findOne({ accessRoleId: AccessRoleIds.SKILL_OWNER }).lean()) as { + _id: mongoose.Types.ObjectId; + permBits: number; + } | null; + if (!role) throw new Error('SKILL_OWNER role not seeded'); + return aclMethods.grantPermission( + PrincipalType.USER, + owner._id, + ResourceType.SKILL, + resourceId, + role.permBits, + owner._id, + undefined, + role._id, + ); +} + +function makeSkillInput(overrides: Record = {}) { + return { + name: 'demo-skill', + description: 'A small demo skill used in tests.', + body: '# Demo\n\nBody content', + frontmatter: { name: 'demo-skill', description: 'A small demo skill used in tests.' }, + author: owner._id, + authorName: owner.name ?? 'Skill Owner', + ...overrides, + }; +} + +describe('skill validation helpers', () => { + it('rejects names starting with reserved brand prefixes', () => { + expect(validateSkillName('anthropic-helper').some((i) => i.code === 'RESERVED_PREFIX')).toBe( + true, + ); + expect(validateSkillName('claude-helper').some((i) => i.code === 'RESERVED_PREFIX')).toBe(true); + }); + + it('allows names that merely contain reserved words as substrings', () => { + expect(validateSkillName('research-anthropic-helper')).toEqual([]); + expect(validateSkillName('about-claude')).toEqual([]); + }); + + it('rejects reserved CLI command names', () => { + for (const word of ['help', 'clear', 'compact', 'model', 'exit', 'quit', 'settings']) { + expect(validateSkillName(word).some((i) => i.code === 'RESERVED_WORD')).toBe(true); + } + }); + + it('rejects non-kebab-case names', () => { + expect(validateSkillName('My Skill').some((i) => i.code === 'INVALID_FORMAT')).toBe(true); + expect(validateSkillName('UPPER').some((i) => i.code === 'INVALID_FORMAT')).toBe(true); + }); + + it('rejects names longer than 64 chars', () => { + const long = 'a'.repeat(65); + expect(validateSkillName(long).some((i) => i.code === 'TOO_LONG')).toBe(true); + }); + + it('accepts valid kebab-case names', () => { + expect(validateSkillName('my-skill-1')).toEqual([]); + }); + + it('requires description', () => { + expect(validateSkillDescription('').some((i) => i.code === 'REQUIRED')).toBe(true); + expect(validateSkillDescription(' ').some((i) => i.code === 'REQUIRED')).toBe(true); + }); + + it('rejects description over 1024 chars', () => { + expect(validateSkillDescription('x'.repeat(1025)).some((i) => i.code === 'TOO_LONG')).toBe( + true, + ); + }); + + it('emits a warning (not an error) for short descriptions', () => { + const issues = validateSkillDescription('too short'); + const warnings = issues.filter((i) => i.severity === 'warning' && i.code === 'TOO_SHORT'); + const errors = issues.filter((i) => i.severity !== 'warning'); + expect(warnings.length).toBe(1); + expect(errors.length).toBe(0); + }); + + it('does not warn for descriptions at or above the threshold', () => { + expect( + validateSkillDescription('This description is comfortably over the threshold length.'), + ).toEqual([]); + }); + + it('rejects path traversal', () => { + expect(validateRelativePath('../evil').some((i) => i.code === 'TRAVERSAL')).toBe(true); + expect(validateRelativePath('scripts/../../etc').some((i) => i.code === 'TRAVERSAL')).toBe( + true, + ); + }); + + it('rejects absolute paths', () => { + expect(validateRelativePath('/abs/path').some((i) => i.code === 'ABSOLUTE_PATH')).toBe(true); + }); + + it('rejects SKILL.md explicitly', () => { + expect(validateRelativePath('SKILL.md').some((i) => i.code === 'RESERVED')).toBe(true); + }); + + it('accepts well-formed relative paths', () => { + expect(validateRelativePath('scripts/parse.sh')).toEqual([]); + expect(validateRelativePath('references/schema.md')).toEqual([]); + }); + + it('infers category from top-level prefix', () => { + expect(inferSkillFileCategory('scripts/parse.sh')).toBe('script'); + expect(inferSkillFileCategory('references/notes.md')).toBe('reference'); + expect(inferSkillFileCategory('assets/image.png')).toBe('asset'); + expect(inferSkillFileCategory('readme.txt')).toBe('other'); + }); + + describe('validateSkillFrontmatter', () => { + it('accepts an undefined or empty frontmatter', () => { + expect(validateSkillFrontmatter(undefined)).toEqual([]); + expect(validateSkillFrontmatter(null)).toEqual([]); + expect(validateSkillFrontmatter({})).toEqual([]); + }); + + it('rejects non-object frontmatter', () => { + expect(validateSkillFrontmatter('not an object').some((i) => i.code === 'INVALID_TYPE')).toBe( + true, + ); + expect(validateSkillFrontmatter([]).some((i) => i.code === 'INVALID_TYPE')).toBe(true); + }); + + it('rejects unknown keys in strict mode', () => { + const issues = validateSkillFrontmatter({ 'not-a-real-key': 'value' }); + expect(issues.some((i) => i.code === 'UNKNOWN_KEY')).toBe(true); + }); + + it('accepts known keys with correct types', () => { + expect( + validateSkillFrontmatter({ + name: 'demo-skill', + description: 'A demo skill', + 'when-to-use': 'When the user needs a demo.', + 'allowed-tools': ['read', 'write'], + 'user-invocable': true, + effort: 5, + version: '1.0.0', + hooks: { 'pre-run': 'echo hi' }, + }), + ).toEqual([]); + }); + + it('rejects known keys with wrong types', () => { + expect( + validateSkillFrontmatter({ 'user-invocable': 'yes' }).some( + (i) => i.code === 'INVALID_TYPE', + ), + ).toBe(true); + expect( + validateSkillFrontmatter({ 'allowed-tools': [1, 2, 3] }).some( + (i) => i.code === 'INVALID_TYPE', + ), + ).toBe(true); + }); + + it('rejects hooks object with excessive nesting', () => { + const deep = { a: { b: { c: { d: { e: { f: 'too deep' } } } } } }; + expect( + validateSkillFrontmatter({ hooks: deep }).some((i) => i.code === 'INVALID_SHAPE'), + ).toBe(true); + }); + }); +}); + +describe('Skill CRUD methods', () => { + it('creates a skill with version 1 and default fileCount 0', async () => { + const { skill, warnings } = await methods.createSkill(makeSkillInput()); + expect(skill.name).toBe('demo-skill'); + expect(skill.version).toBe(1); + expect(skill.fileCount).toBe(0); + expect(skill.source).toBe('inline'); + expect(skill.author.toString()).toBe(owner._id.toString()); + expect(warnings).toEqual([]); + }); + + it('emits a too-short warning when the description is under 20 chars', async () => { + const { skill, warnings } = await methods.createSkill( + makeSkillInput({ name: 'short-desc-skill', description: 'Too short.' }), + ); + expect(skill._id).toBeDefined(); + expect(warnings).toEqual([ + expect.objectContaining({ + field: 'description', + code: 'TOO_SHORT', + severity: 'warning', + }), + ]); + }); + + it('rejects frontmatter with unknown keys (strict mode)', async () => { + await expect( + methods.createSkill( + makeSkillInput({ + name: 'strict-frontmatter', + frontmatter: { 'bogus-key': 'nope' }, + }), + ), + ).rejects.toMatchObject({ code: 'SKILL_VALIDATION_FAILED' }); + }); + + it('rejects names that start with reserved brand prefixes', async () => { + await expect( + methods.createSkill(makeSkillInput({ name: 'anthropic-helper' })), + ).rejects.toMatchObject({ code: 'SKILL_VALIDATION_FAILED' }); + await expect( + methods.createSkill(makeSkillInput({ name: 'claude-helper' })), + ).rejects.toMatchObject({ code: 'SKILL_VALIDATION_FAILED' }); + }); + + it('allows names that merely contain the reserved words as substrings', async () => { + const { skill } = await methods.createSkill( + makeSkillInput({ name: 'research-anthropic-helper' }), + ); + expect(skill.name).toBe('research-anthropic-helper'); + }); + + it('rejects reserved CLI command names', async () => { + await expect(methods.createSkill(makeSkillInput({ name: 'help' }))).rejects.toMatchObject({ + code: 'SKILL_VALIDATION_FAILED', + }); + await expect(methods.createSkill(makeSkillInput({ name: 'settings' }))).rejects.toMatchObject({ + code: 'SKILL_VALIDATION_FAILED', + }); + }); + + it('enforces name uniqueness per author', async () => { + await methods.createSkill(makeSkillInput()); + await expect(methods.createSkill(makeSkillInput())).rejects.toBeDefined(); + }); + + it('throws with validation issues on bad input', async () => { + await expect(methods.createSkill(makeSkillInput({ name: 'BAD NAME' }))).rejects.toMatchObject({ + code: 'SKILL_VALIDATION_FAILED', + }); + }); + + it('optimistic concurrency — stale version returns conflict with current state', async () => { + const { skill } = await methods.createSkill(makeSkillInput()); + const id = skill._id.toString(); + const first = await methods.updateSkill({ + id, + expectedVersion: 1, + update: { description: 'First update description (long enough).' }, + }); + expect(first.status).toBe('updated'); + if (first.status === 'updated') { + expect(first.skill.version).toBe(2); + } + + const stale = await methods.updateSkill({ + id, + expectedVersion: 1, + update: { description: 'Second update description (long enough).' }, + }); + expect(stale.status).toBe('conflict'); + if (stale.status === 'conflict') { + expect(stale.current.version).toBe(2); + expect(stale.current.description).toBe('First update description (long enough).'); + } + }); + + it('deleteSkill cascades ACL entries and skill files', async () => { + const { skill } = await methods.createSkill(makeSkillInput()); + await grantOwner(skill._id); + await methods.upsertSkillFile({ + skillId: skill._id, + relativePath: 'scripts/parse.sh', + file_id: 'file-123', + filename: 'parse.sh', + filepath: '/tmp/parse.sh', + source: 'local', + mimeType: 'text/x-shellscript', + bytes: 42, + author: owner._id, + }); + + expect(await AclEntry.countDocuments({ resourceId: skill._id })).toBe(1); + expect(await SkillFile.countDocuments({ skillId: skill._id })).toBe(1); + + const res = await methods.deleteSkill(skill._id.toString()); + expect(res.deleted).toBe(true); + expect(await AclEntry.countDocuments({ resourceId: skill._id })).toBe(0); + expect(await SkillFile.countDocuments({ skillId: skill._id })).toBe(0); + }); + + it('listSkillsByAccess returns only accessible skills and paginates by cursor', async () => { + const ids: mongoose.Types.ObjectId[] = []; + for (let i = 0; i < 3; i++) { + const { skill } = await methods.createSkill( + makeSkillInput({ + name: `demo-skill-${i}`, + description: `A demo skill used to test pagination (${i}).`, + }), + ); + ids.push(skill._id); + await new Promise((r) => setTimeout(r, 5)); + } + + const page1 = await methods.listSkillsByAccess({ accessibleIds: ids, limit: 2 }); + expect(page1.skills.length).toBe(2); + expect(page1.has_more).toBe(true); + expect(page1.after).not.toBeNull(); + + const page2 = await methods.listSkillsByAccess({ + accessibleIds: ids, + limit: 2, + cursor: page1.after, + }); + expect(page2.skills.length).toBe(1); + expect(page2.has_more).toBe(false); + }); + + it('listSkillsByAccess filters by category and search', async () => { + const { skill: a } = await methods.createSkill( + makeSkillInput({ name: 'alpha-skill', category: 'tools' }), + ); + await methods.createSkill( + makeSkillInput({ + name: 'beta-skill', + category: 'other', + description: 'A beta description that is long enough.', + }), + ); + const ids = await Skill.find().distinct('_id'); + + const byCat = await methods.listSkillsByAccess({ + accessibleIds: ids as unknown as mongoose.Types.ObjectId[], + limit: 10, + category: 'tools', + }); + expect(byCat.skills.length).toBe(1); + expect(byCat.skills[0].name).toBe(a.name); + + const bySearch = await methods.listSkillsByAccess({ + accessibleIds: ids as unknown as mongoose.Types.ObjectId[], + limit: 10, + search: 'beta', + }); + expect(bySearch.skills.length).toBe(1); + expect(bySearch.skills[0].name).toBe('beta-skill'); + }); +}); + +describe('SkillFile methods', () => { + it('upsertSkillFile bumps parent skill version and updates fileCount', async () => { + const { skill } = await methods.createSkill(makeSkillInput()); + expect(skill.version).toBe(1); + expect(skill.fileCount).toBe(0); + + await methods.upsertSkillFile({ + skillId: skill._id, + relativePath: 'scripts/parse.sh', + file_id: 'file-abc', + filename: 'parse.sh', + filepath: '/tmp/parse.sh', + source: 'local', + mimeType: 'text/plain', + bytes: 10, + author: owner._id, + }); + + const updated = await methods.getSkillById(skill._id); + expect(updated?.version).toBe(2); + expect(updated?.fileCount).toBe(1); + }); + + it('upsertSkillFile replaces an existing row and bumps version again', async () => { + const { skill } = await methods.createSkill(makeSkillInput()); + await methods.upsertSkillFile({ + skillId: skill._id, + relativePath: 'scripts/parse.sh', + file_id: 'file-1', + filename: 'parse.sh', + filepath: '/tmp/v1', + source: 'local', + mimeType: 'text/plain', + bytes: 10, + author: owner._id, + }); + await methods.upsertSkillFile({ + skillId: skill._id, + relativePath: 'scripts/parse.sh', + file_id: 'file-2', + filename: 'parse.sh', + filepath: '/tmp/v2', + source: 'local', + mimeType: 'text/plain', + bytes: 20, + author: owner._id, + }); + const after = await methods.getSkillById(skill._id); + expect(after?.fileCount).toBe(1); + expect(after?.version).toBe(3); + const files = await methods.listSkillFiles(skill._id); + expect(files.length).toBe(1); + expect(files[0].file_id).toBe('file-2'); + }); + + it('deleteSkillFile recounts and bumps version', async () => { + const { skill } = await methods.createSkill(makeSkillInput()); + await methods.upsertSkillFile({ + skillId: skill._id, + relativePath: 'scripts/a.sh', + file_id: 'f1', + filename: 'a.sh', + filepath: '/a', + source: 'local', + mimeType: 'text/plain', + bytes: 1, + author: owner._id, + }); + await methods.upsertSkillFile({ + skillId: skill._id, + relativePath: 'scripts/b.sh', + file_id: 'f2', + filename: 'b.sh', + filepath: '/b', + source: 'local', + mimeType: 'text/plain', + bytes: 1, + author: owner._id, + }); + const res = await methods.deleteSkillFile(skill._id, 'scripts/a.sh'); + expect(res.deleted).toBe(true); + const after = await methods.getSkillById(skill._id); + expect(after?.fileCount).toBe(1); + expect(after?.version).toBe(4); + }); + + it('rejects invalid paths via upsertSkillFile', async () => { + const { skill } = await methods.createSkill(makeSkillInput()); + await expect( + methods.upsertSkillFile({ + skillId: skill._id, + relativePath: '../evil', + file_id: 'f1', + filename: 'evil', + filepath: '/x', + source: 'local', + mimeType: 'text/plain', + bytes: 1, + author: owner._id, + }), + ).rejects.toMatchObject({ code: 'SKILL_FILE_VALIDATION_FAILED' }); + }); +}); + +describe('deleteUserSkills', () => { + it('removes sole-owned skills only', async () => { + const { skill: mine } = await methods.createSkill(makeSkillInput({ name: 'mine' })); + await grantOwner(mine._id); + + // Create a second skill owned by someone else + const shared = await Skill.create({ + name: 'shared', + description: 'shared skill', + body: '', + frontmatter: {}, + category: '', + author: other._id, + authorName: other.name ?? 'Other', + version: 1, + source: 'inline', + fileCount: 0, + }); + const sharedId = (shared.toObject() as { _id: mongoose.Types.ObjectId })._id; + // Owner user has viewer access to shared — so it's NOT sole-owned + const viewerRole = (await AccessRole.findOne({ + accessRoleId: AccessRoleIds.SKILL_VIEWER, + }).lean()) as { _id: mongoose.Types.ObjectId; permBits: number } | null; + await aclMethods.grantPermission( + PrincipalType.USER, + owner._id, + ResourceType.SKILL, + sharedId, + viewerRole!.permBits, + other._id, + undefined, + viewerRole!._id, + ); + // And other is owner of the shared skill + const ownerRole = (await AccessRole.findOne({ + accessRoleId: AccessRoleIds.SKILL_OWNER, + }).lean()) as { _id: mongoose.Types.ObjectId; permBits: number } | null; + await aclMethods.grantPermission( + PrincipalType.USER, + other._id, + ResourceType.SKILL, + sharedId, + ownerRole!.permBits, + other._id, + undefined, + ownerRole!._id, + ); + + const deleted = await methods.deleteUserSkills(owner._id as mongoose.Types.ObjectId); + expect(deleted).toBe(1); + expect(await Skill.countDocuments()).toBe(1); + expect(await Skill.countDocuments({ _id: sharedId })).toBe(1); + }); +}); diff --git a/packages/data-schemas/src/methods/skill.ts b/packages/data-schemas/src/methods/skill.ts new file mode 100644 index 0000000000..9a30ec5fc7 --- /dev/null +++ b/packages/data-schemas/src/methods/skill.ts @@ -0,0 +1,883 @@ +import { ResourceType } from 'librechat-data-provider'; +import type { Model, Types, FilterQuery } from 'mongoose'; +import type { + ISkill, + ISkillDocument, + ISkillFile, + ISkillFileDocument, + ISkillSummary, +} from '~/types/skill'; +import { isValidObjectIdString } from '~/utils/objectId'; +import { escapeRegExp } from '~/utils/string'; +import logger from '~/config/winston'; + +/** ---------- Validation helpers (pure) ---------- */ + +/** + * A single validation issue emitted by a skill validator. Most issues are + * errors and block the mutation; some are warnings (e.g. "description is + * awfully short, Claude may undertrigger the skill") that surface inline + * coaching without rejecting the request. + */ +export type ValidationIssue = { + field: string; + code: string; + message: string; + /** + * Defaults to `'error'` when omitted. Errors cause `createSkill` / + * `updateSkill` to throw with code `SKILL_VALIDATION_FAILED`; warnings + * are surfaced on successful responses so the UI can show inline feedback. + */ + severity?: 'error' | 'warning'; +}; + +/** Partition an issue list into blocking errors and non-blocking warnings. */ +export function partitionIssues(issues: ValidationIssue[]): { + errors: ValidationIssue[]; + warnings: ValidationIssue[]; +} { + const errors: ValidationIssue[] = []; + const warnings: ValidationIssue[] = []; + for (const issue of issues) { + if (issue.severity === 'warning') { + warnings.push(issue); + } else { + errors.push(issue); + } + } + return { errors, warnings }; +} + +const SKILL_NAME_MAX = 64; +const SKILL_DESCRIPTION_MAX = 1024; +const SKILL_DESCRIPTION_SHORT_THRESHOLD = 20; +const SKILL_DISPLAY_TITLE_MAX = 128; +const SKILL_BODY_MAX = 100_000; +const SKILL_FILE_PATH_MAX = 500; +const SKILL_NAME_PATTERN = /^[a-z0-9][a-z0-9-]*$/; +const RELATIVE_PATH_CHARS = /^[a-zA-Z0-9._\-/]+$/; + +/** + * Brand namespaces reserved for Anthropic-published skills and first-party + * bundles. Matched as prefixes, so `anthropic-helper` is rejected but + * `research-anthropic-helper` is fine. + */ +const RESERVED_NAME_PREFIXES = ['anthropic-', 'claude-']; + +/** + * Slash-command names that collide with LibreChat / Claude Code CLI commands. + * A skill with one of these names would shadow a real command in any + * slash-command UI. Matched exactly (not as prefix). + */ +const RESERVED_NAME_WORDS = new Set([ + 'help', + 'clear', + 'compact', + 'model', + 'exit', + 'quit', + 'settings', + 'anthropic', + 'claude', +]); + +export function validateSkillName(name: unknown): ValidationIssue[] { + const issues: ValidationIssue[] = []; + if (typeof name !== 'string' || name.length === 0) { + issues.push({ field: 'name', code: 'REQUIRED', message: 'Name is required' }); + return issues; + } + if (name.length > SKILL_NAME_MAX) { + issues.push({ + field: 'name', + code: 'TOO_LONG', + message: `Name must be ${SKILL_NAME_MAX} characters or less`, + }); + } + if (!SKILL_NAME_PATTERN.test(name)) { + issues.push({ + field: 'name', + code: 'INVALID_FORMAT', + message: + 'Name must be kebab-case: start with a lowercase letter or digit and contain only lowercase letters, digits, and hyphens', + }); + } + const lowered = name.toLowerCase(); + if (RESERVED_NAME_PREFIXES.some((prefix) => lowered.startsWith(prefix))) { + issues.push({ + field: 'name', + code: 'RESERVED_PREFIX', + message: `Name cannot start with ${RESERVED_NAME_PREFIXES.map((p) => `"${p}"`).join(' or ')}`, + }); + } + if (RESERVED_NAME_WORDS.has(lowered)) { + issues.push({ + field: 'name', + code: 'RESERVED_WORD', + message: `"${name}" is a reserved name`, + }); + } + return issues; +} + +export function validateSkillDescription(description: unknown): ValidationIssue[] { + const issues: ValidationIssue[] = []; + if (typeof description !== 'string' || description.trim().length === 0) { + issues.push({ + field: 'description', + code: 'REQUIRED', + message: 'Description is required', + }); + return issues; + } + if (description.length > SKILL_DESCRIPTION_MAX) { + issues.push({ + field: 'description', + code: 'TOO_LONG', + message: `Description must be ${SKILL_DESCRIPTION_MAX} characters or less`, + }); + } + if (description.trim().length < SKILL_DESCRIPTION_SHORT_THRESHOLD) { + issues.push({ + field: 'description', + code: 'TOO_SHORT', + severity: 'warning', + message: + 'Short descriptions may cause Claude to miss triggering opportunities — aim for a concrete "when to use this skill" sentence.', + }); + } + return issues; +} + +export function validateSkillBody(body: unknown): ValidationIssue[] { + const issues: ValidationIssue[] = []; + if (body !== undefined && typeof body !== 'string') { + issues.push({ field: 'body', code: 'INVALID_TYPE', message: 'Body must be a string' }); + return issues; + } + if (typeof body === 'string' && body.length > SKILL_BODY_MAX) { + issues.push({ + field: 'body', + code: 'TOO_LONG', + message: `Body must be ${SKILL_BODY_MAX} characters or less`, + }); + } + return issues; +} + +export function validateSkillDisplayTitle(displayTitle: unknown): ValidationIssue[] { + if (displayTitle === undefined || displayTitle === null) { + return []; + } + if (typeof displayTitle !== 'string') { + return [ + { field: 'displayTitle', code: 'INVALID_TYPE', message: 'Display title must be a string' }, + ]; + } + if (displayTitle.length > SKILL_DISPLAY_TITLE_MAX) { + return [ + { + field: 'displayTitle', + code: 'TOO_LONG', + message: `Display title must be ${SKILL_DISPLAY_TITLE_MAX} characters or less`, + }, + ]; + } + return []; +} + +/** + * 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 + * spec plus the fields LibreChat needs to pass through (`name`/`description` + * are duplicated from the top-level columns because real `SKILL.md` files + * include them in their frontmatter block). + */ +const ALLOWED_FRONTMATTER_KEYS = new Set([ + 'name', + 'description', + 'when-to-use', + 'allowed-tools', + 'arguments', + 'argument-hint', + 'user-invocable', + 'disable-model-invocation', + 'model', + 'effort', + 'context', + 'agent', + 'paths', + 'shell', + 'hooks', + 'version', + 'metadata', +]); + +const FRONTMATTER_MAX_STRING = 2000; +const FRONTMATTER_MAX_ARRAY = 100; +const FRONTMATTER_MAX_DEPTH = 4; + +type FrontmatterKind = 'string' | 'number' | 'boolean' | 'stringArray'; + +const FRONTMATTER_KIND: Record = { + name: 'string', + description: 'string', + 'when-to-use': 'string', + 'allowed-tools': ['string', 'stringArray'], + arguments: ['string', 'stringArray'], + 'argument-hint': 'string', + 'user-invocable': 'boolean', + 'disable-model-invocation': 'boolean', + model: 'string', + effort: ['string', 'number'], + context: 'string', + agent: 'string', + paths: ['string', 'stringArray'], + shell: 'string', + version: 'string', +}; + +function isPlainObject(value: unknown): value is Record { + return typeof value === 'object' && value !== null && !Array.isArray(value); +} + +function isStringArray(value: unknown): value is string[] { + return ( + Array.isArray(value) && + value.length <= FRONTMATTER_MAX_ARRAY && + value.every((v) => typeof v === 'string' && v.length <= FRONTMATTER_MAX_STRING) + ); +} + +function matchesKind(value: unknown, kind: FrontmatterKind): boolean { + if (kind === 'string') { + return typeof value === 'string' && value.length <= FRONTMATTER_MAX_STRING; + } + if (kind === 'number') { + return typeof value === 'number' && Number.isFinite(value); + } + if (kind === 'boolean') { + return typeof value === 'boolean'; + } + return isStringArray(value); +} + +/** + * Shallow structural sanity check for `hooks`/`metadata` objects. We don't + * know their full schema yet, so we just verify they are plain objects with + * JSON-serializable leaf values up to a max depth — enough to block pathological + * payloads without constraining legitimate frontmatter extensions. + */ +function isJsonSafe(value: unknown, depth: number): boolean { + if (depth > FRONTMATTER_MAX_DEPTH) { + return false; + } + if (value === null) return true; + const t = typeof value; + if (t === 'string') return (value as string).length <= FRONTMATTER_MAX_STRING; + if (t === 'number') return Number.isFinite(value); + if (t === 'boolean') return true; + if (Array.isArray(value)) { + if (value.length > FRONTMATTER_MAX_ARRAY) return false; + return value.every((v) => isJsonSafe(v, depth + 1)); + } + if (isPlainObject(value)) { + return Object.values(value).every((v) => isJsonSafe(v, depth + 1)); + } + return false; +} + +/** + * Validate a skill's structured YAML frontmatter. Strict mode: unknown keys + * are rejected so any expansion of the allowed set is an intentional code + * change. Known keys are type-checked against `FRONTMATTER_KIND`; `hooks` and + * `metadata` fall back to a shallow JSON-safety check because their full + * schemas live outside this module. + */ +export function validateSkillFrontmatter(frontmatter: unknown): ValidationIssue[] { + if (frontmatter === undefined || frontmatter === null) { + return []; + } + if (!isPlainObject(frontmatter)) { + return [ + { + field: 'frontmatter', + code: 'INVALID_TYPE', + message: 'Frontmatter must be a plain object', + }, + ]; + } + + 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 ')}`, + }); + } + } + return issues; +} + +export function validateRelativePath(relativePath: unknown): ValidationIssue[] { + const issues: ValidationIssue[] = []; + if (typeof relativePath !== 'string' || relativePath.length === 0) { + issues.push({ + field: 'relativePath', + code: 'REQUIRED', + message: 'Relative path is required', + }); + return issues; + } + if (relativePath.length > SKILL_FILE_PATH_MAX) { + issues.push({ + field: 'relativePath', + code: 'TOO_LONG', + message: `Relative path must be ${SKILL_FILE_PATH_MAX} characters or less`, + }); + } + if (relativePath.startsWith('/') || relativePath.startsWith('\\')) { + issues.push({ + field: 'relativePath', + code: 'ABSOLUTE_PATH', + message: 'Relative path must not start with a slash', + }); + } + if (!RELATIVE_PATH_CHARS.test(relativePath)) { + issues.push({ + field: 'relativePath', + code: 'INVALID_CHARS', + message: 'Relative path contains invalid characters', + }); + } + const segments = relativePath.split('/'); + if (segments.some((s) => s === '' || s === '.' || s === '..')) { + issues.push({ + field: 'relativePath', + code: 'TRAVERSAL', + message: 'Relative path cannot contain empty segments or "." / ".."', + }); + } + if (relativePath === 'SKILL.md' || segments[0] === 'SKILL.md') { + issues.push({ + field: 'relativePath', + code: 'RESERVED', + message: 'SKILL.md is managed via the skill body, not file uploads', + }); + } + return issues; +} + +export function inferSkillFileCategory( + relativePath: string, +): 'script' | 'reference' | 'asset' | 'other' { + const [top] = relativePath.split('/'); + if (top === 'scripts') return 'script'; + if (top === 'references') return 'reference'; + if (top === 'assets') return 'asset'; + return 'other'; +} + +/** ---------- Method factory ---------- */ + +export interface SkillDeps { + /** Removes all ACL entries for a resource. Injected from PermissionService. */ + removeAllPermissions: (params: { resourceType: string; resourceId: unknown }) => Promise; + /** Returns resource IDs solely owned by the given user. From createAclEntryMethods. */ + getSoleOwnedResourceIds: ( + userObjectId: Types.ObjectId, + resourceTypes: string | string[], + ) => Promise; +} + +export type CreateSkillInput = { + name: string; + displayTitle?: string; + description: string; + body?: string; + frontmatter?: Record; + category?: string; + author: Types.ObjectId; + authorName: string; + source?: 'inline' | 'github' | 'notion'; + sourceMetadata?: Record; + tenantId?: string; +}; + +export type UpdateSkillInput = { + name?: string; + displayTitle?: string; + description?: string; + body?: string; + frontmatter?: Record; + category?: string; +}; + +export type UpsertSkillFileInput = { + skillId: Types.ObjectId | string; + relativePath: string; + file_id: string; + filename: string; + filepath: string; + source: string; + mimeType: string; + bytes: number; + isExecutable?: boolean; + author: Types.ObjectId; + tenantId?: string; +}; + +export type ListSkillsByAccessParams = { + accessibleIds: Types.ObjectId[]; + category?: string; + search?: string; + limit: number; + cursor?: string | null; +}; + +export type ListSkillsByAccessResult = { + /** + * Summary rows — `body` and `frontmatter` are intentionally omitted at the + * query projection layer to keep list payloads small. Callers that need the + * full document must fetch the detail via `getSkillById`. + */ + skills: Array; + has_more: boolean; + after: string | null; +}; + +export type UpdateSkillResult = + | { + status: 'updated'; + skill: ISkill & { _id: Types.ObjectId }; + warnings: ValidationIssue[]; + } + | { status: 'conflict'; current: ISkill & { _id: Types.ObjectId } } + | { status: 'not_found' }; + +export type CreateSkillResult = { + skill: ISkill & { _id: Types.ObjectId }; + warnings: ValidationIssue[]; +}; + +export function createSkillMethods(mongoose: typeof import('mongoose'), deps: SkillDeps) { + const { ObjectId } = mongoose.Types; + + function buildSkillFilter( + params: Pick, + ): FilterQuery { + const filter: FilterQuery = { + _id: { $in: params.accessibleIds }, + }; + if (params.category && params.category.length > 0) { + filter.category = params.category; + } + if (params.search && params.search.length > 0) { + const rx = new RegExp(escapeRegExp(params.search), 'i'); + filter.$or = [{ name: rx }, { description: rx }, { displayTitle: rx }]; + } + return filter; + } + + function decodeCursor( + cursor: string | null | undefined, + ): { updatedAt: Date; _id: Types.ObjectId } | null { + if (!cursor || cursor === 'undefined' || cursor === 'null') { + return null; + } + try { + const decoded = JSON.parse(Buffer.from(cursor, 'base64').toString('utf8')) as { + updatedAt?: string; + _id?: string; + }; + if ( + !decoded.updatedAt || + !decoded._id || + Number.isNaN(new Date(decoded.updatedAt).getTime()) || + !isValidObjectIdString(decoded._id) + ) { + return null; + } + return { updatedAt: new Date(decoded.updatedAt), _id: new ObjectId(decoded._id) }; + } catch (error) { + logger.warn(`[skill.decodeCursor] Invalid cursor: ${(error as Error).message}`); + return null; + } + } + + function encodeCursor(row: { updatedAt: Date; _id: Types.ObjectId }): string { + return Buffer.from( + JSON.stringify({ updatedAt: row.updatedAt.toISOString(), _id: row._id.toString() }), + ).toString('base64'); + } + + async function createSkill(data: CreateSkillInput): Promise { + const issues: ValidationIssue[] = [ + ...validateSkillName(data.name), + ...validateSkillDescription(data.description), + ...validateSkillBody(data.body), + ...validateSkillDisplayTitle(data.displayTitle), + ...validateSkillFrontmatter(data.frontmatter), + ]; + const { errors, warnings } = partitionIssues(issues); + if (errors.length > 0) { + const error = new Error('Skill validation failed'); + (error as Error & { issues?: ValidationIssue[]; code?: string }).issues = errors; + (error as Error & { code?: string }).code = 'SKILL_VALIDATION_FAILED'; + throw error; + } + + const Skill = mongoose.models.Skill as Model; + + // Application-level uniqueness check on (name, author, tenantId). + // The unique index in the schema is the persistent guarantee, but Mongoose + // creates indexes asynchronously and tests can race ahead of index creation, + // so we also enforce it here for deterministic behavior and a clean error. + const existing = await Skill.findOne({ + name: data.name, + author: data.author, + tenantId: data.tenantId ?? null, + }) + .select('_id') + .lean(); + if (existing) { + const error = new Error(`A skill with name "${data.name}" already exists for this author`); + (error as Error & { code?: string | number }).code = 11000; + throw error; + } + + const doc = await Skill.create({ + name: data.name, + displayTitle: data.displayTitle, + description: data.description, + body: data.body ?? '', + frontmatter: data.frontmatter ?? {}, + category: data.category ?? '', + author: data.author, + authorName: data.authorName, + version: 1, + source: data.source ?? 'inline', + sourceMetadata: data.sourceMetadata, + fileCount: 0, + tenantId: data.tenantId, + }); + return { + skill: doc.toObject() as unknown as ISkill & { _id: Types.ObjectId }, + warnings, + }; + } + + async function getSkillById( + id: string | Types.ObjectId, + ): Promise<(ISkill & { _id: Types.ObjectId }) | null> { + if (typeof id === 'string' && !isValidObjectIdString(id)) { + return null; + } + const Skill = mongoose.models.Skill as Model; + const doc = await Skill.findById(id).lean(); + return (doc as unknown as (ISkill & { _id: Types.ObjectId }) | null) ?? null; + } + + async function listSkillsByAccess( + params: ListSkillsByAccessParams, + ): Promise { + const Skill = mongoose.models.Skill as Model; + const limit = Math.min(Math.max(1, params.limit || 20), 100); + + const baseFilter = buildSkillFilter(params); + const cursor = decodeCursor(params.cursor); + + let filter: FilterQuery = baseFilter; + if (cursor) { + const cursorCondition: FilterQuery = { + $or: [ + { updatedAt: { $lt: cursor.updatedAt } }, + { updatedAt: cursor.updatedAt, _id: { $gt: cursor._id } }, + ], + }; + filter = { $and: [baseFilter, cursorCondition] }; + } + + const rows = await Skill.find(filter) + .sort({ updatedAt: -1, _id: 1 }) + .limit(limit + 1) + .select( + 'name displayTitle description category author authorName version source sourceMetadata fileCount tenantId createdAt updatedAt', + ) + .lean(); + + const has_more = rows.length > limit; + const sliced = has_more ? rows.slice(0, limit) : rows; + const last = sliced[sliced.length - 1]; + const after = + has_more && last + ? encodeCursor({ + updatedAt: last.updatedAt as Date, + _id: last._id as Types.ObjectId, + }) + : null; + + return { + skills: sliced as unknown as Array, + has_more, + after, + }; + } + + async function updateSkill(params: { + id: string; + expectedVersion: number; + update: UpdateSkillInput; + }): Promise { + const { id, expectedVersion, update } = params; + if (!isValidObjectIdString(id)) { + return { status: 'not_found' }; + } + + const issues: ValidationIssue[] = []; + if (update.name !== undefined) issues.push(...validateSkillName(update.name)); + if (update.description !== undefined) + issues.push(...validateSkillDescription(update.description)); + if (update.body !== undefined) issues.push(...validateSkillBody(update.body)); + if (update.displayTitle !== undefined) + issues.push(...validateSkillDisplayTitle(update.displayTitle)); + if (update.frontmatter !== undefined) + issues.push(...validateSkillFrontmatter(update.frontmatter)); + const { errors, warnings } = partitionIssues(issues); + if (errors.length > 0) { + const error = new Error('Skill validation failed'); + (error as Error & { issues?: ValidationIssue[]; code?: string }).issues = errors; + (error as Error & { code?: string }).code = 'SKILL_VALIDATION_FAILED'; + throw error; + } + + const Skill = mongoose.models.Skill as Model; + const setPayload: Record = {}; + if (update.name !== undefined) setPayload.name = update.name; + if (update.displayTitle !== undefined) setPayload.displayTitle = update.displayTitle; + if (update.description !== undefined) setPayload.description = update.description; + if (update.body !== undefined) setPayload.body = update.body; + if (update.frontmatter !== undefined) setPayload.frontmatter = update.frontmatter; + if (update.category !== undefined) setPayload.category = update.category; + + const result = await Skill.findOneAndUpdate( + { _id: new ObjectId(id), version: expectedVersion }, + { $set: setPayload, $inc: { version: 1 } }, + { new: true }, + ).lean(); + + if (result) { + return { + status: 'updated', + skill: result as unknown as ISkill & { _id: Types.ObjectId }, + warnings, + }; + } + + const current = await Skill.findById(id).lean(); + if (!current) { + return { status: 'not_found' }; + } + return { + status: 'conflict', + current: current as unknown as ISkill & { _id: Types.ObjectId }, + }; + } + + async function deleteSkill(id: string): Promise<{ deleted: boolean }> { + if (!isValidObjectIdString(id)) { + return { deleted: false }; + } + const Skill = mongoose.models.Skill as Model; + const SkillFile = mongoose.models.SkillFile as Model; + const objectId = new ObjectId(id); + const res = await Skill.deleteOne({ _id: objectId }); + if (!res.deletedCount) { + return { deleted: false }; + } + await SkillFile.deleteMany({ skillId: objectId }); + try { + await deps.removeAllPermissions({ resourceType: ResourceType.SKILL, resourceId: id }); + } catch (error) { + logger.error(`[deleteSkill] Error removing permissions for ${id}:`, error); + } + return { deleted: true }; + } + + async function deleteUserSkills(userId: Types.ObjectId | string): Promise { + const userObjectId = typeof userId === 'string' ? new ObjectId(userId) : userId; + const Skill = mongoose.models.Skill as Model; + const soleOwned = await deps.getSoleOwnedResourceIds(userObjectId, ResourceType.SKILL); + if (soleOwned.length === 0) { + return 0; + } + const SkillFile = mongoose.models.SkillFile as Model; + await SkillFile.deleteMany({ skillId: { $in: soleOwned } }); + const res = await Skill.deleteMany({ _id: { $in: soleOwned } }); + await Promise.allSettled( + soleOwned.map((rid) => + deps + .removeAllPermissions({ + resourceType: ResourceType.SKILL, + resourceId: rid.toString(), + }) + .catch((error) => + logger.error(`[deleteUserSkills] Error removing permissions for ${rid}:`, error), + ), + ), + ); + return res.deletedCount ?? 0; + } + + /** + * Atomically bumps `Skill.version` and adjusts `fileCount` by `delta`. + * `delta` is `+1` when a new file is inserted, `-1` when one is deleted, and + * `0` when an existing file is replaced in place. + * + * NOTE on consistency: this runs as a **separate** MongoDB operation from + * the `upsertSkillFile` / `deleteSkillFile` that triggers it. MongoDB only + * provides multi-document ACID via transactions (which require a replica + * set), and LibreChat does not currently require that deployment shape. In + * the rare case where a SkillFile write succeeds but the subsequent + * `findByIdAndUpdate` here fails (connection drop, primary failover mid- + * request), the `fileCount` on the parent Skill will drift from the true + * row count until the next successful upsert/delete corrects it. Options if + * this ever shows up in practice: + * - wrap both ops in a transaction (requires a replica set) + * - periodic reconciliation: `fileCount = count(skill_files where skillId = ?)` + * - treat `fileCount` as advisory and recompute on read when accuracy + * matters + * For phase 1, skill files are stubbed at the upload boundary, so the risk + * window doesn't open in practice. + */ + async function bumpSkillVersionAndAdjustFileCount( + skillId: Types.ObjectId | string, + delta: number, + ): Promise { + const Skill = mongoose.models.Skill as Model; + const updateOps: Record> = { $inc: { version: 1 } }; + if (delta !== 0) { + updateOps.$inc.fileCount = delta; + } + await Skill.findByIdAndUpdate(skillId, updateOps); + } + + async function listSkillFiles( + skillId: Types.ObjectId | string, + ): Promise> { + const SkillFile = mongoose.models.SkillFile as Model; + const rows = await SkillFile.find({ skillId }).sort({ relativePath: 1 }).lean(); + return rows as unknown as Array; + } + + async function upsertSkillFile( + row: UpsertSkillFileInput, + ): Promise { + const issues = validateRelativePath(row.relativePath); + if (issues.length > 0) { + const error = new Error('Skill file validation failed'); + (error as Error & { issues?: ValidationIssue[]; code?: string }).issues = issues; + (error as Error & { code?: string }).code = 'SKILL_FILE_VALIDATION_FAILED'; + throw error; + } + const SkillFile = mongoose.models.SkillFile as Model; + const category = inferSkillFileCategory(row.relativePath); + // Atomic new-vs-replace detection: with `new: false, upsert: true`, + // `findOneAndUpdate` returns the pre-update document (or null if the doc + // did not exist and was just inserted). Checking the return value replaces + // a non-atomic `findOne` + `upsert` pair that could double-count on + // concurrent uploads of the same (skillId, relativePath). + const previous = await SkillFile.findOneAndUpdate( + { skillId: row.skillId, relativePath: row.relativePath }, + { + $set: { + skillId: row.skillId, + relativePath: row.relativePath, + file_id: row.file_id, + filename: row.filename, + filepath: row.filepath, + source: row.source, + mimeType: row.mimeType, + bytes: row.bytes, + category, + isExecutable: row.isExecutable ?? false, + author: row.author, + tenantId: row.tenantId, + }, + }, + { new: false, upsert: true }, + ).lean(); + const delta = previous ? 0 : 1; + await bumpSkillVersionAndAdjustFileCount(row.skillId, delta); + + // Fetch the current (post-upsert) document for the caller. This second + // round-trip is an intentional tradeoff for the TOCTOU-safe detection + // above: `new: false` is required to distinguish insert from replace + // atomically, which means `findOneAndUpdate` returns the pre-update + // document (null on insert). A separate `findOne` is the simplest way + // to return the authoritative post-upsert state. Performance impact is + // negligible compared to the file upload I/O this sits behind. + const current = await SkillFile.findOne({ + skillId: row.skillId, + relativePath: row.relativePath, + }).lean(); + return current as unknown as ISkillFile & { _id: Types.ObjectId }; + } + + async function deleteSkillFile( + skillId: Types.ObjectId | string, + relativePath: string, + ): Promise<{ deleted: boolean }> { + const SkillFile = mongoose.models.SkillFile as Model; + const res = await SkillFile.deleteOne({ skillId, relativePath }); + if (!res.deletedCount) { + return { deleted: false }; + } + await bumpSkillVersionAndAdjustFileCount(skillId, -1); + return { deleted: true }; + } + + // The public surface is scoped to methods that handlers and the user + // deletion controller actually call. The per-skill file cascade on + // `deleteSkill` is inlined; there's no need for a separate export. + return { + createSkill, + getSkillById, + listSkillsByAccess, + updateSkill, + deleteSkill, + deleteUserSkills, + listSkillFiles, + upsertSkillFile, + deleteSkillFile, + }; +} + +export type SkillMethods = ReturnType; diff --git a/packages/data-schemas/src/models/index.ts b/packages/data-schemas/src/models/index.ts index 5a8e8f1c2c..dcd88f58d2 100644 --- a/packages/data-schemas/src/models/index.ts +++ b/packages/data-schemas/src/models/index.ts @@ -19,6 +19,8 @@ import { createTransactionModel } from './transaction'; import { createPresetModel } from './preset'; import { createPromptModel } from './prompt'; import { createPromptGroupModel } from './promptGroup'; +import { createSkillModel } from './skill'; +import { createSkillFileModel } from './skillFile'; import { createConversationTagModel } from './conversationTag'; import { createSharedLinkModel } from './sharedLink'; import { createToolCallModel } from './toolCall'; @@ -55,6 +57,8 @@ export function createModels(mongoose: typeof import('mongoose')) { Preset: createPresetModel(mongoose), Prompt: createPromptModel(mongoose), PromptGroup: createPromptGroupModel(mongoose), + Skill: createSkillModel(mongoose), + SkillFile: createSkillFileModel(mongoose), ConversationTag: createConversationTagModel(mongoose), SharedLink: createSharedLinkModel(mongoose), ToolCall: createToolCallModel(mongoose), diff --git a/packages/data-schemas/src/models/skill.ts b/packages/data-schemas/src/models/skill.ts new file mode 100644 index 0000000000..588523f3de --- /dev/null +++ b/packages/data-schemas/src/models/skill.ts @@ -0,0 +1,8 @@ +import skillSchema from '~/schema/skill'; +import { applyTenantIsolation } from '~/models/plugins/tenantIsolation'; +import type { ISkillDocument } from '~/types/skill'; + +export function createSkillModel(mongoose: typeof import('mongoose')) { + applyTenantIsolation(skillSchema); + return mongoose.models.Skill || mongoose.model('Skill', skillSchema); +} diff --git a/packages/data-schemas/src/models/skillFile.ts b/packages/data-schemas/src/models/skillFile.ts new file mode 100644 index 0000000000..230a244910 --- /dev/null +++ b/packages/data-schemas/src/models/skillFile.ts @@ -0,0 +1,10 @@ +import skillFileSchema from '~/schema/skillFile'; +import { applyTenantIsolation } from '~/models/plugins/tenantIsolation'; +import type { ISkillFileDocument } from '~/types/skill'; + +export function createSkillFileModel(mongoose: typeof import('mongoose')) { + applyTenantIsolation(skillFileSchema); + return ( + mongoose.models.SkillFile || mongoose.model('SkillFile', skillFileSchema) + ); +} diff --git a/packages/data-schemas/src/schema/accessRole.ts b/packages/data-schemas/src/schema/accessRole.ts index dbcaf83ddb..9b5bd49010 100644 --- a/packages/data-schemas/src/schema/accessRole.ts +++ b/packages/data-schemas/src/schema/accessRole.ts @@ -15,7 +15,7 @@ const accessRoleSchema = new Schema( description: String, resourceType: { type: String, - enum: ['agent', 'project', 'file', 'promptGroup', 'mcpServer', 'remoteAgent'], + enum: ['agent', 'project', 'file', 'promptGroup', 'mcpServer', 'remoteAgent', 'skill'], required: true, default: 'agent', }, diff --git a/packages/data-schemas/src/schema/role.ts b/packages/data-schemas/src/schema/role.ts index ac478c2a83..ab1fc582e9 100644 --- a/packages/data-schemas/src/schema/role.ts +++ b/packages/data-schemas/src/schema/role.ts @@ -67,6 +67,12 @@ const rolePermissionsSchema = new Schema( [Permissions.SHARE]: { type: Boolean }, [Permissions.SHARE_PUBLIC]: { type: Boolean }, }, + [PermissionTypes.SKILLS]: { + [Permissions.USE]: { type: Boolean }, + [Permissions.CREATE]: { type: Boolean }, + [Permissions.SHARE]: { type: Boolean }, + [Permissions.SHARE_PUBLIC]: { type: Boolean }, + }, }, { _id: false }, ); diff --git a/packages/data-schemas/src/schema/skill.ts b/packages/data-schemas/src/schema/skill.ts new file mode 100644 index 0000000000..219bd291e4 --- /dev/null +++ b/packages/data-schemas/src/schema/skill.ts @@ -0,0 +1,183 @@ +import { Schema } from 'mongoose'; +import type { ISkillDocument } from '~/types/skill'; + +/** Max length for a skill `name` (kebab-case identifier). */ +const SKILL_NAME_MAX_LENGTH = 64; + +/** Max length for a skill `description`. */ +const SKILL_DESCRIPTION_MAX_LENGTH = 1024; + +/** Max length for the skill `body` (the SKILL.md content). */ +const SKILL_BODY_MAX_LENGTH = 100_000; + +/** Max length for the human-friendly `displayTitle`. */ +const SKILL_DISPLAY_TITLE_MAX_LENGTH = 128; + +const skillNamePattern = /^[a-z0-9][a-z0-9-]*$/; + +/** + * Brand namespaces reserved for first-party skills. Matched as prefixes — + * `anthropic-helper` is rejected but `my-helper` is fine. Keep this in sync + * with `RESERVED_NAME_PREFIXES` in `methods/skill.ts`. + */ +const RESERVED_NAME_PREFIXES = ['anthropic-', 'claude-']; + +/** + * Slash-command names that collide with LibreChat / Claude Code CLI commands. + * Matched exactly against the full skill name. Keep this in sync with + * `RESERVED_NAME_WORDS` in `methods/skill.ts`. + */ +const RESERVED_NAME_WORDS = new Set([ + 'help', + 'clear', + 'compact', + 'model', + 'exit', + 'quit', + 'settings', + 'anthropic', + 'claude', +]); + +const skillSchema: Schema = new Schema( + { + /** + * Machine-readable identifier. Kebab-case, max 64 chars, unique per + * (author, tenantId). This is what Claude sees in its system prompt when + * deciding to trigger the skill, and what slash-command integrations key + * off — so it must be stable, concise, and ASCII-friendly. User-facing + * labels live on `displayTitle` instead. + */ + name: { + type: String, + required: true, + index: true, + maxlength: [SKILL_NAME_MAX_LENGTH, `Name cannot exceed ${SKILL_NAME_MAX_LENGTH} characters`], + validate: { + validator: function (value: string): boolean { + if (!skillNamePattern.test(value)) { + return false; + } + const lowered = value.toLowerCase(); + if (RESERVED_NAME_PREFIXES.some((prefix) => lowered.startsWith(prefix))) { + return false; + } + if (RESERVED_NAME_WORDS.has(lowered)) { + return false; + } + return true; + }, + message: + 'Name must be kebab-case (lowercase letters, digits, hyphens), cannot start with "anthropic-" or "claude-", and cannot be a reserved CLI word.', + }, + }, + /** + * Human-readable label shown in the LibreChat UI (skill list, detail + * header, sharing dialogs). NOT sent to Claude and NOT used to trigger + * the skill — `name` + `description` drive triggering. Purely cosmetic: + * useful when the author wants a prettier label than the kebab-case + * identifier while keeping the identifier stable. + */ + displayTitle: { + type: String, + maxlength: [ + SKILL_DISPLAY_TITLE_MAX_LENGTH, + `Display title cannot exceed ${SKILL_DISPLAY_TITLE_MAX_LENGTH} characters`, + ], + }, + /** + * "When to use this skill" sentence that Claude reads from the system + * prompt to decide whether to invoke the skill. This is the highest- + * leverage field for triggering accuracy — a vague description causes + * undertriggering. Denormalized from the YAML frontmatter for indexed + * querying. + */ + description: { + type: String, + required: true, + maxlength: [ + SKILL_DESCRIPTION_MAX_LENGTH, + `Description cannot exceed ${SKILL_DESCRIPTION_MAX_LENGTH} characters`, + ], + }, + /** The SKILL.md body (markdown after the YAML frontmatter). */ + body: { + type: String, + default: '', + maxlength: [SKILL_BODY_MAX_LENGTH, `Body cannot exceed ${SKILL_BODY_MAX_LENGTH} characters`], + }, + /** + * Structured YAML frontmatter bag (everything except `name`/`description`, + * which live as first-class columns). Validated in strict mode against + * `validateSkillFrontmatter` before write — unknown keys are rejected + * so any expansion of the allowed set is an explicit code change. + */ + frontmatter: { + type: Schema.Types.Mixed, + default: {}, + }, + category: { + type: String, + default: '', + index: true, + }, + author: { + type: Schema.Types.ObjectId, + ref: 'User', + required: true, + index: true, + }, + authorName: { + type: String, + required: true, + }, + version: { + type: Number, + default: 1, + min: 1, + }, + /** + * Provenance of this skill's canonical definition. + * + * - `inline` — authored directly inside LibreChat (the only path wired + * up in phase 1). + * - `github` / `notion` — **reserved for phase 2+ external sync**. No + * code path currently produces these values. The column exists now so + * a future sync worker can populate `source` + `sourceMetadata` without + * a schema migration. + */ + source: { + type: String, + enum: ['inline', 'github', 'notion'], + default: 'inline', + }, + /** + * Arbitrary JSON provenance payload keyed by `source`. Phase 2+ sync + * workers will use this to store the upstream commit SHA (github), + * page id (notion), etc. Unused in phase 1 — kept `Mixed` to avoid + * committing to a shape before the sync paths exist. + */ + sourceMetadata: { + type: Schema.Types.Mixed, + }, + fileCount: { + type: Number, + default: 0, + min: 0, + }, + tenantId: { + type: String, + index: true, + }, + }, + { + timestamps: true, + }, +); + +skillSchema.index({ author: 1, tenantId: 1 }); +skillSchema.index({ category: 1, updatedAt: -1 }); +skillSchema.index({ updatedAt: -1, _id: 1 }); +skillSchema.index({ name: 1, author: 1, tenantId: 1 }, { unique: true }); + +export default skillSchema; diff --git a/packages/data-schemas/src/schema/skillFile.ts b/packages/data-schemas/src/schema/skillFile.ts new file mode 100644 index 0000000000..af2371894f --- /dev/null +++ b/packages/data-schemas/src/schema/skillFile.ts @@ -0,0 +1,102 @@ +import { Schema } from 'mongoose'; +import type { ISkillFileDocument } from '~/types/skill'; + +/** Max length for a skill file's relative path (e.g. "scripts/parse.sh"). */ +const SKILL_FILE_PATH_MAX_LENGTH = 500; + +/** Pattern for valid path characters — conservative allowlist. */ +const relativePathCharPattern = /^[a-zA-Z0-9._\-/]+$/; + +const skillFileSchema: Schema = new Schema( + { + skillId: { + type: Schema.Types.ObjectId, + ref: 'Skill', + required: true, + index: true, + }, + relativePath: { + type: String, + required: true, + maxlength: [ + SKILL_FILE_PATH_MAX_LENGTH, + `Relative path cannot exceed ${SKILL_FILE_PATH_MAX_LENGTH} characters`, + ], + validate: { + validator: function (value: string): boolean { + if (!value || value.length === 0) { + return false; + } + if (value.startsWith('/') || value.startsWith('\\')) { + return false; + } + if (!relativePathCharPattern.test(value)) { + return false; + } + const segments = value.split('/'); + if (segments.some((s) => s === '' || s === '.' || s === '..')) { + return false; + } + if (value === 'SKILL.md' || segments[0] === 'SKILL.md') { + return false; + } + return true; + }, + message: + 'Relative path must be relative, cannot contain ".." or absolute paths, and cannot be SKILL.md.', + }, + }, + file_id: { + type: String, + required: true, + index: true, + }, + filename: { + type: String, + required: true, + }, + filepath: { + type: String, + required: true, + }, + source: { + type: String, + required: true, + }, + mimeType: { + type: String, + required: true, + }, + bytes: { + type: Number, + required: true, + min: 0, + }, + category: { + type: String, + enum: ['script', 'reference', 'asset', 'other'], + default: 'other', + }, + isExecutable: { + type: Boolean, + default: false, + }, + author: { + type: Schema.Types.ObjectId, + ref: 'User', + required: true, + }, + tenantId: { + type: String, + index: true, + }, + }, + { + timestamps: true, + }, +); + +skillFileSchema.index({ skillId: 1, relativePath: 1 }, { unique: true }); +skillFileSchema.index({ skillId: 1, category: 1 }); + +export default skillFileSchema; diff --git a/packages/data-schemas/src/types/index.ts b/packages/data-schemas/src/types/index.ts index 748ea5d77d..f0b1fee5ec 100644 --- a/packages/data-schemas/src/types/index.ts +++ b/packages/data-schemas/src/types/index.ts @@ -23,6 +23,8 @@ export * from './pluginAuth'; export * from './memory'; /* Prompts */ export * from './prompts'; +/* Skills */ +export * from './skill'; /* Access Control */ export * from './accessRole'; export * from './aclEntry'; diff --git a/packages/data-schemas/src/types/role.ts b/packages/data-schemas/src/types/role.ts index bc85284c34..7ffa6e8849 100644 --- a/packages/data-schemas/src/types/role.ts +++ b/packages/data-schemas/src/types/role.ts @@ -66,6 +66,12 @@ export interface IRole extends Document { [Permissions.SHARE]?: boolean; [Permissions.SHARE_PUBLIC]?: boolean; }; + [PermissionTypes.SKILLS]?: { + [Permissions.USE]?: boolean; + [Permissions.CREATE]?: boolean; + [Permissions.SHARE]?: boolean; + [Permissions.SHARE_PUBLIC]?: boolean; + }; }; tenantId?: string; } diff --git a/packages/data-schemas/src/types/skill.ts b/packages/data-schemas/src/types/skill.ts new file mode 100644 index 0000000000..afaa1b5c95 --- /dev/null +++ b/packages/data-schemas/src/types/skill.ts @@ -0,0 +1,97 @@ +import type { Document, Types } from 'mongoose'; + +/** + * Skill — the single source of truth for a LibreChat skill. + * Each document is the full SKILL.md body plus structured frontmatter and metadata. + * The `version` field is an integer monotonic counter used for optimistic concurrency. + */ +export interface ISkill { + /** + * Machine-readable kebab-case identifier. This is what Claude sees in the + * system-prompt skill manifest and what slash-command integrations key off. + * Unique per `(author, tenantId)`. Must be stable across edits — a rename + * invalidates any external reference to the skill. + */ + name: string; + /** + * Human-readable label shown only in the LibreChat UI (skill list, detail + * header, sharing dialogs). NOT sent to Claude and NOT part of the trigger + * path — `name` + `description` drive triggering. Purely cosmetic: lets an + * author keep a stable kebab-case `name` while showing something prettier + * in the UI. + */ + displayTitle?: string; + /** + * "When to use this skill" sentence. This is the highest-leverage field for + * Claude's triggering decision — vague or missing descriptions cause + * undertriggering. Denormalized from the YAML frontmatter onto its own + * column so listings can filter/sort on it without loading `body`. + */ + description: string; + /** The SKILL.md body (markdown after the YAML frontmatter). */ + body: string; + /** + * Structured YAML frontmatter (excluding `name` and `description`, which live as + * top-level columns). Stored as Mongoose Mixed so callers can extend without schema + * churn; validated in strict mode via `validateSkillFrontmatter` — unknown keys + * are rejected so expanding the allowed set is an intentional code change. + */ + frontmatter: Record; + category?: string; + author: Types.ObjectId; + authorName: string; + version: number; + /** + * Provenance of this skill's canonical definition. + * - `inline` — authored inside LibreChat (the only value phase 1 produces). + * - `github` / `notion` — reserved for phase 2+ external sync. Kept in the + * enum so a future sync worker can populate it without a migration. + */ + source: 'inline' | 'github' | 'notion'; + /** + * Provenance payload keyed by `source`. Phase 2+ sync workers will store + * upstream identifiers (commit SHA, page id, etc.) here. Unused in phase 1. + */ + sourceMetadata?: Record; + /** Denormalized count of associated `SkillFile` rows. Kept in sync by skill methods. */ + fileCount: number; + tenantId?: string; + createdAt?: Date; + updatedAt?: Date; + /** Computed from ACL at read time, never persisted. */ + isPublic?: boolean; +} + +export interface ISkillDocument extends ISkill, Document {} + +/** + * Lean summary projection returned by `listSkillsByAccess`. The list query + * uses a narrow `.select()` that omits `body` and `frontmatter` to keep + * payloads small, so those fields are truthfully absent on summary rows. + */ +export type ISkillSummary = Omit; + +/** + * SkillFile — metadata for a file bundled inside a skill. + * Blob content lives in the existing file storage layer (local/S3/etc.) and is + * addressed via `source` + `filepath` + `file_id` (mirroring the File schema). + * `(skillId, relativePath)` is unique per skill. + */ +export interface ISkillFile { + skillId: Types.ObjectId; + relativePath: string; + file_id: string; + filename: string; + filepath: string; + source: string; + mimeType: string; + bytes: number; + category: 'script' | 'reference' | 'asset' | 'other'; + isExecutable: boolean; + author: Types.ObjectId; + tenantId?: string; + createdAt?: Date; + updatedAt?: Date; +} + +export interface ISkillFileDocument extends ISkillFile, Document {}