Skip to content

Commit 9660efe

Browse files
JeremyDev87JeremyDev87
committed
feat(mcp): activate rule event tracking in core handlers (#1180)
Instrument 5 core MCP handlers with RuleEventCollector for rule-stats.json: - mode.handler: mode_activated events with mode and agent details - checklist-context.handler: checklist_generated events per domain - agent.handler: specialist_dispatched events per agent dispatched - quality-report.handler: specialist_dispatched events per domain detected - discussion.handler: specialist_dispatched events per specialist All events use fire-and-forget pattern (try/catch, never break handler).
1 parent 540b3ed commit 9660efe

12 files changed

Lines changed: 362 additions & 7 deletions

‎apps/mcp-server/src/mcp/handlers/abstract-handler.integration.spec.ts‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import { StateService } from '../../state/state.service';
2020
import { ContextDocumentService } from '../../context/context-document.service';
2121
import { DiagnosticLogService } from '../../diagnostic/diagnostic-log.service';
2222
import type { ImpactEventService } from '../../impact';
23+
import type { RuleEventCollector } from '../../rules/rule-event-collector';
2324

2425
/**
2526
* Integration tests verifying all concrete handlers inherit
@@ -140,13 +141,19 @@ describe('Handler Security Integration', () => {
140141
} as unknown as DiagnosticLogService;
141142

142143
const mockImpactEventService = { logEvent: vi.fn() } as unknown as ImpactEventService;
144+
const mockRuleEventCollector = { record: vi.fn() } as unknown as RuleEventCollector;
143145

144146
// Initialize handlers
145-
agentHandler = new AgentHandler(mockAgentService, mockImpactEventService);
147+
agentHandler = new AgentHandler(
148+
mockAgentService,
149+
mockImpactEventService,
150+
mockRuleEventCollector,
151+
);
146152
checklistHandler = new ChecklistContextHandler(
147153
mockChecklistService,
148154
mockContextService,
149155
mockImpactEventService,
156+
mockRuleEventCollector,
150157
);
151158
configHandler = new ConfigHandler(
152159
mockConfigService,
@@ -168,6 +175,7 @@ describe('Handler Security Integration', () => {
168175
mockDiagnosticLogService,
169176
mockAgentServiceForMode as AgentService,
170177
mockImpactEventService,
178+
mockRuleEventCollector,
171179
);
172180
rulesHandler = new RulesHandler(
173181
mockRulesService,

‎apps/mcp-server/src/mcp/handlers/agent.handler.spec.ts‎

Lines changed: 78 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,13 @@ import { describe, it, expect, beforeEach, vi } from 'vitest';
22
import { AgentHandler } from './agent.handler';
33
import { AgentService } from '../../agent/agent.service';
44
import type { ImpactEventService } from '../../impact';
5+
import type { RuleEventCollector } from '../../rules/rule-event-collector';
56

67
describe('AgentHandler', () => {
78
let handler: AgentHandler;
89
let mockAgentService: AgentService;
910
let mockImpactEventService: Partial<ImpactEventService>;
11+
let mockRuleEventCollector: Partial<RuleEventCollector>;
1012

1113
const mockSystemPromptResult = {
1214
agentName: 'security-specialist',
@@ -25,8 +27,13 @@ describe('AgentHandler', () => {
2527
} as unknown as AgentService;
2628

2729
mockImpactEventService = { logEvent: vi.fn() };
30+
mockRuleEventCollector = { record: vi.fn() };
2831

29-
handler = new AgentHandler(mockAgentService, mockImpactEventService as ImpactEventService);
32+
handler = new AgentHandler(
33+
mockAgentService,
34+
mockImpactEventService as ImpactEventService,
35+
mockRuleEventCollector as RuleEventCollector,
36+
);
3037
});
3138

3239
describe('handle', () => {
@@ -446,6 +453,76 @@ describe('AgentHandler', () => {
446453
});
447454
});
448455

456+
describe('rule event tracking', () => {
457+
it('should record specialist_dispatched event on dispatch', async () => {
458+
await handler.handle('dispatch_agents', {
459+
mode: 'EVAL',
460+
primaryAgent: 'security-specialist',
461+
taskDescription: 'Review security',
462+
});
463+
464+
expect(mockRuleEventCollector.record).toHaveBeenCalledWith(
465+
expect.objectContaining({
466+
type: 'specialist_dispatched',
467+
domain: 'security-specialist',
468+
}),
469+
);
470+
});
471+
472+
it('should record events for each specialist in parallel dispatch', async () => {
473+
mockAgentService.dispatchAgents = vi.fn().mockResolvedValue({
474+
...mockDispatchResult,
475+
parallelAgents: [
476+
{ name: 'accessibility-specialist' },
477+
{ name: 'performance-specialist' },
478+
],
479+
});
480+
481+
await handler.handle('dispatch_agents', {
482+
mode: 'EVAL',
483+
specialists: ['accessibility-specialist', 'performance-specialist'],
484+
includeParallel: true,
485+
});
486+
487+
expect(mockRuleEventCollector.record).toHaveBeenCalledWith(
488+
expect.objectContaining({
489+
type: 'specialist_dispatched',
490+
domain: 'accessibility-specialist',
491+
}),
492+
);
493+
expect(mockRuleEventCollector.record).toHaveBeenCalledWith(
494+
expect.objectContaining({
495+
type: 'specialist_dispatched',
496+
domain: 'performance-specialist',
497+
}),
498+
);
499+
});
500+
501+
it('should not record events when dispatch fails', async () => {
502+
mockAgentService.dispatchAgents = vi.fn().mockRejectedValue(new Error('Dispatch failed'));
503+
504+
await handler.handle('dispatch_agents', {
505+
mode: 'EVAL',
506+
primaryAgent: 'security-specialist',
507+
});
508+
509+
expect(mockRuleEventCollector.record).not.toHaveBeenCalled();
510+
});
511+
512+
it('should not break handler when event recording throws', async () => {
513+
mockRuleEventCollector.record = vi.fn().mockImplementation(() => {
514+
throw new Error('record error');
515+
});
516+
517+
const result = await handler.handle('dispatch_agents', {
518+
mode: 'EVAL',
519+
primaryAgent: 'security-specialist',
520+
});
521+
522+
expect(result?.isError).toBeFalsy();
523+
});
524+
});
525+
449526
describe('executionStrategy parameter', () => {
450527
it('should pass executionStrategy "subagent" to service', async () => {
451528
await handler.handle('dispatch_agents', {

‎apps/mcp-server/src/mcp/handlers/agent.handler.ts‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import {
1515
isRecordObject,
1616
} from '../../shared/validation.constants';
1717
import { ImpactEventService } from '../../impact';
18+
import { RuleEventCollector } from '../../rules/rule-event-collector';
1819

1920
/**
2021
* Handler for agent-related tools
@@ -27,6 +28,7 @@ export class AgentHandler extends AbstractHandler {
2728
constructor(
2829
private readonly agentService: AgentService,
2930
private readonly impactEventService: ImpactEventService,
31+
private readonly ruleEventCollector: RuleEventCollector,
3032
) {
3133
super();
3234
}
@@ -256,6 +258,28 @@ export class AgentHandler extends AbstractHandler {
256258
// Never break handler execution
257259
}
258260

261+
try {
262+
const timestamp = new Date().toISOString();
263+
if (primaryAgent) {
264+
this.ruleEventCollector.record({
265+
type: 'specialist_dispatched',
266+
timestamp,
267+
domain: primaryAgent,
268+
});
269+
}
270+
if (result.parallelAgents) {
271+
for (const agent of result.parallelAgents as { name: string }[]) {
272+
this.ruleEventCollector.record({
273+
type: 'specialist_dispatched',
274+
timestamp,
275+
domain: agent.name,
276+
});
277+
}
278+
}
279+
} catch {
280+
// Fire-and-forget: never break handler execution
281+
}
282+
259283
return createJsonResponse(result);
260284
} catch (error) {
261285
return createErrorResponse(

‎apps/mcp-server/src/mcp/handlers/checklist-context.handler.spec.ts‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,14 @@ import { ChecklistContextHandler } from './checklist-context.handler';
33
import { ChecklistService } from '../../checklist/checklist.service';
44
import { ContextService } from '../../context/context.service';
55
import type { ImpactEventService } from '../../impact';
6+
import type { RuleEventCollector } from '../../rules/rule-event-collector';
67

78
describe('ChecklistContextHandler', () => {
89
let handler: ChecklistContextHandler;
910
let mockChecklistService: ChecklistService;
1011
let mockContextService: ContextService;
1112
let mockImpactEventService: Partial<ImpactEventService>;
13+
let mockRuleEventCollector: Partial<RuleEventCollector>;
1214

1315
const mockChecklistResult = {
1416
checklists: [{ domain: 'security', items: [], priority: 'high', icon: '🔒' }],
@@ -50,11 +52,13 @@ describe('ChecklistContextHandler', () => {
5052
} as unknown as ContextService;
5153

5254
mockImpactEventService = { logEvent: vi.fn() };
55+
mockRuleEventCollector = { record: vi.fn() };
5356

5457
handler = new ChecklistContextHandler(
5558
mockChecklistService,
5659
mockContextService,
5760
mockImpactEventService as ImpactEventService,
61+
mockRuleEventCollector as RuleEventCollector,
5862
);
5963
});
6064

@@ -126,6 +130,45 @@ describe('ChecklistContextHandler', () => {
126130
text: expect.stringContaining('Checklist error'),
127131
});
128132
});
133+
134+
it('should record checklist_generated event for each domain', async () => {
135+
await handler.handle('generate_checklist', {
136+
domains: ['security', 'accessibility'],
137+
});
138+
139+
expect(mockRuleEventCollector.record).toHaveBeenCalledWith(
140+
expect.objectContaining({
141+
type: 'checklist_generated',
142+
domain: 'security',
143+
}),
144+
);
145+
expect(mockRuleEventCollector.record).toHaveBeenCalledWith(
146+
expect.objectContaining({
147+
type: 'checklist_generated',
148+
domain: 'accessibility',
149+
}),
150+
);
151+
});
152+
153+
it('should not record events when checklist generation fails', async () => {
154+
mockChecklistService.generateChecklist = vi.fn().mockRejectedValue(new Error('fail'));
155+
156+
await handler.handle('generate_checklist', { domains: ['security'] });
157+
158+
expect(mockRuleEventCollector.record).not.toHaveBeenCalled();
159+
});
160+
161+
it('should not break handler when event recording throws', async () => {
162+
mockRuleEventCollector.record = vi.fn().mockImplementation(() => {
163+
throw new Error('record error');
164+
});
165+
166+
const result = await handler.handle('generate_checklist', {
167+
domains: ['security'],
168+
});
169+
170+
expect(result?.isError).toBeFalsy();
171+
});
129172
});
130173

131174
describe('analyze_task', () => {

‎apps/mcp-server/src/mcp/handlers/checklist-context.handler.ts‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import {
1313
type ValidMode,
1414
} from '../../shared/validation.constants';
1515
import { ImpactEventService } from '../../impact';
16+
import { RuleEventCollector } from '../../rules/rule-event-collector';
1617

1718
/**
1819
* Valid checklist domains for runtime validation
@@ -37,6 +38,7 @@ export class ChecklistContextHandler extends AbstractHandler {
3738
private readonly checklistService: ChecklistService,
3839
private readonly contextService: ContextService,
3940
private readonly impactEventService: ImpactEventService,
41+
private readonly ruleEventCollector: RuleEventCollector,
4042
) {
4143
super();
4244
}
@@ -147,6 +149,19 @@ export class ChecklistContextHandler extends AbstractHandler {
147149
// Never break handler execution
148150
}
149151

152+
try {
153+
const timestamp = new Date().toISOString();
154+
for (const domain of domains ?? []) {
155+
this.ruleEventCollector.record({
156+
type: 'checklist_generated',
157+
timestamp,
158+
domain,
159+
});
160+
}
161+
} catch {
162+
// Fire-and-forget: never break handler execution
163+
}
164+
150165
return createJsonResponse(result);
151166
} catch (error) {
152167
return createErrorResponse(

‎apps/mcp-server/src/mcp/handlers/discussion.handler.spec.ts‎

Lines changed: 48 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,15 @@
1-
import { describe, it, expect, beforeEach } from 'vitest';
1+
import { describe, it, expect, beforeEach, vi } from 'vitest';
22
import { DiscussionHandler } from './discussion.handler';
33
import type { AgentOpinion, DiscussionResult } from './discussion.types';
4+
import type { RuleEventCollector } from '../../rules/rule-event-collector';
45

56
describe('DiscussionHandler', () => {
67
let handler: DiscussionHandler;
8+
let mockRuleEventCollector: Partial<RuleEventCollector>;
79

810
beforeEach(() => {
9-
handler = new DiscussionHandler();
11+
mockRuleEventCollector = { record: vi.fn() };
12+
handler = new DiscussionHandler(mockRuleEventCollector as RuleEventCollector);
1013
});
1114

1215
describe('handle', () => {
@@ -167,6 +170,49 @@ describe('DiscussionHandler', () => {
167170
});
168171
});
169172

173+
describe('rule event tracking', () => {
174+
it('should record specialist_dispatched event for each specialist', async () => {
175+
await handler.handle('agent_discussion', {
176+
topic: 'Auth review',
177+
specialists: ['security-specialist', 'performance-specialist'],
178+
});
179+
180+
expect(mockRuleEventCollector.record).toHaveBeenCalledWith(
181+
expect.objectContaining({
182+
type: 'specialist_dispatched',
183+
domain: 'security-specialist',
184+
}),
185+
);
186+
expect(mockRuleEventCollector.record).toHaveBeenCalledWith(
187+
expect.objectContaining({
188+
type: 'specialist_dispatched',
189+
domain: 'performance-specialist',
190+
}),
191+
);
192+
});
193+
194+
it('should not record events when validation fails', async () => {
195+
await handler.handle('agent_discussion', {
196+
specialists: ['security-specialist'],
197+
});
198+
199+
expect(mockRuleEventCollector.record).not.toHaveBeenCalled();
200+
});
201+
202+
it('should not break handler when event recording throws', async () => {
203+
mockRuleEventCollector.record = vi.fn().mockImplementation(() => {
204+
throw new Error('record error');
205+
});
206+
207+
const result = await handler.handle('agent_discussion', {
208+
topic: 'Auth review',
209+
specialists: ['security-specialist'],
210+
});
211+
212+
expect(result?.isError).toBeFalsy();
213+
});
214+
});
215+
170216
describe('getToolDefinitions', () => {
171217
it('should return tool definitions', () => {
172218
const definitions = handler.getToolDefinitions();

0 commit comments

Comments
 (0)