From db57bb585650161dd812b63995baf6f7d32d817d Mon Sep 17 00:00:00 2001 From: Michal Date: Thu, 7 May 2026 13:13:33 +0100 Subject: [PATCH] feat(mcpd+mcplocal+cli): propose-learnings system skill, propose_skill MCP tool, mcpctl review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 4 of the Skills + Revisions + Proposals work. Closes the reflexive loop: Claude sessions can now propose back content (prompts or skills) that maintainers triage via a CLI queue. The system documents itself to Claude through the same mechanism it documents to humans. ## What's added ### propose-learnings global skill (mcpd bootstrap) - src/mcpd/src/bootstrap/system-skills.ts — idempotent upsert, mirrors system-project.ts. Single skill seeded today: `propose-learnings`, ~430 words, explains when to engage with propose_prompt vs propose_skill, what makes a good proposal, what NOT to propose, and the review→approve flow. Priority 9, global scope. - main.ts: `bootstrapSystemSkills(prisma)` called right after `bootstrapSystemProject`. ### gate-encouragement-propose system prompt - system-project.ts gains a new gate prompt (priority 10, alongside the other gate-* prompts) that nudges Claude to call propose_prompt when it discovers a project-specific lesson. Pairs with the propose-learnings skill — the prompt is the trigger, the skill is the manual. ### propose_skill MCP tool (mcplocal) - proxymodel/plugins/gate.ts: new virtual tool registered alongside propose_prompt. Posts to /api/v1/proposals (the new endpoint from PR-2) with resourceType='skill'. Tool description steers Claude toward propose_prompt for project-specific knowledge and reserves propose_skill for cross-cutting cases. propose_prompt's tool description is also expanded to point at the propose-learnings skill for guidance — the bare "creates a pending request" copy was bland enough that nothing in Claude's prior would actually make it engage. ### mcpctl review CLI - New top-level command in src/cli/src/commands/review.ts. Subcommands: mcpctl review pending List pending proposals mcpctl review next Show oldest pending mcpctl review show Full detail mcpctl review approve POST /proposals/:id/approve mcpctl review reject --reason "..." mcpctl review diff Side-by-side current vs proposed - Wired into src/cli/src/index.ts. Registered after createApproveCommand to keep the existing project-ops `mcpctl approve promptrequest` command working (legacy) while the new review surface is the preferred path. ## Tests touched - bootstrap-system-project.test.ts already counts via getSystemPromptNames() length, so it picked up the new prompt automatically; only the priority assertion needed nothing — the new prompt starts with `gate-` so the existing `gate-* → priority 10` invariant validates it. - system-prompt-validation.test.ts: bumped expected length from 11→12 and added a `toContain('gate-encouragement-propose')` assertion. Full suite: 158 test files / 2127 tests green. ## What's NOT in this PR - A SkillService mock-based test for the proposal approval handler — the PromptService approval handler is structurally identical and already covered; the database-backed integration is exercised in PR-2's tests. - Changes to mcplocal's existing handleProposePrompt URL — it still POSTs to the legacy /api/v1/projects/.../promptrequests endpoint, which works because PR-2 left that route in place. PR-7 will cut mcplocal over to /api/v1/proposals along with the PromptRequest table rename + drop. Co-Authored-By: Claude Opus 4.7 (1M context) --- completions/mcpctl.bash | 32 ++- completions/mcpctl.fish | 21 +- src/cli/src/commands/review.ts | 220 ++++++++++++++++++ src/cli/src/index.ts | 6 + src/mcpd/src/bootstrap/system-project.ts | 12 + src/mcpd/src/bootstrap/system-skills.ts | 133 +++++++++++ src/mcpd/src/main.ts | 3 + .../tests/system-prompt-validation.test.ts | 7 +- src/mcplocal/src/proxymodel/plugins/gate.ts | 80 ++++++- 9 files changed, 508 insertions(+), 6 deletions(-) create mode 100644 src/cli/src/commands/review.ts create mode 100644 src/mcpd/src/bootstrap/system-skills.ts diff --git a/completions/mcpctl.bash b/completions/mcpctl.bash index 76997a9..ad3ffb8 100644 --- a/completions/mcpctl.bash +++ b/completions/mcpctl.bash @@ -5,7 +5,7 @@ _mcpctl() { local cur prev words cword _init_completion || return - local commands="status login logout config get describe delete logs create edit apply chat chat-llm patch backup approve console cache provider test migrate rotate" + local commands="status login logout config get describe delete logs create edit apply chat chat-llm patch backup approve review console cache provider test migrate rotate" local project_commands="get describe delete logs create edit attach-server detach-server" local global_opts="-v --version --daemon-url --direct -p --project -h --help" local resources="servers instances secrets secretbackends llms agents personalities templates projects users groups rbac prompts promptrequests serverattachments proxymodels inference-tasks all" @@ -317,6 +317,36 @@ _mcpctl() { COMPREPLY=($(compgen -W "$names -h --help" -- "$cur")) fi return ;; + review) + local review_sub=$(_mcpctl_get_subcmd $subcmd_pos) + if [[ -z "$review_sub" ]]; then + COMPREPLY=($(compgen -W "pending next show approve reject diff help" -- "$cur")) + else + case "$review_sub" in + pending) + COMPREPLY=($(compgen -W "--type -h --help" -- "$cur")) + ;; + next) + COMPREPLY=($(compgen -W "--type -h --help" -- "$cur")) + ;; + show) + COMPREPLY=($(compgen -W "-h --help" -- "$cur")) + ;; + approve) + COMPREPLY=($(compgen -W "-h --help" -- "$cur")) + ;; + reject) + COMPREPLY=($(compgen -W "--reason -h --help" -- "$cur")) + ;; + diff) + COMPREPLY=($(compgen -W "-h --help" -- "$cur")) + ;; + *) + COMPREPLY=($(compgen -W "-h --help" -- "$cur")) + ;; + esac + fi + return ;; mcp) COMPREPLY=($(compgen -W "-p --project -h --help" -- "$cur")) return ;; diff --git a/completions/mcpctl.fish b/completions/mcpctl.fish index cb71e0a..46afbf1 100644 --- a/completions/mcpctl.fish +++ b/completions/mcpctl.fish @@ -4,7 +4,7 @@ # Erase any stale completions from previous versions complete -c mcpctl -e -set -l commands status login logout config get describe delete logs create edit apply chat chat-llm patch backup approve console cache provider test migrate rotate +set -l commands status login logout config get describe delete logs create edit apply chat chat-llm patch backup approve review console cache provider test migrate rotate set -l project_commands get describe delete logs create edit attach-server detach-server # Disable file completions by default @@ -236,6 +236,7 @@ complete -c mcpctl -n "not __mcpctl_has_project; and not __fish_seen_subcommand_ complete -c mcpctl -n "not __mcpctl_has_project; and not __fish_seen_subcommand_from $commands" -a patch -d 'Patch a resource field (e.g. mcpctl patch project myproj llmProvider=none)' complete -c mcpctl -n "not __mcpctl_has_project; and not __fish_seen_subcommand_from $commands" -a backup -d 'Git-based backup status and management' complete -c mcpctl -n "not __mcpctl_has_project; and not __fish_seen_subcommand_from $commands" -a approve -d 'Approve a pending prompt request (atomic: delete request, create prompt)' +complete -c mcpctl -n "not __mcpctl_has_project; and not __fish_seen_subcommand_from $commands" -a review -d 'Triage proposed prompts and skills' complete -c mcpctl -n "not __mcpctl_has_project; and not __fish_seen_subcommand_from $commands" -a console -d 'Interactive MCP console — unified timeline with tools, provenance, and lab replay' complete -c mcpctl -n "not __mcpctl_has_project; and not __fish_seen_subcommand_from $commands" -a cache -d 'Manage ProxyModel pipeline cache' complete -c mcpctl -n "not __mcpctl_has_project; and not __fish_seen_subcommand_from $commands" -a provider -d 'Control local LLM providers (start/stop/status)' @@ -453,6 +454,24 @@ complete -c mcpctl -n "__fish_seen_subcommand_from backup; and not __fish_seen_s # backup log options complete -c mcpctl -n "__mcpctl_subcmd_active backup log" -s n -l limit -d 'number of commits to show' -x +# review subcommands +set -l review_cmds pending next show approve reject diff +complete -c mcpctl -n "__fish_seen_subcommand_from review; and not __fish_seen_subcommand_from $review_cmds" -a pending -d 'List pending proposals' +complete -c mcpctl -n "__fish_seen_subcommand_from review; and not __fish_seen_subcommand_from $review_cmds" -a next -d 'Show the oldest pending proposal' +complete -c mcpctl -n "__fish_seen_subcommand_from review; and not __fish_seen_subcommand_from $review_cmds" -a show -d 'Show full detail of a proposal' +complete -c mcpctl -n "__fish_seen_subcommand_from review; and not __fish_seen_subcommand_from $review_cmds" -a approve -d 'Approve a pending proposal (creates the resource + initial revision)' +complete -c mcpctl -n "__fish_seen_subcommand_from review; and not __fish_seen_subcommand_from $review_cmds" -a reject -d 'Reject a pending proposal with a reviewer note' +complete -c mcpctl -n "__fish_seen_subcommand_from review; and not __fish_seen_subcommand_from $review_cmds" -a diff -d 'Show what would change if this proposal were approved' + +# review pending options +complete -c mcpctl -n "__mcpctl_subcmd_active review pending" -l type -d 'Filter by resource type: prompt or skill' -x + +# review next options +complete -c mcpctl -n "__mcpctl_subcmd_active review next" -l type -d 'Filter by resource type: prompt or skill' -x + +# review reject options +complete -c mcpctl -n "__mcpctl_subcmd_active review reject" -l reason -d 'Reviewer note explaining the rejection' -x + # cache subcommands set -l cache_cmds stats clear complete -c mcpctl -n "__fish_seen_subcommand_from cache; and not __fish_seen_subcommand_from $cache_cmds" -a stats -d 'Show cache statistics' diff --git a/src/cli/src/commands/review.ts b/src/cli/src/commands/review.ts new file mode 100644 index 0000000..b0498b7 --- /dev/null +++ b/src/cli/src/commands/review.ts @@ -0,0 +1,220 @@ +import { Command } from 'commander'; +import type { ApiClient } from '../api-client.js'; + +/** + * `mcpctl review` — triage UX for the proposal queue. Wraps the + * /api/v1/proposals endpoints so reviewers don't have to hand-curl the + * API. Subcommands: + * + * mcpctl review pending List pending proposals + * mcpctl review next Show oldest pending + * mcpctl review show Full detail of one proposal + * mcpctl review approve Approve (creates resource + revision) + * mcpctl review reject --reason Reject with reviewer note + * mcpctl review diff Diff proposal body vs current resource + */ + +interface Proposal { + id: string; + resourceType: 'prompt' | 'skill'; + name: string; + body: Record; + projectId: string | null; + agentId: string | null; + createdBySession: string | null; + createdByUserId: string | null; + status: 'pending' | 'approved' | 'rejected'; + reviewerNote: string; + approvedRevisionId: string | null; + createdAt: string; + updatedAt: string; + project?: { name: string } | null; + agent?: { name: string } | null; +} + +export interface ReviewCommandDeps { + client: ApiClient; + log: (...args: unknown[]) => void; +} + +export function createReviewCommand(deps: ReviewCommandDeps): Command { + const { client, log } = deps; + + const cmd = new Command('review').description('Triage proposed prompts and skills'); + + cmd.command('pending') + .alias('list') + .description('List pending proposals') + .option('--type ', 'Filter by resource type: prompt or skill') + .action(async (opts: { type?: string }) => { + const params = new URLSearchParams({ status: 'pending' }); + if (opts.type) params.set('resourceType', opts.type); + const proposals = await client.get(`/api/v1/proposals?${params.toString()}`); + if (proposals.length === 0) { + log('No pending proposals.'); + return; + } + log(formatTable(proposals)); + }); + + cmd.command('next') + .description('Show the oldest pending proposal') + .option('--type ', 'Filter by resource type: prompt or skill') + .action(async (opts: { type?: string }) => { + const params = new URLSearchParams({ status: 'pending' }); + if (opts.type) params.set('resourceType', opts.type); + const proposals = await client.get(`/api/v1/proposals?${params.toString()}`); + if (proposals.length === 0) { + log('No pending proposals.'); + return; + } + // /api/v1/proposals returns latest-first; we want the oldest pending. + const oldest = proposals[proposals.length - 1] as Proposal; + log(formatDetail(oldest)); + }); + + cmd.command('show') + .description('Show full detail of a proposal') + .argument('', 'Proposal ID') + .action(async (id: string) => { + const proposal = await client.get(`/api/v1/proposals/${id}`); + log(formatDetail(proposal)); + }); + + cmd.command('approve') + .description('Approve a pending proposal (creates the resource + initial revision)') + .argument('', 'Proposal ID') + .action(async (id: string) => { + const updated = await client.post(`/api/v1/proposals/${id}/approve`, {}); + log(`approved proposal '${updated.name}' (resourceType: ${updated.resourceType})`); + if (updated.approvedRevisionId) { + log(` resulting revision: ${updated.approvedRevisionId}`); + } + }); + + cmd.command('reject') + .description('Reject a pending proposal with a reviewer note') + .argument('', 'Proposal ID') + .option('--reason ', 'Reviewer note explaining the rejection') + .action(async (id: string, opts: { reason?: string }) => { + if (!opts.reason) { + throw new Error('--reason is required when rejecting a proposal'); + } + await client.post(`/api/v1/proposals/${id}/reject`, { reviewerNote: opts.reason }); + log(`rejected proposal ${id}`); + }); + + cmd.command('diff') + .description('Show what would change if this proposal were approved') + .argument('', 'Proposal ID') + .action(async (id: string) => { + const proposal = await client.get(`/api/v1/proposals/${id}`); + const proposedContent = (proposal.body as { content?: string }).content ?? ''; + + // Find existing resource (if any) to diff against. Both prompts and + // skills are scoped by (name, projectId|agentId|null=global). + let existingContent: string | null = null; + const projectName = proposal.project?.name; + const agentName = proposal.agent?.name; + try { + if (proposal.resourceType === 'prompt') { + const params = new URLSearchParams(); + if (projectName) params.set('project', projectName); + const list = await client.get>(`/api/v1/prompts?${params.toString()}`); + const match = list.find((p) => p.name === proposal.name); + if (match) existingContent = match.content; + } else { + const params = new URLSearchParams(); + if (projectName) params.set('project', projectName); + else if (agentName) params.set('agent', agentName); + const list = await client.get>(`/api/v1/skills?${params.toString()}`); + const match = list.find((s) => s.name === proposal.name); + if (match) existingContent = match.content; + } + } catch { + // 404 from no project / agent means nothing to diff against. + } + + if (existingContent === null) { + log(`Proposal would create a new ${proposal.resourceType} '${proposal.name}'.`); + log(''); + log('--- proposed body ---'); + log(proposedContent); + return; + } + + log(`Proposal would update the existing ${proposal.resourceType} '${proposal.name}'.`); + log(''); + log('--- current ---'); + log(existingContent); + log('--- proposed ---'); + log(proposedContent); + }); + + return cmd; +} + +// ── Formatting ── + +function formatTable(proposals: Proposal[]): string { + const lines: string[] = []; + const idW = Math.max(2, ...proposals.map((p) => p.id.length)); + const typeW = 6; // 'skill' / 'prompt' + const nameW = Math.max(4, ...proposals.map((p) => p.name.length)); + const scopeW = Math.max(5, ...proposals.map((p) => scopeLabel(p).length)); + const sessW = 8; + + const header = `${pad('ID', idW)} ${pad('TYPE', typeW)} ${pad('NAME', nameW)} ${pad('SCOPE', scopeW)} ${pad('SESSION', sessW)} AGE`; + lines.push(header); + for (const p of proposals) { + const age = ageOf(p.createdAt); + lines.push( + `${pad(p.id, idW)} ${pad(p.resourceType, typeW)} ${pad(p.name, nameW)} ${pad(scopeLabel(p), scopeW)} ${pad((p.createdBySession ?? '—').slice(0, 8), sessW)} ${age}`, + ); + } + return lines.join('\n'); +} + +function formatDetail(p: Proposal): string { + const lines: string[] = []; + lines.push(`=== Proposal: ${p.name} (${p.resourceType}) ===`); + lines.push(`ID: ${p.id}`); + lines.push(`Status: ${p.status}`); + lines.push(`Scope: ${scopeLabel(p)}`); + lines.push(`Created: ${p.createdAt} (session ${p.createdBySession ?? '—'})`); + if (p.reviewerNote) lines.push(`Reviewer note: ${p.reviewerNote}`); + if (p.approvedRevisionId) lines.push(`Approved as revision: ${p.approvedRevisionId}`); + lines.push(''); + lines.push('--- body ---'); + const content = (p.body as { content?: string }).content; + if (typeof content === 'string') { + lines.push(content); + } else { + lines.push(JSON.stringify(p.body, null, 2)); + } + return lines.join('\n'); +} + +function scopeLabel(p: Proposal): string { + if (p.project?.name) return `project:${p.project.name}`; + if (p.agent?.name) return `agent:${p.agent.name}`; + return 'global'; +} + +function pad(s: string, w: number): string { + if (s.length >= w) return s; + return s + ' '.repeat(w - s.length); +} + +function ageOf(iso: string): string { + const t = Date.parse(iso); + if (Number.isNaN(t)) return '?'; + const sec = Math.floor((Date.now() - t) / 1000); + if (sec < 60) return `${String(sec)}s`; + const min = Math.floor(sec / 60); + if (min < 60) return `${String(min)}m`; + const hr = Math.floor(min / 60); + if (hr < 24) return `${String(hr)}h`; + const days = Math.floor(hr / 24); + return `${String(days)}d`; +} diff --git a/src/cli/src/index.ts b/src/cli/src/index.ts index 26bb8fb..c9cd63a 100644 --- a/src/cli/src/index.ts +++ b/src/cli/src/index.ts @@ -23,6 +23,7 @@ import { createChatCommand } from './commands/chat.js'; import { createChatLlmCommand } from './commands/chat-llm.js'; import { createMigrateCommand } from './commands/migrate.js'; import { createRotateCommand } from './commands/rotate.js'; +import { createReviewCommand } from './commands/review.js'; import { ApiClient, ApiError } from './api-client.js'; import { loadConfig } from './config/index.js'; import { loadCredentials } from './auth/index.js'; @@ -268,6 +269,11 @@ export function createProgram(): Command { program.addCommand(createAttachServerCommand(projectOpsDeps), { hidden: true }); program.addCommand(createDetachServerCommand(projectOpsDeps), { hidden: true }); program.addCommand(createApproveCommand(projectOpsDeps)); + // PR-4: reviewer queue for proposed prompts + skills. + program.addCommand(createReviewCommand({ + client, + log: (...args) => console.log(...args), + })); program.addCommand(createMcpCommand({ getProject: () => program.opts().project as string | undefined, }), { hidden: true }); diff --git a/src/mcpd/src/bootstrap/system-project.ts b/src/mcpd/src/bootstrap/system-project.ts index 27b08d9..322cd4d 100644 --- a/src/mcpd/src/bootstrap/system-project.ts +++ b/src/mcpd/src/bootstrap/system-project.ts @@ -37,6 +37,18 @@ The available tools and prompts are listed below. After calling begin_session, y content: `If any of the listed prompts seem relevant to your work, or if you encounter unfamiliar patterns, conventions, or constraints during implementation, use read_prompts({ tags: [...] }) to retrieve them. It is better to check and not need it than to proceed without important context. The project maintainers have documented common pitfalls, architecture decisions, and required patterns — taking 10 seconds to retrieve a prompt can save hours of rework.`, + }, + { + // Pairs with the propose-learnings global skill (PR-4). The skill + // explains the proposal flow in detail; this nudge sits in the + // gating bundle so Claude actually thinks about engaging with it. + // Priority 10 keeps it visible alongside other gate-* prompts; the + // bootstrap test enforces this convention. + name: 'gate-encouragement-propose', + priority: 10, + content: `If during this session you discover a non-obvious project convention, hidden constraint, or lesson learned that future sessions would benefit from, call propose_prompt with the content + a name and brief description. A maintainer reviews it; if approved, future sessions get it as part of their gating bundle. + +Reserve propose_skill for cross-cutting knowledge — a debugging discipline, release hygiene, or review style — not project-specific facts. See the propose-learnings skill for guidance.`, }, { name: 'gate-intercept-preamble', diff --git a/src/mcpd/src/bootstrap/system-skills.ts b/src/mcpd/src/bootstrap/system-skills.ts new file mode 100644 index 0000000..009998f --- /dev/null +++ b/src/mcpd/src/bootstrap/system-skills.ts @@ -0,0 +1,133 @@ +/** + * Bootstrap built-in global skills on every mcpd startup. + * + * Mirrors `bootstrap/system-project.ts` for prompts: idempotent upserts so + * deletes + re-creates work, but never overwrites user edits to the skill + * content. The single skill seeded today is `propose-learnings` — it + * encourages Claude to actually use the propose_prompt / propose_skill + * MCP tools when it discovers something worth capturing. + * + * The skill lives at global scope (no projectId, no agentId) and gets + * synced onto every dev machine by `mcpctl skills sync` (PR-5). + */ + +import type { PrismaClient } from '@prisma/client'; + +interface SystemSkillDef { + name: string; + description: string; + priority: number; + content: string; +} + +const PROPOSE_LEARNINGS_CONTENT = `# propose-learnings + +This Claude Code session is connected to mcpctl, which keeps a curated +library of project knowledge — patterns, gotchas, conventions, and +policies — and makes that knowledge available to future sessions. Two +tools matter here: + +- \`propose_prompt\` — propose a small piece of text knowledge for the + current project. A reviewer approves it, then it appears for every + future session on this project as a \`read_prompts\` candidate. +- \`propose_skill\` — propose a Claude Code skill (a SKILL.md). Use this + only when the knowledge is genuinely cross-cutting (debugging + discipline, release hygiene, security review style) and would help + across many projects, not just this one. + +## When to engage + +Most sessions don't need to propose anything. Engage when one of these +is true and the cost of the next session re-discovering it would be +real: + +1. You hit a bug that took non-trivial digging to root-cause, and the + cause was a project-specific quirk a future you would have benefited + from knowing in advance ("this codebase shadows \`request\` with + \`req\` in three files; grep for both"). +2. You learned a convention by reading code that wasn't documented + anywhere ("services live under \`src/mcpd/src/services\` and are + wired in \`main.ts\` around line 466"). +3. The user told you something corrective ("we don't use Prisma + transactions for migrations here, we use raw SQL files") that would + otherwise be lost. + +## When NOT to engage + +- Anything you read in an existing prompt — it's already captured. +- Generic programming advice. Future sessions have the same training + as you. +- Speculation. Only propose what you actually verified during this + session. +- Anything secret, anything PII, anything that names a customer. + +## How to write a good proposal + +Name it \`lowercase-with-hyphens\`. Keep it under 200 words. Lead with +the shape of the situation, not the resolution — future-you needs to +recognise when this applies. Example: + +> name: prisma-null-fk-workaround +> content: When a Prisma model has an optional FK that's part of a +> compound \`@@unique\`, Postgres treats NULL as distinct, so duplicates +> sneak in. Workaround in this repo: store empty string instead of NULL +> for the FK and use \`?? ''\` at every read site. See +> \`src/mcpd/src/repositories/prompt.repository.ts:75\` for the pattern. + +## How proposals get applied + +Proposals enter a queue. A maintainer runs \`mcpctl review next\`, sees +a diff, and either approves (the prompt or skill goes live for the next +session) or rejects with a note. You will not see the outcome in this +session. That's fine — the system is designed so individual sessions +don't need to follow up. + +If you're unsure whether something is worth proposing, lean toward yes +for prompts (cheap to add, easy to reject) and lean toward no for +skills (harder to scope, larger blast radius). +`; + +const SYSTEM_SKILLS: SystemSkillDef[] = [ + { + name: 'propose-learnings', + description: + 'How and when to use propose_prompt / propose_skill to capture project knowledge for future sessions.', + priority: 9, + content: PROPOSE_LEARNINGS_CONTENT, + }, +]; + +/** + * Ensure system-owned global skills exist. Safe to call on every startup. + * If a user has edited or deleted a system skill, we leave their edit + * alone — same policy as system-project.ts. + */ +export async function bootstrapSystemSkills(prisma: PrismaClient): Promise { + for (const def of SYSTEM_SKILLS) { + const existing = await prisma.skill.findFirst({ + where: { name: def.name, projectId: null, agentId: null }, + }); + if (existing === null) { + await prisma.skill.create({ + data: { + name: def.name, + description: def.description, + priority: def.priority, + content: def.content, + // semver/files/metadata default to schema defaults. + }, + }); + } + // If it exists, leave the user's edits alone. + } +} + +/** Names of all system-seeded skills — useful for delete protection later. */ +export function getSystemSkillNames(): string[] { + return SYSTEM_SKILLS.map((s) => s.name); +} + +/** Default content for a system skill (for reset-on-delete). */ +export function getSystemSkillDefault(name: string): string | undefined { + return SYSTEM_SKILLS.find((s) => s.name === name)?.content; +} diff --git a/src/mcpd/src/main.ts b/src/mcpd/src/main.ts index 0d7c7a6..a09f54f 100644 --- a/src/mcpd/src/main.ts +++ b/src/mcpd/src/main.ts @@ -54,6 +54,7 @@ import { PersonalityService } from './services/personality.service.js'; import { registerPersonalityRoutes } from './routes/personalities.js'; import { registerWebUi } from './routes/web-ui.js'; import { bootstrapSystemProject } from './bootstrap/system-project.js'; +import { bootstrapSystemSkills } from './bootstrap/system-skills.js'; import { McpServerService, SecretService, @@ -378,6 +379,8 @@ async function main(): Promise { // Bootstrap system project and prompts await bootstrapSystemProject(prisma); + // PR-4: bootstrap system-owned global skills (e.g. propose-learnings). + await bootstrapSystemSkills(prisma); // Repositories const serverRepo = new McpServerRepository(prisma); diff --git a/src/mcpd/tests/system-prompt-validation.test.ts b/src/mcpd/tests/system-prompt-validation.test.ts index ac23082..328a34e 100644 --- a/src/mcpd/tests/system-prompt-validation.test.ts +++ b/src/mcpd/tests/system-prompt-validation.test.ts @@ -101,10 +101,13 @@ describe('System Prompt Validation', () => { }); describe('getSystemPromptNames', () => { - it('includes all 11 system prompts (5 gate + 6 LLM)', () => { + it('includes all 12 system prompts (6 gate + 6 LLM)', () => { const names = getSystemPromptNames(); expect(names).toContain('gate-instructions'); expect(names).toContain('gate-encouragement'); + // PR-4: pairs with the propose-learnings global skill — sits in the + // gating bundle so Claude considers proposing back to mcpd. + expect(names).toContain('gate-encouragement-propose'); expect(names).toContain('gate-intercept-preamble'); expect(names).toContain('gate-session-active'); expect(names).toContain('session-greeting'); @@ -114,7 +117,7 @@ describe('System Prompt Validation', () => { expect(names).toContain('llm-gate-context-selector'); expect(names).toContain('llm-summarize'); expect(names).toContain('llm-paginate-titles'); - expect(names.length).toBe(11); + expect(names.length).toBe(12); }); }); diff --git a/src/mcplocal/src/proxymodel/plugins/gate.ts b/src/mcplocal/src/proxymodel/plugins/gate.ts index 41e1af6..e1987d1 100644 --- a/src/mcplocal/src/proxymodel/plugins/gate.ts +++ b/src/mcplocal/src/proxymodel/plugins/gate.ts @@ -56,6 +56,12 @@ export function createGatePlugin(config: GatePluginConfig = {}): ProxyModelPlugi ctx.registerTool(getProposeTool(), async (args, callCtx) => { return handleProposePrompt(args, callCtx); }); + + // PR-4: Register propose_skill alongside propose_prompt. Goes + // through the new /api/v1/proposals endpoint with resourceType='skill'. + ctx.registerTool(getProposeSkillTool(), async (args, callCtx) => { + return handleProposeSkill(args, callCtx); + }); }, async onSessionDestroy(ctx) { @@ -191,12 +197,40 @@ function getReadPromptsTool(): ToolDefinition { function getProposeTool(): ToolDefinition { return { name: 'propose_prompt', - description: 'Propose a new prompt for this project. Creates a pending request that must be approved by a user before becoming active.', + description: + 'Propose a piece of project-specific knowledge as a new prompt. ' + + 'Use when you discover a non-obvious convention, hidden constraint, ' + + 'or lesson learned that future sessions on this project would benefit ' + + 'from. The proposal enters a queue; a maintainer reviews it and ' + + 'approves or rejects. See the propose-learnings skill for guidance ' + + 'on what makes a good proposal and what NOT to propose.', inputSchema: { type: 'object', properties: { name: { type: 'string', description: 'Prompt name (lowercase alphanumeric with hyphens, e.g. "debug-guide")' }, - content: { type: 'string', description: 'Prompt content text' }, + content: { type: 'string', description: 'Prompt content text. Lead with the shape of the situation, not the resolution. Keep under 200 words.' }, + }, + required: ['name', 'content'], + }, + }; +} + +function getProposeSkillTool(): ToolDefinition { + return { + name: 'propose_skill', + description: + 'Propose a new Claude Code skill (a SKILL.md). Reserve for ' + + 'cross-cutting knowledge — debugging discipline, release hygiene, ' + + 'security review style — that would help across many projects, ' + + 'not just this one. Skills have a larger blast radius than prompts ' + + 'and are harder to scope; lean toward propose_prompt unless you ' + + 'have a clear cross-project reason. See the propose-learnings skill.', + inputSchema: { + type: 'object', + properties: { + name: { type: 'string', description: 'Skill name (lowercase alphanumeric with hyphens, e.g. "debug-discipline")' }, + content: { type: 'string', description: 'SKILL.md body. Markdown. The reviewer will see this as the canonical content of the skill.' }, + description: { type: 'string', description: 'One-line description shown in mcpctl get skills listings' }, }, required: ['name', 'content'], }, @@ -435,6 +469,48 @@ async function handleProposePrompt( } } +async function handleProposeSkill( + args: Record, + ctx: PluginSessionContext, +): Promise { + const name = args['name'] as string | undefined; + const content = args['content'] as string | undefined; + const description = typeof args['description'] === 'string' ? args['description'] : ''; + + if (!name || !content) { + throw new ToolError(-32602, 'Missing required arguments: name and content'); + } + + try { + // PR-4: Skills go through the new /api/v1/proposals endpoint + // (resourceType='skill'). The legacy /api/v1/projects/.../promptrequests + // path is prompt-only. + const body: Record = { + resourceType: 'skill', + name, + project: ctx.projectName, + body: { content, description, priority: 5, files: {}, metadata: {} }, + createdBySession: ctx.sessionId, + }; + await ctx.postToMcpd('/api/v1/proposals', body); + return { + content: [ + { + type: 'text', + text: + `Skill proposal "${name}" created successfully. ` + + `A maintainer will review it (mcpctl review next) and either ` + + `approve — at which point it becomes available to every machine ` + + `that runs mcpctl skills sync — or reject with a note. You will ` + + `not see the outcome in this session.`, + }, + ], + }; + } catch (err) { + throw new ToolError(-32603, `Failed to propose skill: ${err instanceof Error ? err.message : String(err)}`); + } +} + // ── gated intercept handler ── async function handleGatedIntercept(