diff --git a/apps/mcp-server/src/mcp/handlers/abstract-handler.integration.spec.ts b/apps/mcp-server/src/mcp/handlers/abstract-handler.integration.spec.ts index 7c216ad7..7c933251 100644 --- a/apps/mcp-server/src/mcp/handlers/abstract-handler.integration.spec.ts +++ b/apps/mcp-server/src/mcp/handlers/abstract-handler.integration.spec.ts @@ -20,6 +20,7 @@ import { StateService } from '../../state/state.service'; import { ContextDocumentService } from '../../context/context-document.service'; import { DiagnosticLogService } from '../../diagnostic/diagnostic-log.service'; import type { ImpactEventService } from '../../impact'; +import type { RuleEventCollector } from '../../rules/rule-event-collector'; /** * Integration tests verifying all concrete handlers inherit @@ -140,13 +141,19 @@ describe('Handler Security Integration', () => { } as unknown as DiagnosticLogService; const mockImpactEventService = { logEvent: vi.fn() } as unknown as ImpactEventService; + const mockRuleEventCollector = { record: vi.fn() } as unknown as RuleEventCollector; // Initialize handlers - agentHandler = new AgentHandler(mockAgentService, mockImpactEventService); + agentHandler = new AgentHandler( + mockAgentService, + mockImpactEventService, + mockRuleEventCollector, + ); checklistHandler = new ChecklistContextHandler( mockChecklistService, mockContextService, mockImpactEventService, + mockRuleEventCollector, ); configHandler = new ConfigHandler( mockConfigService, @@ -168,6 +175,7 @@ describe('Handler Security Integration', () => { mockDiagnosticLogService, mockAgentServiceForMode as AgentService, mockImpactEventService, + mockRuleEventCollector, ); rulesHandler = new RulesHandler( mockRulesService, diff --git a/apps/mcp-server/src/mcp/handlers/agent.handler.spec.ts b/apps/mcp-server/src/mcp/handlers/agent.handler.spec.ts index fe90a900..f83aa086 100644 --- a/apps/mcp-server/src/mcp/handlers/agent.handler.spec.ts +++ b/apps/mcp-server/src/mcp/handlers/agent.handler.spec.ts @@ -2,11 +2,13 @@ import { describe, it, expect, beforeEach, vi } from 'vitest'; import { AgentHandler } from './agent.handler'; import { AgentService } from '../../agent/agent.service'; import type { ImpactEventService } from '../../impact'; +import type { RuleEventCollector } from '../../rules/rule-event-collector'; describe('AgentHandler', () => { let handler: AgentHandler; let mockAgentService: AgentService; let mockImpactEventService: Partial; + let mockRuleEventCollector: Partial; const mockSystemPromptResult = { agentName: 'security-specialist', @@ -25,8 +27,13 @@ describe('AgentHandler', () => { } as unknown as AgentService; mockImpactEventService = { logEvent: vi.fn() }; + mockRuleEventCollector = { record: vi.fn() }; - handler = new AgentHandler(mockAgentService, mockImpactEventService as ImpactEventService); + handler = new AgentHandler( + mockAgentService, + mockImpactEventService as ImpactEventService, + mockRuleEventCollector as RuleEventCollector, + ); }); describe('handle', () => { @@ -446,6 +453,76 @@ describe('AgentHandler', () => { }); }); + describe('rule event tracking', () => { + it('should record specialist_dispatched event on dispatch', async () => { + await handler.handle('dispatch_agents', { + mode: 'EVAL', + primaryAgent: 'security-specialist', + taskDescription: 'Review security', + }); + + expect(mockRuleEventCollector.record).toHaveBeenCalledWith( + expect.objectContaining({ + type: 'specialist_dispatched', + domain: 'security-specialist', + }), + ); + }); + + it('should record events for each specialist in parallel dispatch', async () => { + mockAgentService.dispatchAgents = vi.fn().mockResolvedValue({ + ...mockDispatchResult, + parallelAgents: [ + { name: 'accessibility-specialist' }, + { name: 'performance-specialist' }, + ], + }); + + await handler.handle('dispatch_agents', { + mode: 'EVAL', + specialists: ['accessibility-specialist', 'performance-specialist'], + includeParallel: true, + }); + + expect(mockRuleEventCollector.record).toHaveBeenCalledWith( + expect.objectContaining({ + type: 'specialist_dispatched', + domain: 'accessibility-specialist', + }), + ); + expect(mockRuleEventCollector.record).toHaveBeenCalledWith( + expect.objectContaining({ + type: 'specialist_dispatched', + domain: 'performance-specialist', + }), + ); + }); + + it('should not record events when dispatch fails', async () => { + mockAgentService.dispatchAgents = vi.fn().mockRejectedValue(new Error('Dispatch failed')); + + await handler.handle('dispatch_agents', { + mode: 'EVAL', + primaryAgent: 'security-specialist', + }); + + expect(mockRuleEventCollector.record).not.toHaveBeenCalled(); + }); + + it('should not break handler when event recording throws', async () => { + mockRuleEventCollector.record = vi.fn().mockImplementation(() => { + throw new Error('record error'); + }); + + const result = await handler.handle('dispatch_agents', { + mode: 'EVAL', + primaryAgent: 'security-specialist', + }); + + expect(result?.isError).toBeFalsy(); + }); + }); + describe('executionStrategy parameter', () => { it('should pass executionStrategy "subagent" to service', async () => { await handler.handle('dispatch_agents', { diff --git a/apps/mcp-server/src/mcp/handlers/agent.handler.ts b/apps/mcp-server/src/mcp/handlers/agent.handler.ts index 120ae940..a4d7ca67 100644 --- a/apps/mcp-server/src/mcp/handlers/agent.handler.ts +++ b/apps/mcp-server/src/mcp/handlers/agent.handler.ts @@ -15,6 +15,7 @@ import { isRecordObject, } from '../../shared/validation.constants'; import { ImpactEventService } from '../../impact'; +import { RuleEventCollector } from '../../rules/rule-event-collector'; /** * Handler for agent-related tools @@ -27,6 +28,7 @@ export class AgentHandler extends AbstractHandler { constructor( private readonly agentService: AgentService, private readonly impactEventService: ImpactEventService, + private readonly ruleEventCollector: RuleEventCollector, ) { super(); } @@ -256,6 +258,28 @@ export class AgentHandler extends AbstractHandler { // Never break handler execution } + try { + const timestamp = new Date().toISOString(); + if (primaryAgent) { + this.ruleEventCollector.record({ + type: 'specialist_dispatched', + timestamp, + domain: primaryAgent, + }); + } + if (result.parallelAgents) { + for (const agent of result.parallelAgents as { name: string }[]) { + this.ruleEventCollector.record({ + type: 'specialist_dispatched', + timestamp, + domain: agent.name, + }); + } + } + } catch { + // Fire-and-forget: never break handler execution + } + return createJsonResponse(result); } catch (error) { return createErrorResponse( diff --git a/apps/mcp-server/src/mcp/handlers/checklist-context.handler.spec.ts b/apps/mcp-server/src/mcp/handlers/checklist-context.handler.spec.ts index 438871fe..b682cb06 100644 --- a/apps/mcp-server/src/mcp/handlers/checklist-context.handler.spec.ts +++ b/apps/mcp-server/src/mcp/handlers/checklist-context.handler.spec.ts @@ -3,12 +3,14 @@ import { ChecklistContextHandler } from './checklist-context.handler'; import { ChecklistService } from '../../checklist/checklist.service'; import { ContextService } from '../../context/context.service'; import type { ImpactEventService } from '../../impact'; +import type { RuleEventCollector } from '../../rules/rule-event-collector'; describe('ChecklistContextHandler', () => { let handler: ChecklistContextHandler; let mockChecklistService: ChecklistService; let mockContextService: ContextService; let mockImpactEventService: Partial; + let mockRuleEventCollector: Partial; const mockChecklistResult = { checklists: [{ domain: 'security', items: [], priority: 'high', icon: '🔒' }], @@ -50,11 +52,13 @@ describe('ChecklistContextHandler', () => { } as unknown as ContextService; mockImpactEventService = { logEvent: vi.fn() }; + mockRuleEventCollector = { record: vi.fn() }; handler = new ChecklistContextHandler( mockChecklistService, mockContextService, mockImpactEventService as ImpactEventService, + mockRuleEventCollector as RuleEventCollector, ); }); @@ -126,6 +130,45 @@ describe('ChecklistContextHandler', () => { text: expect.stringContaining('Checklist error'), }); }); + + it('should record checklist_generated event for each domain', async () => { + await handler.handle('generate_checklist', { + domains: ['security', 'accessibility'], + }); + + expect(mockRuleEventCollector.record).toHaveBeenCalledWith( + expect.objectContaining({ + type: 'checklist_generated', + domain: 'security', + }), + ); + expect(mockRuleEventCollector.record).toHaveBeenCalledWith( + expect.objectContaining({ + type: 'checklist_generated', + domain: 'accessibility', + }), + ); + }); + + it('should not record events when checklist generation fails', async () => { + mockChecklistService.generateChecklist = vi.fn().mockRejectedValue(new Error('fail')); + + await handler.handle('generate_checklist', { domains: ['security'] }); + + expect(mockRuleEventCollector.record).not.toHaveBeenCalled(); + }); + + it('should not break handler when event recording throws', async () => { + mockRuleEventCollector.record = vi.fn().mockImplementation(() => { + throw new Error('record error'); + }); + + const result = await handler.handle('generate_checklist', { + domains: ['security'], + }); + + expect(result?.isError).toBeFalsy(); + }); }); describe('analyze_task', () => { diff --git a/apps/mcp-server/src/mcp/handlers/checklist-context.handler.ts b/apps/mcp-server/src/mcp/handlers/checklist-context.handler.ts index 20539888..82fb3979 100644 --- a/apps/mcp-server/src/mcp/handlers/checklist-context.handler.ts +++ b/apps/mcp-server/src/mcp/handlers/checklist-context.handler.ts @@ -13,6 +13,7 @@ import { type ValidMode, } from '../../shared/validation.constants'; import { ImpactEventService } from '../../impact'; +import { RuleEventCollector } from '../../rules/rule-event-collector'; /** * Valid checklist domains for runtime validation @@ -37,6 +38,7 @@ export class ChecklistContextHandler extends AbstractHandler { private readonly checklistService: ChecklistService, private readonly contextService: ContextService, private readonly impactEventService: ImpactEventService, + private readonly ruleEventCollector: RuleEventCollector, ) { super(); } @@ -147,6 +149,19 @@ export class ChecklistContextHandler extends AbstractHandler { // Never break handler execution } + try { + const timestamp = new Date().toISOString(); + for (const domain of domains ?? []) { + this.ruleEventCollector.record({ + type: 'checklist_generated', + timestamp, + domain, + }); + } + } catch { + // Fire-and-forget: never break handler execution + } + return createJsonResponse(result); } catch (error) { return createErrorResponse( diff --git a/apps/mcp-server/src/mcp/handlers/discussion.handler.spec.ts b/apps/mcp-server/src/mcp/handlers/discussion.handler.spec.ts index fc751205..4018f04c 100644 --- a/apps/mcp-server/src/mcp/handlers/discussion.handler.spec.ts +++ b/apps/mcp-server/src/mcp/handlers/discussion.handler.spec.ts @@ -1,12 +1,15 @@ -import { describe, it, expect, beforeEach } from 'vitest'; +import { describe, it, expect, beforeEach, vi } from 'vitest'; import { DiscussionHandler } from './discussion.handler'; import type { AgentOpinion, DiscussionResult } from './discussion.types'; +import type { RuleEventCollector } from '../../rules/rule-event-collector'; describe('DiscussionHandler', () => { let handler: DiscussionHandler; + let mockRuleEventCollector: Partial; beforeEach(() => { - handler = new DiscussionHandler(); + mockRuleEventCollector = { record: vi.fn() }; + handler = new DiscussionHandler(mockRuleEventCollector as RuleEventCollector); }); describe('handle', () => { @@ -167,6 +170,49 @@ describe('DiscussionHandler', () => { }); }); + describe('rule event tracking', () => { + it('should record specialist_dispatched event for each specialist', async () => { + await handler.handle('agent_discussion', { + topic: 'Auth review', + specialists: ['security-specialist', 'performance-specialist'], + }); + + expect(mockRuleEventCollector.record).toHaveBeenCalledWith( + expect.objectContaining({ + type: 'specialist_dispatched', + domain: 'security-specialist', + }), + ); + expect(mockRuleEventCollector.record).toHaveBeenCalledWith( + expect.objectContaining({ + type: 'specialist_dispatched', + domain: 'performance-specialist', + }), + ); + }); + + it('should not record events when validation fails', async () => { + await handler.handle('agent_discussion', { + specialists: ['security-specialist'], + }); + + expect(mockRuleEventCollector.record).not.toHaveBeenCalled(); + }); + + it('should not break handler when event recording throws', async () => { + mockRuleEventCollector.record = vi.fn().mockImplementation(() => { + throw new Error('record error'); + }); + + const result = await handler.handle('agent_discussion', { + topic: 'Auth review', + specialists: ['security-specialist'], + }); + + expect(result?.isError).toBeFalsy(); + }); + }); + describe('getToolDefinitions', () => { it('should return tool definitions', () => { const definitions = handler.getToolDefinitions(); diff --git a/apps/mcp-server/src/mcp/handlers/discussion.handler.ts b/apps/mcp-server/src/mcp/handlers/discussion.handler.ts index bb7edddd..f0f76920 100644 --- a/apps/mcp-server/src/mcp/handlers/discussion.handler.ts +++ b/apps/mcp-server/src/mcp/handlers/discussion.handler.ts @@ -15,6 +15,7 @@ import type { ConsensusStatus, } from './discussion.types'; import { VALID_SEVERITIES } from './discussion.types'; +import { RuleEventCollector } from '../../rules/rule-event-collector'; /** * Specialist domain knowledge mapping. @@ -41,6 +42,10 @@ const SEVERITY_ORDER: readonly OpinionSeverity[] = ['info', 'low', 'medium', 'hi */ @Injectable() export class DiscussionHandler extends AbstractHandler { + constructor(private readonly ruleEventCollector: RuleEventCollector) { + super(); + } + protected getHandledTools(): string[] { return ['agent_discussion']; } @@ -121,6 +126,19 @@ export class DiscussionHandler extends AbstractHandler { maxSeverity, }; + try { + const timestamp = new Date().toISOString(); + for (const specialist of specialists) { + this.ruleEventCollector.record({ + type: 'specialist_dispatched', + timestamp, + domain: specialist, + }); + } + } catch { + // Fire-and-forget: never break handler execution + } + return createJsonResponse(result); } diff --git a/apps/mcp-server/src/mcp/handlers/mode.handler.spec.ts b/apps/mcp-server/src/mcp/handlers/mode.handler.spec.ts index 03c12018..989fa50e 100644 --- a/apps/mcp-server/src/mcp/handlers/mode.handler.spec.ts +++ b/apps/mcp-server/src/mcp/handlers/mode.handler.spec.ts @@ -9,6 +9,7 @@ import { ContextDocumentService } from '../../context/context-document.service'; import { DiagnosticLogService } from '../../diagnostic/diagnostic-log.service'; import type { AgentService } from '../../agent/agent.service'; import type { ImpactEventService } from '../../impact'; +import type { RuleEventCollector } from '../../rules/rule-event-collector'; describe('ModeHandler', () => { let handler: ModeHandler; @@ -21,6 +22,7 @@ describe('ModeHandler', () => { let mockDiagnosticLogService: DiagnosticLogService; let mockAgentService: Partial; let mockImpactEventService: Partial; + let mockRuleEventCollector: Partial; const mockParseModeResult = { mode: 'PLAN', @@ -131,6 +133,8 @@ describe('ModeHandler', () => { logEvent: vi.fn(), }; + mockRuleEventCollector = { record: vi.fn() }; + handler = new ModeHandler( mockKeywordService, mockConfigService, @@ -141,6 +145,7 @@ describe('ModeHandler', () => { mockDiagnosticLogService, mockAgentService as AgentService, mockImpactEventService as ImpactEventService, + mockRuleEventCollector as RuleEventCollector, ); }); @@ -1203,4 +1208,47 @@ describe('ModeHandler', () => { expect(parsed.agentDiscussion).toBeUndefined(); }); }); + + describe('rule event tracking', () => { + it('should record mode_activated event with mode as rule', async () => { + await handler.handle('parse_mode', { + prompt: 'PLAN test task', + }); + + expect(mockRuleEventCollector.record).toHaveBeenCalledWith( + expect.objectContaining({ + type: 'mode_activated', + rule: 'PLAN', + }), + ); + }); + + it('should include agent in event details', async () => { + await handler.handle('parse_mode', { + prompt: 'PLAN test task', + }); + + expect(mockRuleEventCollector.record).toHaveBeenCalledWith( + expect.objectContaining({ + type: 'mode_activated', + rule: 'PLAN', + details: expect.objectContaining({ + agent: 'plan-mode-agent', + }), + }), + ); + }); + + it('should not break handler when event recording throws', async () => { + mockRuleEventCollector.record = vi.fn().mockImplementation(() => { + throw new Error('record error'); + }); + + const result = await handler.handle('parse_mode', { + prompt: 'PLAN test task', + }); + + expect(result?.isError).toBeFalsy(); + }); + }); }); diff --git a/apps/mcp-server/src/mcp/handlers/mode.handler.ts b/apps/mcp-server/src/mcp/handlers/mode.handler.ts index c63c20e3..663d3e8b 100644 --- a/apps/mcp-server/src/mcp/handlers/mode.handler.ts +++ b/apps/mcp-server/src/mcp/handlers/mode.handler.ts @@ -30,6 +30,7 @@ import { buildVisualData, type AgentVisualInput } from '../../keyword/visual-dat import { isValidVerbosity } from '../../shared/verbosity.types'; import { AgentService } from '../../agent/agent.service'; import { ImpactEventService } from '../../impact'; +import { RuleEventCollector } from '../../rules/rule-event-collector'; /** Maximum length for context title slug generation */ const CONTEXT_TITLE_MAX_LENGTH = 50; @@ -104,6 +105,7 @@ export class ModeHandler extends AbstractHandler { private readonly diagnosticLogService: DiagnosticLogService, private readonly agentService: AgentService, private readonly impactEventService: ImpactEventService, + private readonly ruleEventCollector: RuleEventCollector, ) { super(); } @@ -312,6 +314,17 @@ export class ModeHandler extends AbstractHandler { // Never break handler execution } + try { + this.ruleEventCollector.record({ + type: 'mode_activated', + timestamp: new Date().toISOString(), + rule: result.mode, + details: { agent: result.agent }, + }); + } catch { + // Fire-and-forget: never break handler execution + } + return response; } catch (error) { return createErrorResponse( diff --git a/apps/mcp-server/src/mcp/handlers/quality-report.handler.spec.ts b/apps/mcp-server/src/mcp/handlers/quality-report.handler.spec.ts index de4137ca..0cc894e0 100644 --- a/apps/mcp-server/src/mcp/handlers/quality-report.handler.spec.ts +++ b/apps/mcp-server/src/mcp/handlers/quality-report.handler.spec.ts @@ -1,13 +1,16 @@ -import { describe, it, expect, beforeEach } from 'vitest'; +import { describe, it, expect, beforeEach, vi } from 'vitest'; import { QualityReportHandler } from './quality-report.handler'; import { QualityReportService } from '../../ship/quality-report.service'; +import type { RuleEventCollector } from '../../rules/rule-event-collector'; describe('QualityReportHandler', () => { let handler: QualityReportHandler; + let mockRuleEventCollector: Partial; beforeEach(() => { const service = new QualityReportService(); - handler = new QualityReportHandler(service); + mockRuleEventCollector = { record: vi.fn() }; + handler = new QualityReportHandler(service, mockRuleEventCollector as RuleEventCollector); }); it('should return null for unhandled tools', async () => { @@ -60,4 +63,38 @@ describe('QualityReportHandler', () => { expect(defs[0].name).toBe('pr_quality_report'); expect(defs[0].inputSchema.required).toContain('changedFiles'); }); + + describe('rule event tracking', () => { + it('should record specialist_dispatched events for detected domains', async () => { + await handler.handle('pr_quality_report', { + changedFiles: ['src/auth/token.ts'], + }); + + expect(mockRuleEventCollector.record).toHaveBeenCalledWith( + expect.objectContaining({ + type: 'specialist_dispatched', + }), + ); + }); + + it('should not record events for empty changed files', async () => { + await handler.handle('pr_quality_report', { + changedFiles: [], + }); + + expect(mockRuleEventCollector.record).not.toHaveBeenCalled(); + }); + + it('should not break handler when event recording throws', async () => { + mockRuleEventCollector.record = vi.fn().mockImplementation(() => { + throw new Error('record error'); + }); + + const result = await handler.handle('pr_quality_report', { + changedFiles: ['src/auth/token.ts'], + }); + + expect(result?.isError).toBeFalsy(); + }); + }); }); diff --git a/apps/mcp-server/src/mcp/handlers/quality-report.handler.ts b/apps/mcp-server/src/mcp/handlers/quality-report.handler.ts index ac34eda8..320573b9 100644 --- a/apps/mcp-server/src/mcp/handlers/quality-report.handler.ts +++ b/apps/mcp-server/src/mcp/handlers/quality-report.handler.ts @@ -5,12 +5,16 @@ import { AbstractHandler } from './abstract-handler'; import { QualityReportService } from '../../ship/quality-report.service'; import { createJsonResponse, createErrorResponse } from '../response.utils'; import { extractStringArray } from '../../shared/validation.constants'; +import { RuleEventCollector } from '../../rules/rule-event-collector'; @Injectable() export class QualityReportHandler extends AbstractHandler { private readonly logger = new Logger(QualityReportHandler.name); - constructor(private readonly qualityReportService: QualityReportService) { + constructor( + private readonly qualityReportService: QualityReportService, + private readonly ruleEventCollector: RuleEventCollector, + ) { super(); } @@ -37,6 +41,22 @@ export class QualityReportHandler extends AbstractHandler { timeout, }); + try { + const domains = result.domains as { domain: string }[] | undefined; + if (domains?.length) { + const timestamp = new Date().toISOString(); + for (const d of domains) { + this.ruleEventCollector.record({ + type: 'specialist_dispatched', + timestamp, + domain: d.domain, + }); + } + } + } catch { + // Fire-and-forget: never break handler execution + } + return createJsonResponse(result); } catch (error) { this.logger.error(`Quality report failed: ${error}`); diff --git a/apps/mcp-server/src/mcp/mcp.service.spec.ts b/apps/mcp-server/src/mcp/mcp.service.spec.ts index a0ce76a1..4e036154 100644 --- a/apps/mcp-server/src/mcp/mcp.service.spec.ts +++ b/apps/mcp-server/src/mcp/mcp.service.spec.ts @@ -18,6 +18,7 @@ import { ModelResolverService } from '../model'; import { StateService } from '../state/state.service'; import { DiagnosticLogService } from '../diagnostic/diagnostic-log.service'; import type { ImpactEventService } from '../impact'; +import type { RuleEventCollector } from '../rules/rule-event-collector'; import type { ToolHandler } from './handlers'; import { RulesHandler, @@ -452,6 +453,7 @@ interface CreateMcpServiceOptions { stateService?: Partial; diagnosticLogService?: Partial; impactEventService?: Partial; + ruleEventCollector?: Partial; } function createMcpServiceWithHandlers( @@ -478,6 +480,7 @@ function createMcpServiceWithHandlers( new AgentHandler( services.agentService as AgentService, services.impactEventService as ImpactEventService, + services.ruleEventCollector as RuleEventCollector, ), new ModeHandler( services.keywordService as KeywordService, @@ -489,11 +492,13 @@ function createMcpServiceWithHandlers( services.diagnosticLogService as DiagnosticLogService, services.agentService as AgentService, services.impactEventService as ImpactEventService, + services.ruleEventCollector as RuleEventCollector, ), new ChecklistContextHandler( services.checklistService as ChecklistService, services.contextService as ContextService, services.impactEventService as ImpactEventService, + services.ruleEventCollector as RuleEventCollector, ), ]; @@ -582,6 +587,7 @@ describe('McpService', () => { stateService: mockStateService, diagnosticLogService: mockDiagnosticLogService, impactEventService: { logEvent: vi.fn() }, + ruleEventCollector: { record: vi.fn() }, }; const mcpService = createMcpServiceWithHandlers(defaultMocks);