From 98956861156caa6d3b53b1cb3b3c216168c8c4f8 Mon Sep 17 00:00:00 2001 From: JeremyDev87 Date: Sat, 4 Apr 2026 03:16:04 +0900 Subject: [PATCH 1/2] fix(mcp): preserve skill frontmatter metadata at runtime (#1242) Extend SkillFrontmatterSchema to parse user-invocable, disable-model-invocation, context, agent, and allowed-tools fields. Map kebab-case to camelCase in parseSkill() and propagate through listSkillSummaries and listSkillsFromDir. Filter skills with userInvocable: false from recommendSkills() results so internal skills like agent-discussion and tmux-master no longer appear in recommendations. --- apps/mcp-server/src/rules/rules.service.ts | 11 +- .../mcp-server/src/rules/skill.schema.spec.ts | 118 ++++++++++++++++++ apps/mcp-server/src/rules/skill.schema.ts | 15 +++ apps/mcp-server/src/shared/rules-core.ts | 23 +++- .../skill-recommendation.service.spec.ts | 77 ++++++++++++ .../src/skill/skill-recommendation.service.ts | 14 ++- 6 files changed, 255 insertions(+), 3 deletions(-) diff --git a/apps/mcp-server/src/rules/rules.service.ts b/apps/mcp-server/src/rules/rules.service.ts index 7eb17951..dd5501bb 100644 --- a/apps/mcp-server/src/rules/rules.service.ts +++ b/apps/mcp-server/src/rules/rules.service.ts @@ -194,7 +194,16 @@ export class RulesService { * @returns Array of skill summaries with name, description, and optional triggers */ async listSkillsFromDir(): Promise< - Array<{ name: string; description: string; triggers?: SkillFrontmatterTrigger[] }> + Array<{ + name: string; + description: string; + triggers?: SkillFrontmatterTrigger[]; + userInvocable?: boolean; + disableModelInvocation?: boolean; + context?: string; + agent?: string; + allowedTools?: string[]; + }> > { try { return await listSkillSummaries(this.rulesDir); diff --git a/apps/mcp-server/src/rules/skill.schema.spec.ts b/apps/mcp-server/src/rules/skill.schema.spec.ts index aac60c63..a4702d7a 100644 --- a/apps/mcp-server/src/rules/skill.schema.spec.ts +++ b/apps/mcp-server/src/rules/skill.schema.spec.ts @@ -52,6 +52,124 @@ Content here. }); }); + it('should parse user-invocable field', () => { + const content = `--- +name: internal-skill +description: An internal skill +user-invocable: false +--- + +Content. +`; + const result = parseSkill(content, 'path'); + + expect(result.userInvocable).toBe(false); + }); + + it('should parse disable-model-invocation field', () => { + const content = `--- +name: manual-only +description: Manual only skill +disable-model-invocation: true +--- + +Content. +`; + const result = parseSkill(content, 'path'); + + expect(result.disableModelInvocation).toBe(true); + }); + + it('should parse context field', () => { + const content = `--- +name: ctx-skill +description: Skill with context +context: conversation +--- + +Content. +`; + const result = parseSkill(content, 'path'); + + expect(result.context).toBe('conversation'); + }); + + it('should parse agent field', () => { + const content = `--- +name: agent-skill +description: Skill with agent +agent: security-specialist +--- + +Content. +`; + const result = parseSkill(content, 'path'); + + expect(result.agent).toBe('security-specialist'); + }); + + it('should parse allowed-tools field', () => { + const content = `--- +name: tools-skill +description: Skill with allowed tools +allowed-tools: + - Read + - Grep + - Glob +--- + +Content. +`; + const result = parseSkill(content, 'path'); + + expect(result.allowedTools).toEqual(['Read', 'Grep', 'Glob']); + }); + + it('should leave extended fields undefined when not present', () => { + const content = `--- +name: basic-skill +description: Basic skill without extended fields +--- + +Content. +`; + const result = parseSkill(content, 'path'); + + expect(result.userInvocable).toBeUndefined(); + expect(result.disableModelInvocation).toBeUndefined(); + expect(result.context).toBeUndefined(); + expect(result.agent).toBeUndefined(); + expect(result.allowedTools).toBeUndefined(); + }); + + it('should parse skill with all extended fields together', () => { + const content = `--- +name: full-skill +description: Skill with all fields +user-invocable: true +disable-model-invocation: false +context: project +agent: frontend-developer +allowed-tools: + - Edit + - Write +triggers: + - pattern: "test" + confidence: high +--- + +Content. +`; + const result = parseSkill(content, 'path'); + + expect(result.userInvocable).toBe(true); + expect(result.disableModelInvocation).toBe(false); + expect(result.context).toBe('project'); + expect(result.agent).toBe('frontend-developer'); + expect(result.allowedTools).toEqual(['Edit', 'Write']); + expect(result.triggers).toHaveLength(1); + }); + it('should parse skill with empty triggers array', () => { const content = `--- name: no-triggers diff --git a/apps/mcp-server/src/rules/skill.schema.ts b/apps/mcp-server/src/rules/skill.schema.ts index f361e180..f11674c1 100644 --- a/apps/mcp-server/src/rules/skill.schema.ts +++ b/apps/mcp-server/src/rules/skill.schema.ts @@ -53,6 +53,11 @@ const SkillFrontmatterSchema = z.object({ .regex(/^[a-z0-9-]+$/, 'Skill name must be lowercase alphanumeric with hyphens only'), description: z.string().min(1).max(500), triggers: z.array(SkillFrontmatterTriggerSchema).optional(), + 'user-invocable': z.boolean().optional(), + 'disable-model-invocation': z.boolean().optional(), + context: z.string().optional(), + agent: z.string().optional(), + 'allowed-tools': z.array(z.string()).optional(), }); // ============================================================================ @@ -70,6 +75,11 @@ export interface Skill { content: string; path: string; triggers?: SkillFrontmatterTrigger[]; + userInvocable?: boolean; + disableModelInvocation?: boolean; + context?: string; + agent?: string; + allowedTools?: string[]; } // ============================================================================ @@ -146,5 +156,10 @@ export function parseSkill(content: string, filePath: string): Skill { content: body, path: filePath, triggers: result.data.triggers, + userInvocable: result.data['user-invocable'], + disableModelInvocation: result.data['disable-model-invocation'], + context: result.data.context, + agent: result.data.agent, + allowedTools: result.data['allowed-tools'], }; } diff --git a/apps/mcp-server/src/shared/rules-core.ts b/apps/mcp-server/src/shared/rules-core.ts index 01691e3e..569f3113 100644 --- a/apps/mcp-server/src/shared/rules-core.ts +++ b/apps/mcp-server/src/shared/rules-core.ts @@ -244,7 +244,18 @@ export async function searchInRuleFiles( export async function listSkillSummaries( rulesDir: string, deps?: FileSystemDeps, -): Promise> { +): Promise< + Array<{ + name: string; + description: string; + triggers?: SkillFrontmatterTrigger[]; + userInvocable?: boolean; + disableModelInvocation?: boolean; + context?: string; + agent?: string; + allowedTools?: string[]; + }> +> { const skillsDir = path.join(rulesDir, 'skills'); const readdir = getReaddir(deps); const readFile = getReadFile(deps); @@ -252,6 +263,11 @@ export async function listSkillSummaries( name: string; description: string; triggers?: SkillFrontmatterTrigger[]; + userInvocable?: boolean; + disableModelInvocation?: boolean; + context?: string; + agent?: string; + allowedTools?: string[]; }> = []; try { @@ -269,6 +285,11 @@ export async function listSkillSummaries( name: skill.name, description: skill.description, triggers: skill.triggers, + userInvocable: skill.userInvocable, + disableModelInvocation: skill.disableModelInvocation, + context: skill.context, + agent: skill.agent, + allowedTools: skill.allowedTools, }); } catch { // Skip invalid/missing skills diff --git a/apps/mcp-server/src/skill/skill-recommendation.service.spec.ts b/apps/mcp-server/src/skill/skill-recommendation.service.spec.ts index 99976a39..2f4d0c90 100644 --- a/apps/mcp-server/src/skill/skill-recommendation.service.spec.ts +++ b/apps/mcp-server/src/skill/skill-recommendation.service.spec.ts @@ -12,6 +12,7 @@ function createMockRulesService( name: string; description: string; triggers?: SkillFrontmatterTrigger[]; + userInvocable?: boolean; }> = [], ): RulesService { return { @@ -522,6 +523,82 @@ describe('SkillRecommendationService', () => { }); }); + describe('user-invocable filtering', () => { + it('should exclude skills with userInvocable: false from recommendations', async () => { + const skillsWithNonInvocable = [ + ...FILESYSTEM_SKILLS, + { + name: 'agent-discussion', + description: 'Internal agent discussion skill', + userInvocable: false, + triggers: [{ pattern: 'discuss.*agent', confidence: 'high' as const }], + }, + ]; + const filteredService = new SkillRecommendationService( + createMockRulesService(skillsWithNonInvocable), + ); + await filteredService.loadFrontmatterTriggers(); + + const result = filteredService.recommendSkills('discuss the agent architecture'); + + const agentDiscussion = result.recommendations.find( + r => r.skillName === 'agent-discussion', + ); + expect(agentDiscussion).toBeUndefined(); + }); + + it('should include skills with userInvocable: true in recommendations', async () => { + const skillsWithInvocable = [ + ...FILESYSTEM_SKILLS, + { + name: 'invocable-skill', + description: 'User invocable skill', + userInvocable: true, + triggers: [{ pattern: 'invocable-test-xyz', confidence: 'high' as const }], + }, + ]; + const invocableService = new SkillRecommendationService( + createMockRulesService(skillsWithInvocable), + ); + await invocableService.loadFrontmatterTriggers(); + + const result = invocableService.recommendSkills('invocable-test-xyz'); + + const skill = result.recommendations.find(r => r.skillName === 'invocable-skill'); + expect(skill).toBeDefined(); + }); + + it('should include skills without userInvocable field (default behavior)', async () => { + // Skills without userInvocable should be included (backwards compatible) + const result = service.recommendSkills('widget slot architecture'); + + const wsa = result.recommendations.find(r => r.skillName === 'widget-slot-architecture'); + expect(wsa).toBeDefined(); + }); + + it('should filter non-invocable skills even when matched by keyword triggers', async () => { + const skillsWithNonInvocable = [ + { + name: 'systematic-debugging', + description: 'Debugging skill', + userInvocable: false, + }, + ]; + const filteredService = new SkillRecommendationService( + createMockRulesService(skillsWithNonInvocable), + ); + await filteredService.loadFrontmatterTriggers(); + + // "fix this bug" normally matches systematic-debugging via keyword triggers + const result = filteredService.recommendSkills('I need to fix this bug'); + + const debugging = result.recommendations.find( + r => r.skillName === 'systematic-debugging', + ); + expect(debugging).toBeUndefined(); + }); + }); + describe('listSkills', () => { it('should return all filesystem skills sorted by priority descending', async () => { const result = await service.listSkills(); diff --git a/apps/mcp-server/src/skill/skill-recommendation.service.ts b/apps/mcp-server/src/skill/skill-recommendation.service.ts index d2934b84..ab4c85db 100644 --- a/apps/mcp-server/src/skill/skill-recommendation.service.ts +++ b/apps/mcp-server/src/skill/skill-recommendation.service.ts @@ -26,6 +26,7 @@ interface CachedFrontmatterTrigger { interface FrontmatterTriggerEntry { description: string; triggers: CachedFrontmatterTrigger[]; + userInvocable?: boolean; } /** @@ -42,6 +43,7 @@ function getSkillDescription(skillName: string): string { @Injectable() export class SkillRecommendationService implements OnModuleInit { private frontmatterTriggerCache = new Map(); + private skillMetadataCache = new Map(); constructor(private readonly rulesService: RulesService) {} @@ -56,8 +58,12 @@ export class SkillRecommendationService implements OnModuleInit { async loadFrontmatterTriggers(): Promise { const skills = await this.rulesService.listSkillsFromDir(); this.frontmatterTriggerCache.clear(); + this.skillMetadataCache.clear(); for (const skill of skills) { + if (skill.userInvocable !== undefined) { + this.skillMetadataCache.set(skill.name, { userInvocable: skill.userInvocable }); + } if (skill.triggers && skill.triggers.length > 0) { const compiled: CachedFrontmatterTrigger[] = []; for (const t of skill.triggers) { @@ -160,10 +166,16 @@ export class SkillRecommendationService implements OnModuleInit { } } - // Step 3: Convert to SkillRecommendation array and sort + // Step 3: Convert to SkillRecommendation array, filter non-invocable, and sort const recommendations: SkillRecommendation[] = []; for (const [skillName, match] of skillMatches) { + // Filter out skills explicitly marked as user-invocable: false + const metadata = this.skillMetadataCache.get(skillName); + if (metadata?.userInvocable === false) { + continue; + } + recommendations.push({ skillName, confidence: match.confidence, From d72b10df6b490646f3a8f6741dd02f0bde73a4e2 Mon Sep 17 00:00:00 2001 From: JeremyDev87 Date: Sat, 4 Apr 2026 03:20:49 +0900 Subject: [PATCH 2/2] style: fix prettier formatting in skill-recommendation spec --- .../src/skill/skill-recommendation.service.spec.ts | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/apps/mcp-server/src/skill/skill-recommendation.service.spec.ts b/apps/mcp-server/src/skill/skill-recommendation.service.spec.ts index 2f4d0c90..5eeb8987 100644 --- a/apps/mcp-server/src/skill/skill-recommendation.service.spec.ts +++ b/apps/mcp-server/src/skill/skill-recommendation.service.spec.ts @@ -541,9 +541,7 @@ describe('SkillRecommendationService', () => { const result = filteredService.recommendSkills('discuss the agent architecture'); - const agentDiscussion = result.recommendations.find( - r => r.skillName === 'agent-discussion', - ); + const agentDiscussion = result.recommendations.find(r => r.skillName === 'agent-discussion'); expect(agentDiscussion).toBeUndefined(); }); @@ -592,9 +590,7 @@ describe('SkillRecommendationService', () => { // "fix this bug" normally matches systematic-debugging via keyword triggers const result = filteredService.recommendSkills('I need to fix this bug'); - const debugging = result.recommendations.find( - r => r.skillName === 'systematic-debugging', - ); + const debugging = result.recommendations.find(r => r.skillName === 'systematic-debugging'); expect(debugging).toBeUndefined(); }); });