feat(mcpd+mcplocal+cli): propose-learnings system skill, propose_skill MCP tool, mcpctl review

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 <id>     Full detail
    mcpctl review approve <id>  POST /proposals/:id/approve
    mcpctl review reject <id> --reason "..."
    mcpctl review diff <id>     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) <noreply@anthropic.com>
This commit is contained in:
Michal
2026-05-07 13:13:33 +01:00
parent 20a541a5d6
commit db57bb5856
9 changed files with 508 additions and 6 deletions

View File

@@ -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 <id> Full detail of one proposal
* mcpctl review approve <id> Approve (creates resource + revision)
* mcpctl review reject <id> --reason Reject with reviewer note
* mcpctl review diff <id> Diff proposal body vs current resource
*/
interface Proposal {
id: string;
resourceType: 'prompt' | 'skill';
name: string;
body: Record<string, unknown>;
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 <kind>', '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<Proposal[]>(`/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 <kind>', '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<Proposal[]>(`/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('<id>', 'Proposal ID')
.action(async (id: string) => {
const proposal = await client.get<Proposal>(`/api/v1/proposals/${id}`);
log(formatDetail(proposal));
});
cmd.command('approve')
.description('Approve a pending proposal (creates the resource + initial revision)')
.argument('<id>', 'Proposal ID')
.action(async (id: string) => {
const updated = await client.post<Proposal>(`/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('<id>', 'Proposal ID')
.option('--reason <text>', '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('<id>', 'Proposal ID')
.action(async (id: string) => {
const proposal = await client.get<Proposal>(`/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<Array<{ name: string; content: string }>>(`/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<Array<{ name: string; content: string }>>(`/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`;
}

View File

@@ -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 });

View File

@@ -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',

View File

@@ -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<void> {
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;
}

View File

@@ -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<void> {
// 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);

View File

@@ -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);
});
});

View File

@@ -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<string, unknown>,
ctx: PluginSessionContext,
): Promise<unknown> {
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<string, unknown> = {
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(