feat(mcp): add review_pr MCP tool for EVAL-mode PR review

- Add ReviewPrHandler with review_pr tool accepting pr_number and optional issue_number
- Add ReviewPrService orchestrating gh CLI, generate_checklist, and FileSpecialistMapper
- Returns PR metadata, diff, checklists, recommended specialists, and review protocol
- Register handler in McpModule and service in ShipModule

Closes #1409

Author: JeremyDev87

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

@@ -162,6 +162,12 @@ export { ResumeHandler } from './resume.handler';
  */
 export { RuleImpactHandler } from './rule-impact.handler';
 
+/**
+ * Handler for PR review tools (review_pr)
+ * @see {@link ReviewPrHandler}
+ */
+export { ReviewPrHandler } from './review-pr.handler';
+
 /**
  * Injection token for the array of all tool handlers.
  *

apps/mcp-server/src/mcp/handlers/review-pr.handler.spec.ts

@@ -0,0 +1,86 @@
+import { describe, it, expect, beforeEach, vi } from 'vitest';
+import { ReviewPrHandler } from './review-pr.handler';
+import { ReviewPrService } from '../../ship/review-pr.service';
+import type { ReviewPrResult } from '../../ship/review-pr.types';
+
+describe('ReviewPrHandler', () => {
+  let handler: ReviewPrHandler;
+  let mockService: { reviewPr: ReturnType<typeof vi.fn> };
+
+  const sampleResult: ReviewPrResult = {
+    prMeta: {
+      title: 'feat: add feature',
+      branch: 'feat/add-feature-123',
+      changedFiles: ['src/auth/login.ts', 'src/auth/login.spec.ts'],
+      additions: 50,
+      deletions: 10,
+    },
+    diff: 'diff --git a/src/auth/login.ts ...',
+    checklists: [{ domain: 'security', items: ['Check auth'] }],
+    recommendedSpecialists: ['security-specialist', 'code-quality-specialist'],
+    acceptanceCriteria: '- [ ] Login works\n- [ ] Tests pass',
+    reviewProtocol: 'Follow pr-review-cycle.md',
+    duration: 150,
+  };
+
+  beforeEach(() => {
+    mockService = { reviewPr: vi.fn().mockResolvedValue(sampleResult) };
+    handler = new ReviewPrHandler(mockService as unknown as ReviewPrService);
+  });
+
+  it('should return null for unhandled tools', async () => {
+    const result = await handler.handle('unknown_tool', {});
+    expect(result).toBeNull();
+  });
+
+  it('should handle review_pr with pr_number', async () => {
+    const result = await handler.handle('review_pr', { pr_number: 1406 });
+    expect(result).not.toBeNull();
+    expect(result!.isError).toBeUndefined();
+    const data = JSON.parse(result!.content[0].text);
+    expect(data.prMeta.title).toBe('feat: add feature');
+    expect(data.recommendedSpecialists).toContain('security-specialist');
+    expect(mockService.reviewPr).toHaveBeenCalledWith({
+      prNumber: 1406,
+      issueNumber: undefined,
+      timeout: 30000,
+    });
+  });
+
+  it('should pass issue_number when provided', async () => {
+    await handler.handle('review_pr', { pr_number: 1406, issue_number: 1364 });
+    expect(mockService.reviewPr).toHaveBeenCalledWith({
+      prNumber: 1406,
+      issueNumber: 1364,
+      timeout: 30000,
+    });
+  });
+
+  it('should return error when pr_number is missing', async () => {
+    const result = await handler.handle('review_pr', {});
+    expect(result).not.toBeNull();
+    expect(result!.isError).toBe(true);
+    expect(result!.content[0].text).toContain('pr_number');
+  });
+
+  it('should return error when pr_number is not a number', async () => {
+    const result = await handler.handle('review_pr', { pr_number: 'abc' });
+    expect(result).not.toBeNull();
+    expect(result!.isError).toBe(true);
+  });
+
+  it('should handle service errors gracefully', async () => {
+    mockService.reviewPr.mockRejectedValue(new Error('gh command failed'));
+    const result = await handler.handle('review_pr', { pr_number: 9999 });
+    expect(result).not.toBeNull();
+    expect(result!.isError).toBe(true);
+    expect(result!.content[0].text).toContain('gh command failed');
+  });
+
+  it('should expose tool definitions', () => {
+    const defs = handler.getToolDefinitions();
+    expect(defs).toHaveLength(1);
+    expect(defs[0].name).toBe('review_pr');
+    expect(defs[0].inputSchema.required).toContain('pr_number');
+  });
+});

apps/mcp-server/src/mcp/handlers/review-pr.handler.ts

@@ -0,0 +1,79 @@
+import { Injectable, Logger } from '@nestjs/common';
+import type { ToolDefinition } from './base.handler';
+import type { ToolResponse } from '../response.utils';
+import { AbstractHandler } from './abstract-handler';
+import { ReviewPrService } from '../../ship/review-pr.service';
+import { createJsonResponse, createErrorResponse } from '../response.utils';
+
+@Injectable()
+export class ReviewPrHandler extends AbstractHandler {
+  private readonly logger = new Logger(ReviewPrHandler.name);
+
+  constructor(private readonly reviewPrService: ReviewPrService) {
+    super();
+  }
+
+  protected getHandledTools(): string[] {
+    return ['review_pr'];
+  }
+
+  protected async handleTool(
+    _toolName: string,
+    args: Record<string, unknown> | undefined,
+  ): Promise<ToolResponse> {
+    try {
+      const prNumber = args?.pr_number;
+      if (typeof prNumber !== 'number' || !Number.isFinite(prNumber) || prNumber <= 0) {
+        return createErrorResponse('pr_number is required and must be a positive number');
+      }
+
+      const issueNumber =
+        typeof args?.issue_number === 'number' && args.issue_number > 0
+          ? args.issue_number
+          : undefined;
+
+      const timeout = typeof args?.timeout === 'number' && args.timeout > 0 ? args.timeout : 30000;
+
+      const result = await this.reviewPrService.reviewPr({
+        prNumber,
+        issueNumber,
+        timeout,
+      });
+
+      return createJsonResponse(result);
+    } catch (error) {
+      this.logger.error(`review_pr failed: ${error}`);
+      return createErrorResponse(
+        `review_pr failed: ${error instanceof Error ? error.message : String(error)}`,
+      );
+    }
+  }
+
+  getToolDefinitions(): ToolDefinition[] {
+    return [
+      {
+        name: 'review_pr',
+        description:
+          'Fetch PR metadata, diff, checklists, and specialist recommendations for EVAL-mode PR review. Returns structured data for comprehensive code review.',
+        inputSchema: {
+          type: 'object',
+          properties: {
+            pr_number: {
+              type: 'number',
+              description: 'PR number to review',
+            },
+            issue_number: {
+              type: 'number',
+              description: 'Optional linked issue number for spec compliance check',
+            },
+            timeout: {
+              type: 'number',
+              description: 'Timeout in milliseconds (default: 30000)',
+            },
+          },
+          required: ['pr_number'],
+        },
+      },
+    ];
+  }
+}

apps/mcp-server/src/mcp/mcp.module.ts

@@ -39,6 +39,7 @@ import {
   PluginValidationHandler,
   ReleaseCheckHandler,
   QualityReportHandler,
+  ReviewPrHandler,
   BriefingHandler,
   ResumeHandler,
   RuleImpactHandler,
@@ -63,6 +64,7 @@ const handlers = [
   PluginValidationHandler,
   ReleaseCheckHandler,
   QualityReportHandler,
+  ReviewPrHandler,
   BriefingHandler,
   ResumeHandler,
   RuleImpactHandler,

apps/mcp-server/src/ship/review-pr.service.ts

@@ -0,0 +1,131 @@
+import { Injectable, Logger } from '@nestjs/common';
+import { execSync } from 'child_process';
+import { FileSpecialistMapper } from './file-specialist-mapper';
+import { ChecklistService } from '../checklist/checklist.service';
+import type { ReviewPrInput, ReviewPrResult, PrMeta } from './review-pr.types';
+
+@Injectable()
+export class ReviewPrService {
+  private readonly logger = new Logger(ReviewPrService.name);
+  private readonly mapper = new FileSpecialistMapper();
+
+  constructor(private readonly checklistService: ChecklistService) {}
+
+  async reviewPr(input: ReviewPrInput): Promise<ReviewPrResult> {
+    const start = Date.now();
+    const { prNumber, issueNumber } = input;
+
+    const prMeta = this.fetchPrMeta(prNumber);
+    const diff = this.fetchPrDiff(prNumber);
+
+    const checklists = await this.generateChecklists(prMeta.changedFiles);
+    const recommendedSpecialists = this.mapSpecialists(prMeta.changedFiles);
+    const acceptanceCriteria = issueNumber ? this.fetchIssueCriteria(issueNumber) : null;
+
+    const reviewProtocol = this.buildReviewProtocol(prNumber, issueNumber);
+
+    return {
+      prMeta,
+      diff,
+      checklists,
+      recommendedSpecialists,
+      acceptanceCriteria,
+      reviewProtocol,
+      duration: Date.now() - start,
+    };
+  }
+
+  private fetchPrMeta(prNumber: number): PrMeta {
+    try {
+      const raw = execSync(
+        `gh pr view ${prNumber} --json title,headRefName,files,additions,deletions`,
+        { encoding: 'utf-8', timeout: 15000 },
+      );
+      const data = JSON.parse(raw);
+      return {
+        title: data.title ?? '',
+        branch: data.headRefName ?? '',
+        changedFiles: (data.files ?? []).map((f: { path: string }) => f.path),
+        additions: data.additions ?? 0,
+        deletions: data.deletions ?? 0,
+      };
+    } catch (error) {
+      throw new Error(
+        `Failed to fetch PR #${prNumber}: ${error instanceof Error ? error.message : String(error)}`,
+      );
+    }
+  }
+
+  private fetchPrDiff(prNumber: number): string {
+    try {
+      return execSync(`gh pr diff ${prNumber}`, {
+        encoding: 'utf-8',
+        timeout: 15000,
+        maxBuffer: 10 * 1024 * 1024,
+      });
+    } catch (error) {
+      this.logger.warn(`Failed to fetch diff for PR #${prNumber}: ${error}`);
+      return '';
+    }
+  }
+
+  private async generateChecklists(changedFiles: string[]): Promise<unknown[]> {
+    if (changedFiles.length === 0) return [];
+    try {
+      const result = await this.checklistService.generateChecklist({
+        files: changedFiles,
+      });
+      return result.checklists;
+    } catch (error) {
+      this.logger.warn(`Checklist generation failed: ${error}`);
+      return [];
+    }
+  }
+
+  private mapSpecialists(changedFiles: string[]): string[] {
+    const mapped = this.mapper.mapFiles(changedFiles);
+    const specialists = [...new Set(mapped.map(m => m.specialist))];
+    return specialists;
+  }
+
+  private fetchIssueCriteria(issueNumber: number): string | null {
+    try {
+      const raw = execSync(`gh issue view ${issueNumber} --json body`, {
+        encoding: 'utf-8',
+        timeout: 10000,
+      });
+      const data = JSON.parse(raw);
+      const body: string = data.body ?? '';
+
+      const criteriaMatch = body.match(/## Acceptance criteria\s*\n([\s\S]*?)(?=\n## |\n---|$)/i);
+      return criteriaMatch ? criteriaMatch[1].trim() : body.slice(0, 2000);
+    } catch {
+      return null;
+    }
+  }
+
+  private buildReviewProtocol(prNumber: number, issueNumber?: number): string {
+    const issueCmd = issueNumber ? `\ngh issue view ${issueNumber}` : '';
+    return [
+      'Follow the canonical PR review cycle (pr-review-cycle.md):',
+      '',
+      '1. CI GATE (BLOCKING): gh pr checks ' + prNumber,
+      '2. LOCAL VERIFICATION: yarn lint && yarn type-check',
+      '3. READ THE DIFF: gh pr diff ' + prNumber,
+      '4. CODE QUALITY SCAN: unused imports, any types, missing error handling, dead code',
+      '5. SPEC COMPLIANCE: ' + (issueNumber ? `gh issue view ${issueNumber}` : 'N/A'),
+      '6. TEST COVERAGE: new logic tested? edge cases covered?',
+      '7. WRITE REVIEW: gh pr review ' + prNumber + ' --comment --body "<structured review>"',
+      '',
+      'Severity scale: critical / high / medium / low (see severity-classification.md)',
+      'Approval gate: Critical=0 AND High=0',
+      '',
+      'Commands:',
+      `gh pr checks ${prNumber}`,
+      `gh pr diff ${prNumber}`,
+      issueCmd,
+    ]
+      .filter(Boolean)
+      .join('\n');
+  }
+}

apps/mcp-server/src/ship/review-pr.types.ts

@@ -0,0 +1,23 @@
+export interface ReviewPrInput {
+  prNumber: number;
+  issueNumber?: number;
+  timeout?: number;
+}
+
+export interface PrMeta {
+  title: string;
+  branch: string;
+  changedFiles: string[];
+  additions: number;
+  deletions: number;
+}
+
+export interface ReviewPrResult {
+  prMeta: PrMeta;
+  diff: string;
+  checklists: unknown[];
+  recommendedSpecialists: string[];
+  acceptanceCriteria: string | null;
+  reviewProtocol: string;
+  duration: number;
+}

apps/mcp-server/src/ship/ship.module.ts

@@ -1,8 +1,11 @@
 import { Module } from '@nestjs/common';
+import { ChecklistModule } from '../checklist/checklist.module';
 import { QualityReportService } from './quality-report.service';
+import { ReviewPrService } from './review-pr.service';
 
 @Module({
-  providers: [QualityReportService],
-  exports: [QualityReportService],
+  imports: [ChecklistModule],
+  providers: [QualityReportService, ReviewPrService],
+  exports: [QualityReportService, ReviewPrService],
 })
 export class ShipModule {}