Skip to content

Commit e4b33f4

Browse files
committed
feat(mcp/plugin): add self-evolving rules pipeline and suggest_rules MCP tool
- Wire PatternDetector → RuleSuggester pipeline in Python (SuggestPipeline) - Add suggest_rules MCP handler that spawns Python pipeline via child_process - Add effectiveness scoring to RuleInsightsService for auto-generated rules - Render effectiveness scores in rule impact report - Register SuggestRulesHandler in index.ts and mcp.module.ts - Add comprehensive tests for all new code (Python + TypeScript) Closes #1437
1 parent bd033cc commit e4b33f4

11 files changed

Lines changed: 704 additions & 0 deletions

apps/mcp-server/src/mcp/handlers/index.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,12 @@ export { ReviewPrHandler } from './review-pr.handler';
174174
*/
175175
export { ActivateHandler } from './activate.handler';
176176

177+
/**
178+
* Handler for suggest_rules tool (self-evolving rules pipeline)
179+
* @see {@link SuggestRulesHandler}
180+
*/
181+
export { SuggestRulesHandler } from './suggest-rules.handler';
182+
177183
/**
178184
* Injection token for the array of all tool handlers.
179185
*

apps/mcp-server/src/mcp/handlers/rule-impact.handler.spec.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ describe('RuleImpactHandler', () => {
3535
emerging: ['security'],
3636
},
3737
suggestions: ['Some suggestion about high-frequency rules'],
38+
effectivenessScores: [],
3839
};
3940

4041
beforeEach(() => {
@@ -140,6 +141,7 @@ describe('RuleImpactHandler', () => {
140141
suggestions: [
141142
'No tracking data available yet — use parse_mode to start collecting rule usage data',
142143
],
144+
effectivenessScores: [],
143145
};
144146
(mockInsightsService.generateInsights as ReturnType<typeof vi.fn>).mockReturnValue(
145147
emptyInsight,

apps/mcp-server/src/mcp/handlers/rule-impact.handler.ts

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,9 @@ export class RuleImpactHandler extends AbstractHandler {
131131
// Domain Coverage
132132
this.appendDomainCoverage(lines, insights);
133133

134+
// Effectiveness Scores
135+
this.appendEffectivenessScores(lines, insights);
136+
134137
// Unused Rules
135138
this.appendUnusedRules(lines, insights);
136139

@@ -216,6 +219,28 @@ export class RuleImpactHandler extends AbstractHandler {
216219
lines.push('');
217220
}
218221

222+
private appendEffectivenessScores(lines: string[], insights: RuleInsight): void {
223+
const scores = insights.effectivenessScores;
224+
if (!scores || scores.length === 0) return;
225+
226+
lines.push('## Auto-Generated Rule Effectiveness');
227+
lines.push('');
228+
lines.push('| Rule | Baseline | Current | Reduction | Verdict |');
229+
lines.push('| --- | ---: | ---: | ---: | --- |');
230+
for (const score of scores) {
231+
const verdictBadge =
232+
score.verdict === 'effective'
233+
? '**EFFECTIVE**'
234+
: score.verdict === 'needs-review'
235+
? 'needs-review'
236+
: '_ineffective_';
237+
lines.push(
238+
`| ${score.ruleName} | ${(score.baselineFailureRate * 100).toFixed(1)}% | ${(score.currentFailureRate * 100).toFixed(1)}% | ${score.reductionPercent}% | ${verdictBadge} |`,
239+
);
240+
}
241+
lines.push('');
242+
}
243+
219244
private appendUnusedRules(lines: string[], insights: RuleInsight): void {
220245
lines.push('## Unused Rules');
221246
lines.push('');

apps/mcp-server/src/mcp/handlers/rule-insights.handler.spec.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ describe('RuleInsightsHandler', () => {
2727
emerging: ['new-rule'],
2828
},
2929
suggestions: ['Some suggestion'],
30+
effectivenessScores: [],
3031
};
3132

3233
beforeEach(() => {
Lines changed: 201 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,201 @@
1+
import { SuggestRulesHandler } from './suggest-rules.handler';
2+
import * as child_process from 'child_process';
3+
4+
vi.mock('child_process');
5+
6+
describe('SuggestRulesHandler', () => {
7+
let handler: SuggestRulesHandler;
8+
9+
const mockSuggestions = [
10+
{
11+
title: 'Repeated Bash failure: rm -rf /bad/path',
12+
description:
13+
'The `Bash` tool failed 5 times across 3 sessions with input `rm -rf /bad/path`.',
14+
rule_content: '# Repeated Bash failure\n\n> Auto-detected rule\n',
15+
pattern: {
16+
tool_name: 'Bash',
17+
input_summary: 'rm -rf /bad/path',
18+
failure_count: 5,
19+
session_count: 3,
20+
first_seen: 1700000000,
21+
last_seen: 1700100000,
22+
},
23+
},
24+
{
25+
title: 'Repeated Read failure: /nonexistent/file.ts',
26+
description:
27+
'The `Read` tool failed 3 times across 3 sessions with input `/nonexistent/file.ts`.',
28+
rule_content: '# Repeated Read failure\n\n> Auto-detected rule\n',
29+
pattern: {
30+
tool_name: 'Read',
31+
input_summary: '/nonexistent/file.ts',
32+
failure_count: 3,
33+
session_count: 3,
34+
first_seen: 1700000000,
35+
last_seen: 1700100000,
36+
},
37+
},
38+
];
39+
40+
beforeEach(() => {
41+
vi.mocked(child_process.execFile).mockImplementation(
42+
(_cmd: string, _args: readonly string[] | undefined | null, _opts: unknown, cb: unknown) => {
43+
const callback = cb as (err: Error | null, stdout: string, stderr: string) => void;
44+
callback(null, JSON.stringify(mockSuggestions), '');
45+
return {} as child_process.ChildProcess;
46+
},
47+
);
48+
49+
handler = new SuggestRulesHandler();
50+
});
51+
52+
afterEach(() => {
53+
vi.restoreAllMocks();
54+
});
55+
56+
it('should return null for unhandled tools', async () => {
57+
const result = await handler.handle('unknown_tool', {});
58+
expect(result).toBeNull();
59+
});
60+
61+
describe('suggest_rules', () => {
62+
it('should return suggestions from pipeline', async () => {
63+
const result = await handler.handle('suggest_rules', {});
64+
65+
expect(result).not.toBeNull();
66+
expect(result?.isError).toBeFalsy();
67+
68+
const parsed = JSON.parse(result!.content[0].text);
69+
expect(parsed.suggestions).toHaveLength(2);
70+
expect(parsed.suggestions[0].title).toContain('Bash');
71+
});
72+
73+
it('should pass minOccurrences parameter to pipeline', async () => {
74+
await handler.handle('suggest_rules', { minOccurrences: 5 });
75+
76+
expect(child_process.execFile).toHaveBeenCalledWith(
77+
expect.any(String),
78+
expect.arrayContaining(['--min-occurrences', '5']),
79+
expect.any(Object),
80+
expect.any(Function),
81+
);
82+
});
83+
84+
it('should pass days parameter to pipeline', async () => {
85+
await handler.handle('suggest_rules', { days: 14 });
86+
87+
expect(child_process.execFile).toHaveBeenCalledWith(
88+
expect.any(String),
89+
expect.arrayContaining(['--days', '14']),
90+
expect.any(Object),
91+
expect.any(Function),
92+
);
93+
});
94+
95+
it('should pass dbPath parameter to pipeline', async () => {
96+
await handler.handle('suggest_rules', { dbPath: '/custom/history.db' });
97+
98+
expect(child_process.execFile).toHaveBeenCalledWith(
99+
expect.any(String),
100+
expect.arrayContaining(['--db-path', '/custom/history.db']),
101+
expect.any(Object),
102+
expect.any(Function),
103+
);
104+
});
105+
106+
it('should handle pipeline returning empty suggestions', async () => {
107+
vi.mocked(child_process.execFile).mockImplementation(
108+
(
109+
_cmd: string,
110+
_args: readonly string[] | undefined | null,
111+
_opts: unknown,
112+
cb: unknown,
113+
) => {
114+
const callback = cb as (err: Error | null, stdout: string, stderr: string) => void;
115+
callback(null, '[]', '');
116+
return {} as child_process.ChildProcess;
117+
},
118+
);
119+
120+
const result = await handler.handle('suggest_rules', {});
121+
122+
expect(result).not.toBeNull();
123+
const parsed = JSON.parse(result!.content[0].text);
124+
expect(parsed.suggestions).toHaveLength(0);
125+
});
126+
127+
it('should return error when pipeline fails', async () => {
128+
vi.mocked(child_process.execFile).mockImplementation(
129+
(
130+
_cmd: string,
131+
_args: readonly string[] | undefined | null,
132+
_opts: unknown,
133+
cb: unknown,
134+
) => {
135+
const callback = cb as (err: Error | null, stdout: string, stderr: string) => void;
136+
callback(new Error('Python not found'), '', 'error');
137+
return {} as child_process.ChildProcess;
138+
},
139+
);
140+
141+
const result = await handler.handle('suggest_rules', {});
142+
143+
expect(result).not.toBeNull();
144+
expect(result?.isError).toBe(true);
145+
expect(result!.content[0].text).toContain('Pipeline execution failed');
146+
});
147+
148+
it('should return error when pipeline outputs invalid JSON', async () => {
149+
vi.mocked(child_process.execFile).mockImplementation(
150+
(
151+
_cmd: string,
152+
_args: readonly string[] | undefined | null,
153+
_opts: unknown,
154+
cb: unknown,
155+
) => {
156+
const callback = cb as (err: Error | null, stdout: string, stderr: string) => void;
157+
callback(null, 'not valid json', '');
158+
return {} as child_process.ChildProcess;
159+
},
160+
);
161+
162+
const result = await handler.handle('suggest_rules', {});
163+
164+
expect(result).not.toBeNull();
165+
expect(result?.isError).toBe(true);
166+
expect(result!.content[0].text).toContain('Failed to parse');
167+
});
168+
169+
it('should include metadata in response', async () => {
170+
const result = await handler.handle('suggest_rules', {});
171+
172+
const parsed = JSON.parse(result!.content[0].text);
173+
expect(parsed).toHaveProperty('generatedAt');
174+
expect(parsed).toHaveProperty('count');
175+
expect(parsed.count).toBe(2);
176+
});
177+
});
178+
179+
describe('getToolDefinitions', () => {
180+
it('should return suggest_rules definition', () => {
181+
const definitions = handler.getToolDefinitions();
182+
183+
expect(definitions).toHaveLength(1);
184+
expect(definitions[0].name).toBe('suggest_rules');
185+
});
186+
187+
it('should have correct input schema properties', () => {
188+
const definitions = handler.getToolDefinitions();
189+
const schema = definitions[0].inputSchema;
190+
191+
expect(schema.properties).toHaveProperty('minOccurrences');
192+
expect(schema.properties).toHaveProperty('days');
193+
expect(schema.properties).toHaveProperty('dbPath');
194+
});
195+
196+
it('should have no required parameters', () => {
197+
const definitions = handler.getToolDefinitions();
198+
expect(definitions[0].inputSchema.required).toEqual([]);
199+
});
200+
});
201+
});

0 commit comments

Comments
 (0)