Skip to content

Commit 33345a4

Browse files
wishket-pjwJeremyDev87
wishket-pjw
authored and
JeremyDev87
committed
feat: improve skill recommendation feature
- Improve skill auto-recommendation feature and fix bugs - Enhance keyword pattern matching logic - Strengthen multi-language support - Improve test coverage - Enhance MCP service integration close #120
1 parent 61ce982 commit 33345a4

8 files changed

Lines changed: 432 additions & 13 deletions

‎apps/mcp-server/src/mcp/mcp.service.spec.ts‎

Lines changed: 230 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,10 @@ import {
1010
} from '../config/config-diff.service';
1111
import { AnalyzerService } from '../analyzer/analyzer.service';
1212
import { SkillRecommendationService } from '../skill/skill-recommendation.service';
13-
import type { RecommendSkillsResult } from '../skill/skill-recommendation.types';
13+
import type {
14+
RecommendSkillsResult,
15+
ListSkillsResult,
16+
} from '../skill/skill-recommendation.types';
1417

1518
// Handler function type for MCP request handlers
1619
type McpHandler = (request: unknown) => Promise<unknown>;
@@ -154,6 +157,29 @@ const createMockSkillRecommendationService =
154157
],
155158
originalPrompt: 'I have a bug in my code',
156159
} as RecommendSkillsResult),
160+
listSkills: vi.fn().mockReturnValue({
161+
skills: [
162+
{
163+
name: 'systematic-debugging',
164+
priority: 100,
165+
description: 'Systematic approach to debugging',
166+
concepts: ['bug', 'error', 'debug'],
167+
},
168+
{
169+
name: 'test-driven-development',
170+
priority: 90,
171+
description: 'Test-driven development workflow',
172+
concepts: ['test', 'tdd'],
173+
},
174+
{
175+
name: 'brainstorming',
176+
priority: 80,
177+
description: 'Explore requirements before implementation',
178+
concepts: ['design', 'feature'],
179+
},
180+
],
181+
total: 3,
182+
} as ListSkillsResult),
157183
});
158184

