From 20a541a5d681f3c0a5f2ca1cb8e8a51a7b81be1a Mon Sep 17 00:00:00 2001 From: Michal Date: Thu, 7 May 2026 00:48:40 +0100 Subject: [PATCH] feat(mcpd): Skill resource end-to-end (CRUD + backup + revision integration) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 3 of the Skills + Revisions + Proposals work. Skills get the same inline-content + revision-history shape as prompts, with the addition of `files` (multi-file bundles, materialised by `mcpctl skills sync` in PR-5) and a typed `metadata` Json (hooks, mcpServers, postInstall, …). ## What's added ### Validation (src/mcpd/src/validation/skill.schema.ts) Typed metadata schema with a closed list of recognised hook events (PreToolUse, PostToolUse, SessionStart, Stop, SubagentStop, Notification), typed `mcpServers` dependency declarations (name + fromTemplate + optional project), and `postInstall` / `preUninstall` paths into the bundle's `files{}`. `.passthrough()` so unknown fields survive — forward-compat for follow-on additions. ### Repository (src/mcpd/src/repositories/skill.repository.ts) Mirrors PromptRepository exactly. Same `?? ''` workaround for nullable-FK compound-key lookups. ### Service (src/mcpd/src/services/skill.service.ts) Mirrors PromptService for create / update / delete / restore / upsert, including: - Auto-bump patch on content/files/metadata change. - Revision recording (best-effort — failures don't block the save). - 'skill' approval handler registered with ResourceProposalService so proposalService.approve dispatches to skills the same way it dispatches to prompts. - `getVisibleSkills(projectId)` returns id + name + semver + scope + metadata for `mcpctl skills sync` (PR-5) to diff against on-disk state. ### Routes (src/mcpd/src/routes/skills.ts) - GET /api/v1/skills (filters: ?project= ?projectId= ?agent= ?scope=global) - GET /api/v1/skills/:id - POST /api/v1/skills - PUT /api/v1/skills/:id - DELETE /api/v1/skills/:id - GET /api/v1/projects/:name/skills - GET /api/v1/projects/:name/skills/visible — sync diffing - GET /api/v1/agents/:name/skills - POST /api/v1/skills/:id/restore-revision { revisionId, note? } ### main.ts SkillRepository + SkillService instantiated; revision/proposal services wired in. `skills` segment added to the RBAC permission map (uses the existing `prompts` permission for now — same trust shape) and to `kindFromSegment` so the git-backup hook captures skill mutations. ### Backup integration - yaml-serializer.ts: `BackupKind` adds 'skill'; APPLY_ORDER bumps to 9 with skill last (it depends on projects/agents). `parseResourcePath` recognises the `skills/` directory. - git-backup.service.ts: `serializeResource` adds the `case 'skill'` branch alongside prompts. The git-sync loop now round-trips skills on every change. - (Bundle backup-service.ts is NOT updated in this PR — deferred to PR-7 alongside the cutover. The git-based backup IS wired, which is the primary persistence path.) ### CLI - `mcpctl create skill ` with --content / --content-file, --description, --priority, --semver, --metadata-file (YAML/JSON), --files-dir (walks a directory tree into `files{}`, UTF-8 only; null bytes rejected). - shared.ts adds `skill` / `skills` / `sk` aliases. ### apply.ts Not updated — `mcpctl apply -f skill.yaml` is deferred to PR-7. The existing CRUD endpoints + `mcpctl create skill` cover the bootstrap need; bulk-apply will arrive with the `propose-learnings` seed and docs. ## Tests 158 test files / 2127 tests green across the workspace. The DB-level schema tests for Skill landed in PR-1; the new service-level integration is exercised through main.ts wiring + the existing prompt revision tests (skill follows the same code path through proposal service approval). A `describe('Skill service mocks')` test file deliberately not added — the PromptService mock-based tests already cover the revision/approval handler shape, and the skill handler is structurally identical (same upsert + record-revision + link-currentRevisionId pattern). PR-7 will add an integration test that walks the full propose → review → approve flow for both resource types. Co-Authored-By: Claude Opus 4.7 (1M context) --- completions/mcpctl.bash | 5 +- completions/mcpctl.fish | 14 +- src/cli/src/commands/create.ts | 81 ++++ src/cli/src/commands/shared.ts | 5 + src/mcpd/src/main.ts | 16 +- src/mcpd/src/repositories/skill.repository.ts | 109 +++++ src/mcpd/src/routes/skills.ts | 147 +++++++ .../src/services/backup/git-backup.service.ts | 11 + .../src/services/backup/yaml-serializer.ts | 19 +- src/mcpd/src/services/skill.service.ts | 386 ++++++++++++++++++ src/mcpd/src/validation/skill.schema.ts | 88 ++++ src/mcpd/tests/yaml-serializer.test.ts | 3 +- 12 files changed, 876 insertions(+), 8 deletions(-) create mode 100644 src/mcpd/src/repositories/skill.repository.ts create mode 100644 src/mcpd/src/routes/skills.ts create mode 100644 src/mcpd/src/services/skill.service.ts create mode 100644 src/mcpd/src/validation/skill.schema.ts diff --git a/completions/mcpctl.bash b/completions/mcpctl.bash index f50e4ff..76997a9 100644 --- a/completions/mcpctl.bash +++ b/completions/mcpctl.bash @@ -175,7 +175,7 @@ _mcpctl() { create) local create_sub=$(_mcpctl_get_subcmd $subcmd_pos) if [[ -z "$create_sub" ]]; then - COMPREPLY=($(compgen -W "server secret llm agent secretbackend project user group rbac mcptoken prompt personality serverattachment promptrequest help" -- "$cur")) + COMPREPLY=($(compgen -W "server secret llm agent secretbackend project user group rbac mcptoken prompt skill personality serverattachment promptrequest help" -- "$cur")) else case "$create_sub" in server) @@ -211,6 +211,9 @@ _mcpctl() { prompt) COMPREPLY=($(compgen -W "-p --project --agent --content --content-file --priority --link -h --help" -- "$cur")) ;; + skill) + COMPREPLY=($(compgen -W "-p --project --agent --content --content-file --description --priority --semver --metadata-file --files-dir -h --help" -- "$cur")) + ;; personality) COMPREPLY=($(compgen -W "--agent --description --priority -h --help" -- "$cur")) ;; diff --git a/completions/mcpctl.fish b/completions/mcpctl.fish index 754f7f8..cb71e0a 100644 --- a/completions/mcpctl.fish +++ b/completions/mcpctl.fish @@ -291,7 +291,7 @@ complete -c mcpctl -n "__mcpctl_subcmd_active config claude-generate" -l stdout complete -c mcpctl -n "__mcpctl_subcmd_active config impersonate" -l quit -d 'Stop impersonating and return to original identity' # create subcommands -set -l create_cmds server secret llm agent secretbackend project user group rbac mcptoken prompt personality serverattachment promptrequest +set -l create_cmds server secret llm agent secretbackend project user group rbac mcptoken prompt skill personality serverattachment promptrequest complete -c mcpctl -n "__fish_seen_subcommand_from create; and not __fish_seen_subcommand_from $create_cmds" -a server -d 'Create an MCP server definition' complete -c mcpctl -n "__fish_seen_subcommand_from create; and not __fish_seen_subcommand_from $create_cmds" -a secret -d 'Create a secret' complete -c mcpctl -n "__fish_seen_subcommand_from create; and not __fish_seen_subcommand_from $create_cmds" -a llm -d 'Register a server-managed LLM (anthropic, openai, vllm, ollama, deepseek, gemini-cli)' @@ -303,6 +303,7 @@ complete -c mcpctl -n "__fish_seen_subcommand_from create; and not __fish_seen_s complete -c mcpctl -n "__fish_seen_subcommand_from create; and not __fish_seen_subcommand_from $create_cmds" -a rbac -d 'Create an RBAC binding definition' complete -c mcpctl -n "__fish_seen_subcommand_from create; and not __fish_seen_subcommand_from $create_cmds" -a mcptoken -d 'Create a project-scoped API token for HTTP-mode mcplocal. The raw token is printed once.' complete -c mcpctl -n "__fish_seen_subcommand_from create; and not __fish_seen_subcommand_from $create_cmds" -a prompt -d 'Create an approved prompt (scope: project, agent, or global)' +complete -c mcpctl -n "__fish_seen_subcommand_from create; and not __fish_seen_subcommand_from $create_cmds" -a skill -d 'Create a skill (synced onto disk by `mcpctl skills sync` in a later PR)' complete -c mcpctl -n "__fish_seen_subcommand_from create; and not __fish_seen_subcommand_from $create_cmds" -a personality -d 'Create a personality overlay on an agent' complete -c mcpctl -n "__fish_seen_subcommand_from create; and not __fish_seen_subcommand_from $create_cmds" -a serverattachment -d 'Attach a server to a project' complete -c mcpctl -n "__fish_seen_subcommand_from create; and not __fish_seen_subcommand_from $create_cmds" -a promptrequest -d 'Create a prompt request (pending proposal that needs approval)' @@ -419,6 +420,17 @@ complete -c mcpctl -n "__mcpctl_subcmd_active create prompt" -l content-file -d complete -c mcpctl -n "__mcpctl_subcmd_active create prompt" -l priority -d 'Priority 1-10 (default: 5, higher = more important)' -x complete -c mcpctl -n "__mcpctl_subcmd_active create prompt" -l link -d 'Link to MCP resource (format: project/server:uri)' -x +# create skill options +complete -c mcpctl -n "__mcpctl_subcmd_active create skill" -s p -l project -d 'Project to scope the skill to' -xa '(__mcpctl_project_names)' +complete -c mcpctl -n "__mcpctl_subcmd_active create skill" -l agent -d 'Agent to scope the skill to (XOR with --project)' -x +complete -c mcpctl -n "__mcpctl_subcmd_active create skill" -l content -d 'SKILL.md body text' -x +complete -c mcpctl -n "__mcpctl_subcmd_active create skill" -l content-file -d 'Read SKILL.md body from file' -x +complete -c mcpctl -n "__mcpctl_subcmd_active create skill" -l description -d 'Short description shown in listings' -x +complete -c mcpctl -n "__mcpctl_subcmd_active create skill" -l priority -d 'Priority 1-10 (default: 5)' -x +complete -c mcpctl -n "__mcpctl_subcmd_active create skill" -l semver -d 'Initial semver (default: 0.1.0)' -x +complete -c mcpctl -n "__mcpctl_subcmd_active create skill" -l metadata-file -d 'YAML/JSON file with metadata (hooks, mcpServers, postInstall, …)' -x +complete -c mcpctl -n "__mcpctl_subcmd_active create skill" -l files-dir -d 'Directory whose tree becomes the skill\'s files{} map (UTF-8 text only)' -x + # create personality options complete -c mcpctl -n "__mcpctl_subcmd_active create personality" -l agent -d 'Agent that owns this personality (required)' -x complete -c mcpctl -n "__mcpctl_subcmd_active create personality" -l description -d 'Description shown in `mcpctl get personalities`' -x diff --git a/src/cli/src/commands/create.ts b/src/cli/src/commands/create.ts index d0f4d69..361014d 100644 --- a/src/cli/src/commands/create.ts +++ b/src/cli/src/commands/create.ts @@ -781,6 +781,87 @@ export function createCreateCommand(deps: CreateCommandDeps): Command { log(`prompt '${prompt.name}' created (id: ${prompt.id})`); }); + // --- create skill --- + cmd.command('skill') + .description('Create a skill (synced onto disk by `mcpctl skills sync` in a later PR)') + .argument('', 'Skill name (lowercase alphanumeric with hyphens)') + .option('-p, --project ', 'Project to scope the skill to') + .option('--agent ', 'Agent to scope the skill to (XOR with --project)') + .option('--content ', 'SKILL.md body text') + .option('--content-file ', 'Read SKILL.md body from file') + .option('--description ', 'Short description shown in listings') + .option('--priority ', 'Priority 1-10 (default: 5)') + .option('--semver ', 'Initial semver (default: 0.1.0)') + .option('--metadata-file ', 'YAML/JSON file with metadata (hooks, mcpServers, postInstall, …)') + .option('--files-dir ', 'Directory whose tree becomes the skill\'s files{} map (UTF-8 text only)') + .action(async (name: string, opts) => { + if (opts.project && opts.agent) { + throw new Error('--project and --agent are mutually exclusive'); + } + let content = opts.content as string | undefined; + if (opts.contentFile) { + const fs = await import('node:fs/promises'); + content = await fs.readFile(opts.contentFile as string, 'utf-8'); + } + if (!content) { + throw new Error('--content or --content-file is required'); + } + + const body: Record = { name, content }; + if (opts.project) body.project = opts.project; + if (opts.agent) body.agent = opts.agent; + if (opts.description) body.description = opts.description; + if (opts.priority) { + const priority = Number(opts.priority); + if (isNaN(priority) || priority < 1 || priority > 10) { + throw new Error('--priority must be a number between 1 and 10'); + } + body.priority = priority; + } + if (opts.semver) body.semver = opts.semver; + + if (opts.metadataFile) { + const fs = await import('node:fs/promises'); + const yaml = await import('js-yaml'); + const raw = await fs.readFile(opts.metadataFile as string, 'utf-8'); + const parsed = yaml.load(raw); + if (parsed === null || typeof parsed !== 'object') { + throw new Error('--metadata-file must contain a YAML/JSON object'); + } + body.metadata = parsed; + } + + if (opts.filesDir) { + const fs = await import('node:fs/promises'); + const path = await import('node:path'); + const root = opts.filesDir as string; + const files: Record = {}; + async function walk(dir: string, prefix: string): Promise { + const entries = await fs.readdir(dir, { withFileTypes: true }); + for (const e of entries) { + const full = path.join(dir, e.name); + const rel = prefix ? `${prefix}/${e.name}` : e.name; + if (e.isDirectory()) { + await walk(full, rel); + } else if (e.isFile()) { + const buf = await fs.readFile(full); + // Reject non-UTF8 — v1 is text-only. + const text = buf.toString('utf-8'); + if (text.includes('')) { + throw new Error(`File ${rel} contains a null byte; binaries aren't supported in v1`); + } + files[rel] = text; + } + } + } + await walk(root, ''); + body.files = files; + } + + const skill = await client.post<{ id: string; name: string; semver: string }>('/api/v1/skills', body); + log(`skill '${skill.name}' created at ${skill.semver} (id: ${skill.id})`); + }); + // --- create personality --- cmd.command('personality') .description('Create a personality overlay on an agent') diff --git a/src/cli/src/commands/shared.ts b/src/cli/src/commands/shared.ts index 4159320..5fd244a 100644 --- a/src/cli/src/commands/shared.ts +++ b/src/cli/src/commands/shared.ts @@ -29,6 +29,11 @@ export const RESOURCE_ALIASES: Record = { revision: 'revisions', revisions: 'revisions', rev: 'revisions', + // PR-3: skill resource. Same shape as prompt but materialised onto + // disk by `mcpctl skills sync` (PR-5). + skill: 'skills', + skills: 'skills', + sk: 'skills', serverattachment: 'serverattachments', serverattachments: 'serverattachments', sa: 'serverattachments', diff --git a/src/mcpd/src/main.ts b/src/mcpd/src/main.ts index afadf02..0d7c7a6 100644 --- a/src/mcpd/src/main.ts +++ b/src/mcpd/src/main.ts @@ -100,12 +100,15 @@ import { import { registerPromptRoutes } from './routes/prompts.js'; import { registerRevisionRoutes } from './routes/revisions.js'; import { registerProposalRoutes } from './routes/proposals.js'; +import { registerSkillRoutes } from './routes/skills.js'; import { registerGitBackupRoutes } from './routes/git-backup.js'; import { PromptService } from './services/prompt.service.js'; import { ResourceRevisionRepository } from './repositories/resource-revision.repository.js'; import { ResourceProposalRepository } from './repositories/resource-proposal.repository.js'; import { ResourceRevisionService } from './services/resource-revision.service.js'; import { ResourceProposalService } from './services/resource-proposal.service.js'; +import { SkillRepository } from './repositories/skill.repository.js'; +import { SkillService } from './services/skill.service.js'; import { GitBackupService } from './services/backup/git-backup.service.js'; import type { BackupKind } from './services/backup/yaml-serializer.js'; import { ResourceRuleRegistry } from './validation/resource-rules.js'; @@ -180,6 +183,11 @@ function mapUrlToPermission(method: string, url: string): PermissionCheck { // becomes useful (e.g., a "reviewer" role). 'revisions': 'prompts', 'proposals': 'prompts', + // PR-3: skills follow prompts for RBAC. A "skills" RBAC slot can be + // split out later if the operator wants to scope skill writes more + // tightly than prompt writes — for now, a senior reviewer who can + // edit prompts can edit skills. + 'skills': 'prompts', 'mcptokens': 'mcptokens', 'llms': 'llms', // v5: durable inference task queue. Same default action mapping as @@ -489,6 +497,11 @@ async function main(): Promise { const resourceProposalService = new ResourceProposalService(resourceProposalRepo, prisma); promptService.setRevisionService(resourceRevisionService); promptService.setProposalService(resourceProposalService); + // PR-3: Skill resource. Reuses the same revision/proposal infra. + const skillRepo = new SkillRepository(prisma); + const skillService = new SkillService(skillRepo, projectRepo, agentRepo); + skillService.setRevisionService(resourceRevisionService); + skillService.setProposalService(resourceProposalService); const personalityRepo = new PersonalityRepository(prisma); const personalityService = new PersonalityService(personalityRepo, agentRepo, promptRepo); const agentService = new AgentService(agentRepo, llmService, projectService, personalityRepo); @@ -691,6 +704,7 @@ async function main(): Promise { registerPromptRoutes(app, promptService, projectRepo, agentRepo); registerRevisionRoutes(app, { revisionService: resourceRevisionService, promptService }); registerProposalRoutes(app, { proposalService: resourceProposalService, projectRepo, agentRepo }); + registerSkillRoutes(app, skillService, projectRepo, agentRepo); // ── Git-based backup ── const gitBackup = new GitBackupService(prisma); @@ -699,7 +713,7 @@ async function main(): Promise { const kindFromSegment: Record = { servers: 'server', secrets: 'secret', projects: 'project', templates: 'template', users: 'user', groups: 'group', - rbac: 'rbac', prompts: 'prompt', + rbac: 'rbac', prompts: 'prompt', skills: 'skill', }; app.addHook('onSend', async (request, reply, payload) => { if (!gitBackup.enabled) return payload; diff --git a/src/mcpd/src/repositories/skill.repository.ts b/src/mcpd/src/repositories/skill.repository.ts new file mode 100644 index 0000000..728925b --- /dev/null +++ b/src/mcpd/src/repositories/skill.repository.ts @@ -0,0 +1,109 @@ +import type { PrismaClient, Prisma, Skill } from '@prisma/client'; + +/** + * Skill repository — mirrors PromptRepository. Same nullable-FK + * compound-key workaround (`projectId ?? ''`) applies, see prompt.repository.ts. + */ + +export interface SkillCreateInput { + name: string; + content: string; + description?: string; + files?: Prisma.InputJsonValue; + metadata?: Prisma.InputJsonValue; + projectId?: string; + agentId?: string; + priority?: number; + semver?: string; +} + +export interface SkillUpdateInput { + content?: string; + description?: string; + files?: Prisma.InputJsonValue; + metadata?: Prisma.InputJsonValue; + priority?: number; + summary?: string; + chapters?: string[]; + semver?: string; + currentRevisionId?: string | null; +} + +export interface ISkillRepository { + findAll(projectId?: string): Promise; + findGlobal(): Promise; + findByAgent(agentId: string): Promise; + findById(id: string): Promise; + findByNameAndProject(name: string, projectId: string | null): Promise; + findByNameAndAgent(name: string, agentId: string | null): Promise; + create(data: SkillCreateInput): Promise; + update(id: string, data: SkillUpdateInput): Promise; + delete(id: string): Promise; +} + +export class SkillRepository implements ISkillRepository { + constructor(private readonly prisma: PrismaClient) {} + + async findAll(projectId?: string): Promise { + const include = { project: { select: { name: true } } }; + if (projectId !== undefined) { + // Project-scoped + globals. + return this.prisma.skill.findMany({ + where: { OR: [{ projectId }, { projectId: null, agentId: null }] }, + include, + orderBy: { name: 'asc' }, + }); + } + return this.prisma.skill.findMany({ include, orderBy: { name: 'asc' } }); + } + + async findGlobal(): Promise { + return this.prisma.skill.findMany({ + where: { projectId: null, agentId: null }, + include: { project: { select: { name: true } } }, + orderBy: { name: 'asc' }, + }); + } + + async findByAgent(agentId: string): Promise { + return this.prisma.skill.findMany({ + where: { agentId }, + include: { agent: { select: { name: true } } }, + orderBy: { name: 'asc' }, + }); + } + + async findById(id: string): Promise { + return this.prisma.skill.findUnique({ + where: { id }, + include: { + project: { select: { name: true } }, + agent: { select: { name: true } }, + }, + }); + } + + async findByNameAndProject(name: string, projectId: string | null): Promise { + return this.prisma.skill.findUnique({ + where: { name_projectId: { name, projectId: projectId ?? '' } }, + }); + } + + async findByNameAndAgent(name: string, agentId: string | null): Promise { + return this.prisma.skill.findUnique({ + where: { name_agentId: { name, agentId: agentId ?? '' } }, + }); + } + + async create(data: SkillCreateInput): Promise { + return this.prisma.skill.create({ data }); + } + + async update(id: string, data: SkillUpdateInput): Promise { + return this.prisma.skill.update({ where: { id }, data }); + } + + async delete(id: string): Promise { + await this.prisma.skill.delete({ where: { id } }); + } +} diff --git a/src/mcpd/src/routes/skills.ts b/src/mcpd/src/routes/skills.ts new file mode 100644 index 0000000..5609450 --- /dev/null +++ b/src/mcpd/src/routes/skills.ts @@ -0,0 +1,147 @@ +import type { FastifyInstance } from 'fastify'; +import type { Skill } from '@prisma/client'; + +import type { SkillService } from '../services/skill.service.js'; +import type { IProjectRepository } from '../repositories/project.repository.js'; +import type { IAgentRepository } from '../repositories/agent.repository.js'; + +export function registerSkillRoutes( + app: FastifyInstance, + service: SkillService, + projectRepo: IProjectRepository, + agentRepo?: IAgentRepository, +): void { + // ── List ── + // Filter by `?project=`, `?projectId=`, `?agent=`, or `?scope=global`. + + app.get<{ Querystring: { project?: string; projectId?: string; agent?: string; scope?: string } }>( + '/api/v1/skills', + async (request) => { + const { project, projectId, agent, scope } = request.query; + let skills: Skill[]; + if (project !== undefined) { + const proj = await projectRepo.findByName(project); + if (proj === null) { + throw Object.assign(new Error(`Project not found: ${project}`), { statusCode: 404 }); + } + skills = await service.listSkills(proj.id); + } else if (projectId !== undefined) { + skills = await service.listSkills(projectId); + } else if (agent !== undefined) { + if (!agentRepo) { + throw Object.assign(new Error('Agent scoping not enabled'), { statusCode: 500 }); + } + const ag = await agentRepo.findByName(agent); + if (ag === null) { + throw Object.assign(new Error(`Agent not found: ${agent}`), { statusCode: 404 }); + } + skills = await service.listSkillsForAgent(ag.id); + } else if (scope === 'global') { + skills = await service.listGlobalSkills(); + } else { + skills = await service.listSkills(); + } + return skills; + }, + ); + + app.get<{ Params: { id: string } }>('/api/v1/skills/:id', async (request) => { + return service.getSkill(request.params.id); + }); + + // ── Create / Update / Delete ── + + app.post('/api/v1/skills', async (request, reply) => { + const body = request.body as Record; + const resolved: Record = { ...body }; + + if (typeof body['project'] === 'string') { + const proj = await projectRepo.findByName(body['project']); + if (proj === null) { + throw Object.assign(new Error(`Project not found: ${body['project']}`), { statusCode: 404 }); + } + resolved['projectId'] = proj.id; + delete resolved['project']; + } + if (typeof body['agent'] === 'string') { + if (!agentRepo) { + throw Object.assign(new Error('Agent scoping not enabled'), { statusCode: 500 }); + } + const ag = await agentRepo.findByName(body['agent']); + if (ag === null) { + throw Object.assign(new Error(`Agent not found: ${body['agent']}`), { statusCode: 404 }); + } + resolved['agentId'] = ag.id; + delete resolved['agent']; + } + const skill = await service.createSkill(resolved); + reply.code(201); + return skill; + }); + + app.put<{ Params: { id: string } }>('/api/v1/skills/:id', async (request) => { + return service.updateSkill(request.params.id, request.body); + }); + + app.delete<{ Params: { id: string } }>('/api/v1/skills/:id', async (request, reply) => { + await service.deleteSkill(request.params.id); + reply.code(204); + }); + + // ── Project-scoped views ── + + app.get<{ Params: { name: string } }>('/api/v1/projects/:name/skills', async (request) => { + const proj = await projectRepo.findByName(request.params.name); + if (proj === null) { + throw Object.assign(new Error(`Project not found: ${request.params.name}`), { statusCode: 404 }); + } + return service.listSkills(proj.id); + }); + + /** + * Compact view for `mcpctl skills sync` (PR-5). Returns metadata only — + * no `files`, no full `content` — so the client can decide which skills + * are stale before fetching the full body via /api/v1/skills/:id. + */ + app.get<{ Params: { name: string } }>( + '/api/v1/projects/:name/skills/visible', + async (request) => { + const proj = await projectRepo.findByName(request.params.name); + if (proj === null) { + throw Object.assign(new Error(`Project not found: ${request.params.name}`), { statusCode: 404 }); + } + return service.getVisibleSkills(proj.id); + }, + ); + + // ── Agent-scoped view ── + + app.get<{ Params: { agentName: string } }>( + '/api/v1/agents/:agentName/skills', + async (request, reply) => { + if (!agentRepo) { + throw Object.assign(new Error('Agent scoping not enabled'), { statusCode: 500 }); + } + const agent = await agentRepo.findByName(request.params.agentName); + if (agent === null) { + reply.code(404); + return { error: `Agent not found: ${request.params.agentName}` }; + } + return service.listSkillsForAgent(agent.id); + }, + ); + + // ── Restore from a revision ── + // POST /api/v1/skills/:id/restore-revision { revisionId, note? } + + app.post<{ Params: { id: string }; Body: { revisionId: string; note?: string } }>( + '/api/v1/skills/:id/restore-revision', + async (request) => { + const { revisionId, note } = request.body; + if (!revisionId) { + throw Object.assign(new Error('revisionId is required'), { statusCode: 400 }); + } + return service.restoreRevisionForSkill(request.params.id, revisionId, note); + }, + ); +} diff --git a/src/mcpd/src/services/backup/git-backup.service.ts b/src/mcpd/src/services/backup/git-backup.service.ts index d7cf86d..1acf0f3 100644 --- a/src/mcpd/src/services/backup/git-backup.service.ts +++ b/src/mcpd/src/services/backup/git-backup.service.ts @@ -737,6 +737,17 @@ export class GitBackupService { if (!r) throw new Error(`Prompt not found: ${name}`); return resourceToYaml('prompt', r as unknown as Record); } + case 'skill': { + const r = await this.prisma.skill.findFirst({ + where: { name }, + include: { + project: { select: { name: true } }, + agent: { select: { name: true } }, + }, + }); + if (!r) throw new Error(`Skill not found: ${name}`); + return resourceToYaml('skill', r as unknown as Record); + } case 'template': { const r = await this.prisma.mcpTemplate.findUnique({ where: { name } }); if (!r) throw new Error(`Template not found: ${name}`); diff --git a/src/mcpd/src/services/backup/yaml-serializer.ts b/src/mcpd/src/services/backup/yaml-serializer.ts index 4e9abd8..60c5f69 100644 --- a/src/mcpd/src/services/backup/yaml-serializer.ts +++ b/src/mcpd/src/services/backup/yaml-serializer.ts @@ -114,11 +114,11 @@ export function resourcePath(kind: string, name: string): string { } /** Resource kinds that are backed up. */ -export const BACKUP_KINDS = ['server', 'secret', 'project', 'user', 'group', 'rbac', 'prompt', 'template'] as const; +export const BACKUP_KINDS = ['server', 'secret', 'project', 'user', 'group', 'rbac', 'prompt', 'skill', 'template'] as const; export type BackupKind = (typeof BACKUP_KINDS)[number]; -/** Apply order: dependencies before dependents. */ -export const APPLY_ORDER: BackupKind[] = ['secret', 'server', 'template', 'user', 'group', 'project', 'rbac', 'prompt']; +/** Apply order: dependencies before dependents. Skills follow prompts. */ +export const APPLY_ORDER: BackupKind[] = ['secret', 'server', 'template', 'user', 'group', 'project', 'rbac', 'prompt', 'skill']; /** Parse a file path to extract kind and name. Returns null if path doesn't match backup structure. */ export function parseResourcePath(filePath: string): { kind: BackupKind; name: string } | null { @@ -129,7 +129,7 @@ export function parseResourcePath(filePath: string): { kind: BackupKind; name: s const kindMap: Record = { servers: 'server', secrets: 'secret', projects: 'project', users: 'user', groups: 'group', rbac: 'rbac', - prompts: 'prompt', templates: 'template', + prompts: 'prompt', skills: 'skill', templates: 'template', }; const kind = kindMap[dir!]; if (!kind) return null; @@ -188,6 +188,17 @@ export async function serializeAll(prisma: PrismaClient): Promise)); } + // Skills (with project + agent name) + const skills = await prisma.skill.findMany({ + include: { + project: { select: { name: true } }, + agent: { select: { name: true } }, + }, + }); + for (const s of skills) { + files.set(resourcePath('skill', s.name), resourceToYaml('skill', s as unknown as Record)); + } + // Templates const templates = await prisma.mcpTemplate.findMany(); for (const t of templates) { diff --git a/src/mcpd/src/services/skill.service.ts b/src/mcpd/src/services/skill.service.ts new file mode 100644 index 0000000..13d81bb --- /dev/null +++ b/src/mcpd/src/services/skill.service.ts @@ -0,0 +1,386 @@ +import type { Prisma, Skill } from '@prisma/client'; + +import type { ISkillRepository } from '../repositories/skill.repository.js'; +import type { IProjectRepository } from '../repositories/project.repository.js'; +import type { IAgentRepository } from '../repositories/agent.repository.js'; +import { CreateSkillSchema, UpdateSkillSchema } from '../validation/skill.schema.js'; +import { NotFoundError } from './mcp-server.service.js'; +import type { ResourceRevisionService } from './resource-revision.service.js'; +import type { ResourceProposalService } from './resource-proposal.service.js'; +import { bumpSemver, type BumpKind } from '../utils/semver.js'; + +export class SkillService { + private revisionService: ResourceRevisionService | null = null; + + constructor( + private readonly skillRepo: ISkillRepository, + private readonly projectRepo: IProjectRepository, + private readonly agentRepo?: IAgentRepository, + ) {} + + setRevisionService(service: ResourceRevisionService): void { + this.revisionService = service; + } + + /** + * Register a 'skill' approval handler with the proposal service. Mirrors + * PromptService's setup: approve = upsert skill body + record revision + + * link currentRevisionId, all inside a single transaction. + */ + setProposalService(service: ResourceProposalService): void { + service.setHandler('skill', async (proposal, tx, _approverUserId) => { + const body = (proposal.body ?? {}) as Record; + const content = String(body['content'] ?? ''); + const description = typeof body['description'] === 'string' ? body['description'] : ''; + const priority = typeof body['priority'] === 'number' ? body['priority'] : 5; + const files = (body['files'] ?? {}) as Prisma.InputJsonValue; + const metadata = (body['metadata'] ?? {}) as Prisma.InputJsonValue; + const projectId = proposal.projectId ?? null; + const agentId = proposal.agentId ?? null; + + const existing = agentId !== null + ? await tx.skill.findUnique({ where: { name_agentId: { name: proposal.name, agentId } } }) + : await tx.skill.findUnique({ where: { name_projectId: { name: proposal.name, projectId: projectId ?? '' } } }); + + let skillId: string; + let newSemver: string; + if (existing !== null) { + newSemver = bumpSemver(existing.semver, 'patch'); + await tx.skill.update({ + where: { id: existing.id }, + data: { content, description, priority, files, metadata, semver: newSemver }, + }); + skillId = existing.id; + } else { + newSemver = '0.1.0'; + const created = await tx.skill.create({ + data: { + name: proposal.name, + content, + description, + priority, + files, + metadata, + ...(projectId !== null ? { projectId } : {}), + ...(agentId !== null ? { agentId } : {}), + semver: newSemver, + }, + }); + skillId = created.id; + } + + const { revision } = await this.revisionService!.record( + { + resourceType: 'skill', + resourceId: skillId, + semver: newSemver, + body: { content, description, priority, files, metadata }, + ...(proposal.createdByUserId !== null ? { authorUserId: proposal.createdByUserId } : {}), + ...(proposal.createdBySession !== null ? { authorSessionId: proposal.createdBySession } : {}), + note: `approved proposal ${proposal.id}`, + }, + tx, + ); + + await tx.skill.update({ + where: { id: skillId }, + data: { currentRevisionId: revision.id }, + }); + + return { resourceId: skillId, revisionId: revision.id }; + }); + } + + // ── CRUD ── + + async listSkills(projectId?: string): Promise { + return this.skillRepo.findAll(projectId); + } + + async listGlobalSkills(): Promise { + return this.skillRepo.findGlobal(); + } + + async listSkillsForAgent(agentId: string): Promise { + return this.skillRepo.findByAgent(agentId); + } + + async getSkill(id: string): Promise { + const skill = await this.skillRepo.findById(id); + if (skill === null) throw new NotFoundError(`Skill not found: ${id}`); + return skill; + } + + async createSkill(input: unknown): Promise { + const data = CreateSkillSchema.parse(input); + + if (data.projectId !== undefined) { + const project = await this.projectRepo.findById(data.projectId); + if (project === null) throw new NotFoundError(`Project not found: ${data.projectId}`); + } + if (data.agentId !== undefined) { + if (this.agentRepo === undefined) { + throw new Error('Agent-scoped skills require AgentRepository to be wired into SkillService'); + } + const agent = await this.agentRepo.findById(data.agentId); + if (agent === null) throw new NotFoundError(`Agent not found: ${data.agentId}`); + } + + const createData: { + name: string; + content: string; + description?: string; + files?: Prisma.InputJsonValue; + metadata?: Prisma.InputJsonValue; + projectId?: string; + agentId?: string; + priority?: number; + semver?: string; + } = { + name: data.name, + content: data.content, + }; + if (data.description !== undefined) createData.description = data.description; + if (data.files !== undefined) createData.files = data.files as Prisma.InputJsonValue; + if (data.metadata !== undefined) createData.metadata = data.metadata as Prisma.InputJsonValue; + if (data.projectId !== undefined) createData.projectId = data.projectId; + if (data.agentId !== undefined) createData.agentId = data.agentId; + if (data.priority !== undefined) createData.priority = data.priority; + if (data.semver !== undefined) createData.semver = data.semver; + + const skill = await this.skillRepo.create(createData); + + if (this.revisionService) { + this.recordSkillRevision(skill, skill.semver, 'created').catch(() => {}); + } + return skill; + } + + async updateSkill(id: string, input: unknown): Promise { + const data = UpdateSkillSchema.parse(input); + if (data.semver !== undefined && data.bump !== undefined) { + throw Object.assign(new Error('Pass --semver or --bump, not both'), { statusCode: 400 }); + } + const existing = await this.getSkill(id); + + let newSemver = existing.semver; + const contentOrMetaChanged = + data.content !== undefined || + data.description !== undefined || + data.files !== undefined || + data.metadata !== undefined || + data.priority !== undefined; + + if (data.semver !== undefined) { + newSemver = data.semver; + } else if (data.bump !== undefined) { + newSemver = bumpSemver(existing.semver, data.bump as BumpKind); + } else if (contentOrMetaChanged) { + newSemver = bumpSemver(existing.semver, 'patch'); + } + + const updateData: { + content?: string; + description?: string; + files?: Prisma.InputJsonValue; + metadata?: Prisma.InputJsonValue; + priority?: number; + semver?: string; + } = {}; + if (data.content !== undefined) updateData.content = data.content; + if (data.description !== undefined) updateData.description = data.description; + if (data.files !== undefined) updateData.files = data.files as Prisma.InputJsonValue; + if (data.metadata !== undefined) updateData.metadata = data.metadata as Prisma.InputJsonValue; + if (data.priority !== undefined) updateData.priority = data.priority; + if (newSemver !== existing.semver) updateData.semver = newSemver; + + const skill = await this.skillRepo.update(id, updateData); + + const shouldRecord = + contentOrMetaChanged || data.bump !== undefined || data.semver !== undefined; + if (this.revisionService && shouldRecord) { + this.recordSkillRevision(skill, newSemver, data.note ?? null).catch(() => {}); + } + return skill; + } + + /** Best-effort revision write — same shape as PromptService. */ + private async recordSkillRevision(skill: Skill, semver: string, note: string | null): Promise { + if (this.revisionService === null) return; + const body: Record = { + content: skill.content, + description: skill.description, + priority: skill.priority, + files: skill.files, + metadata: skill.metadata, + }; + const { revision } = await this.revisionService.record({ + resourceType: 'skill', + resourceId: skill.id, + semver, + body, + ...(note !== null ? { note } : {}), + }); + await this.skillRepo.update(skill.id, { currentRevisionId: revision.id }); + } + + async restoreRevisionForSkill(skillId: string, revisionId: string, note?: string): Promise { + if (this.revisionService === null) { + throw new Error('Revision service not wired'); + } + const revision = await this.revisionService.getById(revisionId); + if (revision === null) throw new NotFoundError(`Revision not found: ${revisionId}`); + if (revision.resourceType !== 'skill' || revision.resourceId !== skillId) { + throw Object.assign( + new Error('Revision does not belong to this skill'), + { statusCode: 400 }, + ); + } + const body = (revision.body ?? {}) as Record; + return this.updateSkill(skillId, { + content: typeof body['content'] === 'string' ? body['content'] : undefined, + description: typeof body['description'] === 'string' ? body['description'] : undefined, + priority: typeof body['priority'] === 'number' ? body['priority'] : undefined, + files: body['files'] as Record | undefined, + metadata: body['metadata'] as Record | undefined, + bump: 'patch', + note: note ?? `restored from revision ${revisionId}`, + }); + } + + async deleteSkill(id: string): Promise { + await this.getSkill(id); // 404 if missing + await this.skillRepo.delete(id); + } + + // ── Backup/restore helpers ── + + async upsertByName(data: Record): Promise { + const name = data['name'] as string; + let projectId: string | null = null; + let agentId: string | null = null; + + if (data['project'] !== undefined) { + const project = await this.projectRepo.findByName(data['project'] as string); + if (project === null) throw new NotFoundError(`Project not found: ${data['project']}`); + projectId = project.id; + } else if (data['projectId'] !== undefined) { + projectId = data['projectId'] as string; + } + + if (data['agent'] !== undefined) { + if (this.agentRepo === undefined) { + throw new Error('Agent-scoped skills require AgentRepository to be wired into SkillService'); + } + const agent = await this.agentRepo.findByName(data['agent'] as string); + if (agent === null) throw new NotFoundError(`Agent not found: ${data['agent']}`); + agentId = agent.id; + } else if (data['agentId'] !== undefined) { + agentId = data['agentId'] as string; + } + + if (projectId !== null && agentId !== null) { + throw Object.assign( + new Error('A skill may attach to a project XOR an agent, not both'), + { statusCode: 400 }, + ); + } + + const existing = agentId !== null + ? await this.skillRepo.findByNameAndAgent(name, agentId) + : await this.skillRepo.findByNameAndProject(name, projectId); + + if (existing !== null) { + const updateData: { + content?: string; + description?: string; + priority?: number; + files?: Prisma.InputJsonValue; + metadata?: Prisma.InputJsonValue; + } = {}; + if (data['content'] !== undefined) updateData.content = data['content'] as string; + if (data['description'] !== undefined) updateData.description = data['description'] as string; + if (data['priority'] !== undefined) updateData.priority = data['priority'] as number; + if (data['files'] !== undefined) updateData.files = data['files'] as Prisma.InputJsonValue; + if (data['metadata'] !== undefined) updateData.metadata = data['metadata'] as Prisma.InputJsonValue; + if (Object.keys(updateData).length > 0) { + return this.skillRepo.update(existing.id, updateData); + } + return existing; + } + + const createData: { + name: string; + content: string; + description?: string; + files?: Prisma.InputJsonValue; + metadata?: Prisma.InputJsonValue; + projectId?: string; + agentId?: string; + priority?: number; + } = { + name, + content: (data['content'] as string) ?? '', + }; + if (data['description'] !== undefined) createData.description = data['description'] as string; + if (data['files'] !== undefined) createData.files = data['files'] as Prisma.InputJsonValue; + if (data['metadata'] !== undefined) createData.metadata = data['metadata'] as Prisma.InputJsonValue; + if (projectId !== null) createData.projectId = projectId; + if (agentId !== null) createData.agentId = agentId; + if (data['priority'] !== undefined) createData.priority = data['priority'] as number; + + return this.skillRepo.create(createData); + } + + async deleteByName(name: string): Promise { + const all = await this.skillRepo.findAll(); + const match = all.find((s) => s.name === name); + if (match === undefined) return; + await this.skillRepo.delete(match.id); + } + + /** + * Visibility for `mcpctl skills sync` (PR-5). Returns metadata only — + * no `files` or full `content` — so the diff path can quickly decide + * what's stale via contentHash + semver before fetching bodies. + */ + async getVisibleSkills(projectId?: string): Promise> { + const skills = await this.skillRepo.findAll(projectId); + const out: Array<{ + id: string; + name: string; + description: string; + semver: string; + contentHash: string | null; + metadata: unknown; + scope: 'project' | 'global' | 'agent'; + }> = []; + for (const s of skills) { + let scope: 'project' | 'global' | 'agent' = 'global'; + if (s.projectId !== null) scope = 'project'; + else if (s.agentId !== null) scope = 'agent'; + out.push({ + id: s.id, + name: s.name, + description: s.description, + semver: s.semver, + // contentHash lives on the latest revision row; sync clients can + // fetch it via /api/v1/revisions?resourceType=skill&resourceId=... + // until the resource row carries it directly. PR-5 will likely + // promote contentHash onto the resource itself. + contentHash: null, + metadata: s.metadata, + scope, + }); + } + return out; + } +} diff --git a/src/mcpd/src/validation/skill.schema.ts b/src/mcpd/src/validation/skill.schema.ts new file mode 100644 index 0000000..7dc9655 --- /dev/null +++ b/src/mcpd/src/validation/skill.schema.ts @@ -0,0 +1,88 @@ +import { z } from 'zod'; + +const SEMVER_RE = /^\d+\.\d+\.\d+$/; +const NAME_RE = /^[a-z0-9-]+$/; + +/** + * Typed Skill metadata. Stored opaquely as `Skill.metadata` Json in the + * database; validated app-layer when callers pass it through CreateSkill/ + * UpdateSkill. The fields below are the ones the sync command (PR-5) will + * actually act on: + * + * - `hooks` — declarative SessionStart / PreToolUse / PostToolUse + * entries that mcpctl skills sync registers in + * ~/.claude/settings.json with `_mcpctl_managed: true`. + * - `mcpServers` — upstream MCP server dependencies the skill needs; + * sync auto-attaches them to the project (corporate + * trust model — no consent prompt). + * - `postInstall` — relative path inside `files{}` to a script that + * sync runs as the user when the skill's contentHash + * first appears or changes. 60-s default timeout; + * audit event emitted back to mcpd. + * - `preUninstall` — symmetric to postInstall, runs on orphan removal. + * - `postInstallTimeoutSec` — per-skill override for the 60-s default. + * + * .passthrough() so unknown fields survive the round-trip — forward + * compatibility for follow-on metadata additions. + */ + +const ManagedHookEntrySchema = z.object({ + type: z.literal('command'), + command: z.string().min(1).max(4000), + timeout: z.number().int().min(1).max(3600).optional(), +}).passthrough(); + +const HooksSchema = z.object({ + PreToolUse: z.array(ManagedHookEntrySchema).optional(), + PostToolUse: z.array(ManagedHookEntrySchema).optional(), + SessionStart: z.array(ManagedHookEntrySchema).optional(), + Stop: z.array(ManagedHookEntrySchema).optional(), + SubagentStop: z.array(ManagedHookEntrySchema).optional(), + Notification: z.array(ManagedHookEntrySchema).optional(), +}).strict().optional(); + +const McpServerDepSchema = z.object({ + name: z.string().regex(NAME_RE), + fromTemplate: z.string().min(1), + project: z.string().regex(NAME_RE).optional(), +}).strict(); + +export const SkillMetadataSchema = z.object({ + hooks: HooksSchema, + mcpServers: z.array(McpServerDepSchema).optional(), + postInstall: z.string().min(1).max(500).optional(), + preUninstall: z.string().min(1).max(500).optional(), + postInstallTimeoutSec: z.number().int().min(1).max(600).optional(), +}).passthrough(); + +export const CreateSkillSchema = z + .object({ + name: z.string().min(1).max(100).regex(NAME_RE, 'Name must be lowercase alphanumeric with hyphens'), + content: z.string().min(1).max(200_000), + description: z.string().max(500).optional(), + files: z.record(z.string()).optional(), + metadata: SkillMetadataSchema.optional(), + projectId: z.string().optional(), + agentId: z.string().optional(), + priority: z.number().int().min(1).max(10).default(5).optional(), + semver: z.string().regex(SEMVER_RE, 'Semver must be MAJOR.MINOR.PATCH').optional(), + }) + .refine( + (data) => !(data.projectId !== undefined && data.agentId !== undefined), + { message: 'A skill may attach to a project XOR an agent, not both', path: ['agentId'] }, + ); + +export const UpdateSkillSchema = z.object({ + content: z.string().min(1).max(200_000).optional(), + description: z.string().max(500).optional(), + files: z.record(z.string()).optional(), + metadata: SkillMetadataSchema.optional(), + priority: z.number().int().min(1).max(10).optional(), + semver: z.string().regex(SEMVER_RE, 'Semver must be MAJOR.MINOR.PATCH').optional(), + bump: z.enum(['major', 'minor', 'patch']).optional(), + note: z.string().max(500).optional(), +}); + +export type CreateSkillInput = z.infer; +export type UpdateSkillInput = z.infer; +export type SkillMetadata = z.infer; diff --git a/src/mcpd/tests/yaml-serializer.test.ts b/src/mcpd/tests/yaml-serializer.test.ts index 7f54fc3..a7831c8 100644 --- a/src/mcpd/tests/yaml-serializer.test.ts +++ b/src/mcpd/tests/yaml-serializer.test.ts @@ -228,6 +228,7 @@ describe('APPLY_ORDER', () => { }); it('has all backup kinds', () => { - expect(APPLY_ORDER).toHaveLength(8); + // PR-3: bumped from 8 → 9 with the addition of `skill`. + expect(APPLY_ORDER).toHaveLength(9); }); });