159185
// Import after mocks
@@ -932,4 +958,207 @@ describe('McpService', () => {
932958
});
933959
});
934960
});
961+
962+
// ============================================================================
963+
// list_skills Tool Tests
964+
// ============================================================================
965+
966+
describe('list_skills tool', () => {
967+
describe('Tool Registration', () => {
968+
it('should list list_skills tool', async () => {
969+
const handler = handlers.get('tools/list');
970+
expect(handler).toBeDefined();
971+
972+
const result = (await handler!({})) as {
973+
tools: { name: string; description: string; inputSchema: object }[];
974+
};
975+
const listTool = result.tools.find(t => t.name === 'list_skills');
976+
977+
expect(listTool).toBeDefined();
978+
expect(listTool!.description).toContain('skills');
979+
});
980+
981+
it('should have correct inputSchema for list_skills', async () => {
982+
const handler = handlers.get('tools/list');
983+
expect(handler).toBeDefined();
984+
985+
const result = (await handler!({})) as {
986+
tools: {
987+
name: string;
988+
inputSchema: { properties: object; required: string[] };
989+
}[];
990+
};
991+
const listTool = result.tools.find(t => t.name === 'list_skills');
992+
993+
expect(listTool).toBeDefined();
994+
expect(listTool!.inputSchema.properties).toHaveProperty('minPriority');
995+
expect(listTool!.inputSchema.properties).toHaveProperty('maxPriority');
996+
expect(listTool!.inputSchema.required).toEqual([]);
997+
});
998+
});
999+
1000+
describe('Basic Functionality', () => {
1001+
it('should return all skills when called without options', async () => {
1002+
const handler = handlers.get('tools/call');
1003+
expect(handler).toBeDefined();
1004+
1005+
const result = (await handler!({
1006+
params: {
1007+
name: 'list_skills',
1008+
arguments: {},
1009+
},
1010+
})) as { content: { type: string; text: string }[] };
1011+
1012+
expect(result.content).toHaveLength(1);
1013+
expect(result.content[0].type).toBe('text');
1014+
1015+
const parsed = JSON.parse(result.content[0].text);
1016+
expect(parsed.skills).toBeDefined();
1017+
expect(parsed.skills.length).toBe(3);
1018+
expect(parsed.total).toBe(3);
1019+
expect(mockSkillRecommendationService.listSkills).toHaveBeenCalledWith(
1020+
{},
1021+
);
1022+
});
1023+
1024+
it('should filter by minPriority', async () => {
1025+
vi.mocked(mockSkillRecommendationService.listSkills!).mockReturnValue({
1026+
skills: [
1027+
{
1028+
name: 'systematic-debugging',
1029+
priority: 100,
1030+
description: 'Systematic approach to debugging',
1031+
concepts: ['bug', 'error', 'debug'],
1032+
},
1033+
{
1034+
name: 'test-driven-development',
1035+
priority: 90,
1036+
description: 'Test-driven development workflow',
1037+
concepts: ['test', 'tdd'],
1038+
},
1039+
],
1040+
total: 2,
1041+
} as ListSkillsResult);
1042+
1043+
const handler = handlers.get('tools/call');
1044+
expect(handler).toBeDefined();
1045+
1046+
const result = (await handler!({
1047+
params: {
1048+
name: 'list_skills',
1049+
arguments: { minPriority: 90 },
1050+
},
1051+
})) as { content: { type: string; text: string }[] };
1052+
1053+
const parsed = JSON.parse(result.content[0].text);
1054+
expect(parsed.skills.length).toBe(2);
1055+
expect(parsed.total).toBe(2);
1056+
expect(mockSkillRecommendationService.listSkills).toHaveBeenCalledWith({
1057+
minPriority: 90,
1058+
});
1059+
});
1060+
1061+
it('should filter by maxPriority', async () => {
1062+
vi.mocked(mockSkillRecommendationService.listSkills!).mockReturnValue({
1063+
skills: [
1064+
{
1065+
name: 'brainstorming',
1066+
priority: 80,
1067+
description: 'Explore requirements before implementation',
1068+
concepts: ['design', 'feature'],
1069+
},
1070+
],
1071+
total: 1,
1072+
} as ListSkillsResult);
1073+
1074+
const handler = handlers.get('tools/call');
1075+
expect(handler).toBeDefined();
1076+
1077+
const result = (await handler!({
1078+
params: {
1079+
name: 'list_skills',
1080+
arguments: { maxPriority: 80 },
1081+
},
1082+
})) as { content: { type: string; text: string }[] };
1083+
1084+
const parsed = JSON.parse(result.content[0].text);
1085+
expect(parsed.skills.length).toBe(1);
1086+
expect(parsed.total).toBe(1);
1087+
expect(mockSkillRecommendationService.listSkills).toHaveBeenCalledWith({
1088+
maxPriority: 80,
1089+
});
1090+
});
1091+
1092+
it('should filter by both minPriority and maxPriority', async () => {
1093+
vi.mocked(mockSkillRecommendationService.listSkills!).mockReturnValue({
1094+
skills: [
1095+
{
1096+
name: 'test-driven-development',
1097+
priority: 90,
1098+
description: 'Test-driven development workflow',
1099+
concepts: ['test', 'tdd'],
1100+
},
1101+
],
1102+
total: 1,
1103+
} as ListSkillsResult);
1104+
1105+
const handler = handlers.get('tools/call');
1106+
expect(handler).toBeDefined();
1107+
1108+
const result = (await handler!({
1109+
params: {
1110+
name: 'list_skills',
1111+
arguments: { minPriority: 85, maxPriority: 95 },
1112+
},
1113+
})) as { content: { type: string; text: string }[] };
1114+
1115+
const parsed = JSON.parse(result.content[0].text);
1116+
expect(parsed.skills.length).toBe(1);
1117+
expect(mockSkillRecommendationService.listSkills).toHaveBeenCalledWith({
1118+
minPriority: 85,
1119+
maxPriority: 95,
1120+
});
1121+
});
1122+
});
1123+
1124+
describe('Error Handling', () => {
1125+
it('should return error when service throws', async () => {
1126+
vi.mocked(
1127+
mockSkillRecommendationService.listSkills!,
1128+
).mockImplementation(() => {
1129+
throw new Error('Service error');
1130+
});
1131+
1132+
const handler = handlers.get('tools/call');
1133+
expect(handler).toBeDefined();
1134+
1135+
const result = (await handler!({
1136+
params: {
1137+
name: 'list_skills',
1138+
arguments: {},
1139+
},
1140+
})) as { isError: boolean; content: { text: string }[] };
1141+
1142+
expect(result.isError).toBe(true);
1143+
expect(result.content[0].text).toContain('Failed to list skills');
1144+
});
1145+
1146+
it('should ignore non-number priority values', async () => {
1147+
const handler = handlers.get('tools/call');
1148+
expect(handler).toBeDefined();
1149+
1150+
await handler!({
1151+
params: {
1152+
name: 'list_skills',
1153+
arguments: { minPriority: 'invalid', maxPriority: null },
1154+
},
1155+
});
1156+
1157+
// Should be called with empty options since non-number values are ignored
1158+
expect(mockSkillRecommendationService.listSkills).toHaveBeenCalledWith(
1159+
{},
1160+
);
1161+
});
1162+
});
1163+
});
9351164
});

‎apps/mcp-server/src/mcp/mcp.service.ts‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import { ConfigService } from '../config/config.service';
1818
import { ConfigDiffService } from '../config/config-diff.service';
1919
import { AnalyzerService } from '../analyzer/analyzer.service';
2020
import { SkillRecommendationService } from '../skill/skill-recommendation.service';
21+
import type { ListSkillsOptions } from '../skill/skill-recommendation.types';
2122
import type { CodingBuddyConfig } from '../config/config.schema';
2223

2324
@Injectable()
@@ -235,6 +236,24 @@ export class McpService implements OnModuleInit {
235236
required: ['prompt'],
236237
},
237238
},
239+
{
240+
name: 'list_skills',
241+
description: 'List all available skills with optional filtering',
242+
inputSchema: {
243+
type: 'object',
244+
properties: {
245+
minPriority: {
246+
type: 'number',
247+
description: 'Minimum priority threshold (inclusive)',
248+
},
249+
maxPriority: {
250+
type: 'number',
251+
description: 'Maximum priority threshold (inclusive)',
252+
},
253+
},
254+
required: [],
255+
},
256+
},
238257
],
239258
};
240259
});
@@ -255,6 +274,8 @@ export class McpService implements OnModuleInit {
255274
return this.handleSuggestConfigUpdates(args);
256275
case 'recommend_skills':
257276
return this.handleRecommendSkills(args);
277+
case 'list_skills':
278+
return this.handleListSkills(args);
258279
default:
259280
throw new McpError(
260281
ErrorCode.MethodNotFound,
@@ -410,6 +431,26 @@ export class McpService implements OnModuleInit {
410431
}
411432
}
412433

434+
private handleListSkills(args: Record<string, unknown> | undefined) {
435+
try {
436+
const options: ListSkillsOptions = {};
437+
438+
if (typeof args?.minPriority === 'number') {
439+
options.minPriority = args.minPriority;
440+
}
441+
if (typeof args?.maxPriority === 'number') {
442+
options.maxPriority = args.maxPriority;
443+
}
444+
445+
const result = this.skillRecommendationService.listSkills(options);
446+
return this.jsonResponse(result);
447+
} catch (error) {
448+
return this.errorResponse(
449+
`Failed to list skills: ${error instanceof Error ? error.message : 'Unknown error'}`,
450+
);
451+
}
452+
}
453+
413454
// ============================================================================
414455
// Response Helpers
415456
// ============================================================================

‎apps/mcp-server/src/skill/i18n/keywords.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ export const SKILL_KEYWORDS: SkillKeywordConfig[] = [
1313
{
1414
skillName: 'systematic-debugging',
1515
priority: 25,
16+
description: 'Systematic approach to debugging',
1617
concepts: {
1718
error: {
1819
en: [
@@ -66,6 +67,7 @@ export const SKILL_KEYWORDS: SkillKeywordConfig[] = [
6667
{
6768
skillName: 'executing-plans',
6869
priority: 22,
70+
description: 'Execute implementation plans with checkpoints',
6971
concepts: {
7072
execute: {
7173
en: ['execute plan', 'follow plan', 'run plan', 'implement plan'],
@@ -97,6 +99,7 @@ export const SKILL_KEYWORDS: SkillKeywordConfig[] = [
9799
{
98100
skillName: 'writing-plans',
99101
priority: 20,
102+
description: 'Create implementation plans',
100103
concepts: {
101104
plan: {
102105
en: ['plan', 'roadmap', 'schedule', 'milestone'],
@@ -135,6 +138,7 @@ export const SKILL_KEYWORDS: SkillKeywordConfig[] = [
135138
{
136139
skillName: 'frontend-design',
137140
priority: 18,
141+
description: 'Build production-grade UI components',
138142
concepts: {
139143
ui_element: {
140144
en: [
@@ -230,6 +234,7 @@ export const SKILL_KEYWORDS: SkillKeywordConfig[] = [
230234
{
231235
skillName: 'test-driven-development',
232236
priority: 15,
237+
description: 'Test-driven development workflow',
233238
concepts: {
234239
tdd: {
235240
en: ['TDD', 'test first', 'red green', 'test driven'],
@@ -268,6 +273,7 @@ export const SKILL_KEYWORDS: SkillKeywordConfig[] = [
268273
{
269274
skillName: 'dispatching-parallel-agents',
270275
priority: 12,
276+
description: 'Handle parallel independent tasks',
271277
concepts: {
272278
parallel: {
273279
en: ['parallel', 'concurrent', 'simultaneously', 'at the same time'],
@@ -292,6 +298,7 @@ export const SKILL_KEYWORDS: SkillKeywordConfig[] = [
292298
{
293299
skillName: 'subagent-driven-development',
294300
priority: 12,
301+
description: 'Execute plans in current session',
295302
concepts: {
296303
subagent: {
297304
en: ['subagent', 'sub-agent'],
@@ -316,6 +323,7 @@ export const SKILL_KEYWORDS: SkillKeywordConfig[] = [
316323
{
317324
skillName: 'brainstorming',
318325
priority: 10,
326+
description: 'Explore requirements before implementation',
319327
concepts: {
320328
create: {
321329
en: ['create', 'build', 'make', 'develop', 'implement'],

‎apps/mcp-server/src/skill/i18n/keywords.types.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ export type ConceptKeywords = {
1616
export interface SkillKeywordConfig {
1717
skillName: string;
1818
priority: number;
19+
description: string;
1920
concepts: {
2021
[conceptName: string]: ConceptKeywords;
2122
};

0 commit comments

Comments
 (0)