diff --git a/apps/frontend/src/main/ipc-handlers/github/pr-handlers.ts b/apps/frontend/src/main/ipc-handlers/github/pr-handlers.ts index 736fdadf..d1dacecf 100644 --- a/apps/frontend/src/main/ipc-handlers/github/pr-handlers.ts +++ b/apps/frontend/src/main/ipc-handlers/github/pr-handlers.ts @@ -8,27 +8,32 @@ * 4. Apply fixes */ -import { ipcMain } from 'electron'; -import type { BrowserWindow } from 'electron'; -import path from 'path'; -import fs from 'fs'; -import { IPC_CHANNELS, MODEL_ID_MAP, DEFAULT_FEATURE_MODELS, DEFAULT_FEATURE_THINKING } from '../../../shared/constants'; -import { getGitHubConfig, githubFetch } from './utils'; -import { readSettingsFile } from '../../settings-utils'; -import { getAugmentedEnv } from '../../env-utils'; -import { getMemoryService, getDefaultDbPath } from '../../memory-service'; -import type { Project, AppSettings } from '../../../shared/types'; -import { createContextLogger } from './utils/logger'; -import { withProjectOrNull } from './utils/project-middleware'; -import { createIPCCommunicators } from './utils/ipc-communicator'; -import { getRunnerEnv } from './utils/runner-env'; +import { ipcMain } from "electron"; +import type { BrowserWindow } from "electron"; +import path from "path"; +import fs from "fs"; +import { + IPC_CHANNELS, + MODEL_ID_MAP, + DEFAULT_FEATURE_MODELS, + DEFAULT_FEATURE_THINKING, +} from "../../../shared/constants"; +import { getGitHubConfig, githubFetch } from "./utils"; +import { readSettingsFile } from "../../settings-utils"; +import { getAugmentedEnv } from "../../env-utils"; +import { getMemoryService, getDefaultDbPath } from "../../memory-service"; +import type { Project, AppSettings } from "../../../shared/types"; +import { createContextLogger } from "./utils/logger"; +import { withProjectOrNull } from "./utils/project-middleware"; +import { createIPCCommunicators } from "./utils/ipc-communicator"; +import { getRunnerEnv } from "./utils/runner-env"; import { runPythonSubprocess, getPythonPath, getRunnerPath, validateGitHubModule, buildRunnerArgs, -} from './utils/subprocess-runner'; +} from "./utils/subprocess-runner"; /** * Sanitize network data before writing to file @@ -38,15 +43,34 @@ function sanitizeNetworkData(data: string, maxLength = 1000000): string { // Remove null bytes and other control characters except newlines/tabs/carriage returns // Using code points instead of escape sequences to avoid no-control-regex ESLint rule const controlCharsPattern = new RegExp( - '[' + - String.fromCharCode(0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08) + // \x00-\x08 - String.fromCharCode(0x0B, 0x0C) + // \x0B, \x0C (skip \x0A which is newline) - String.fromCharCode(0x0E, 0x0F, 0x10, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17, 0x18, 0x19, 0x1A, 0x1B, 0x1C, 0x1D, 0x1E, 0x1F) + // \x0E-\x1F - String.fromCharCode(0x7F) + // \x7F (DEL) - ']', - 'g' + "[" + + String.fromCharCode(0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08) + // \x00-\x08 + String.fromCharCode(0x0b, 0x0c) + // \x0B, \x0C (skip \x0A which is newline) + String.fromCharCode( + 0x0e, + 0x0f, + 0x10, + 0x11, + 0x12, + 0x13, + 0x14, + 0x15, + 0x16, + 0x17, + 0x18, + 0x19, + 0x1a, + 0x1b, + 0x1c, + 0x1d, + 0x1e, + 0x1f + ) + // \x0E-\x1F + String.fromCharCode(0x7f) + // \x7F (DEL) + "]", + "g" ); - let sanitized = data.replace(controlCharsPattern, ''); + let sanitized = data.replace(controlCharsPattern, ""); // Limit length to prevent DoS if (sanitized.length > maxLength) { @@ -57,13 +81,13 @@ function sanitizeNetworkData(data: string, maxLength = 1000000): string { } // Debug logging -const { debug: debugLog } = createContextLogger('GitHub PR'); +const { debug: debugLog } = createContextLogger("GitHub PR"); /** * Registry of running PR review processes * Key format: `${projectId}:${prNumber}` */ -const runningReviews = new Map(); +const runningReviews = new Map(); /** * Get the registry key for a PR review @@ -76,7 +100,7 @@ function getReviewKey(projectId: string, prNumber: number): string { * Returns env vars for Claude.md usage; enabled unless explicitly opted out. */ function getClaudeMdEnv(project: Project): Record | undefined { - return project.settings?.useClaudeMd !== false ? { USE_CLAUDE_MD: 'true' } : undefined; + return project.settings?.useClaudeMd !== false ? { USE_CLAUDE_MD: "true" } : undefined; } /** @@ -84,8 +108,8 @@ function getClaudeMdEnv(project: Project): Record | undefined { */ export interface PRReviewFinding { id: string; - severity: 'critical' | 'high' | 'medium' | 'low'; - category: 'security' | 'quality' | 'style' | 'test' | 'docs' | 'pattern' | 'performance'; + severity: "critical" | "high" | "medium" | "low"; + category: "security" | "quality" | "style" | "test" | "docs" | "pattern" | "performance"; title: string; description: string; file: string; @@ -104,7 +128,7 @@ export interface PRReviewResult { success: boolean; findings: PRReviewFinding[]; summary: string; - overallStatus: 'approve' | 'request_changes' | 'comment'; + overallStatus: "approve" | "request_changes" | "comment"; reviewId?: number; reviewedAt: string; error?: string; @@ -142,11 +166,11 @@ export interface MergeReadiness { /** PR is in draft mode */ isDraft: boolean; /** GitHub's mergeable status */ - mergeable: 'MERGEABLE' | 'CONFLICTING' | 'UNKNOWN'; + mergeable: "MERGEABLE" | "CONFLICTING" | "UNKNOWN"; /** Branch is behind base branch (out of date) */ isBehind: boolean; /** Simplified CI status */ - ciStatus: 'passing' | 'failing' | 'pending' | 'none'; + ciStatus: "passing" | "failing" | "pending" | "none"; /** List of blockers that contradict a "ready to merge" verdict */ blockers: string[]; } @@ -198,14 +222,14 @@ async function savePRReviewToMemory( ): Promise { const settings = readSettingsFile(); if (!settings?.memoryEnabled) { - debugLog('Memory not enabled, skipping PR review memory save'); + debugLog("Memory not enabled, skipping PR review memory save"); return; } try { const memoryService = getMemoryService({ dbPath: getDefaultDbPath(), - database: 'auto_claude_memory', + database: "auto_claude_memory", }); // Build the memory content with comprehensive insights @@ -213,17 +237,17 @@ async function savePRReviewToMemory( // Prioritize findings: critical > high > medium > low // Include all critical/high, top 5 medium, top 3 low - const criticalFindings = result.findings.filter(f => f.severity === 'critical'); - const highFindings = result.findings.filter(f => f.severity === 'high'); - const mediumFindings = result.findings.filter(f => f.severity === 'medium').slice(0, 5); - const lowFindings = result.findings.filter(f => f.severity === 'low').slice(0, 3); + const criticalFindings = result.findings.filter((f) => f.severity === "critical"); + const highFindings = result.findings.filter((f) => f.severity === "high"); + const mediumFindings = result.findings.filter((f) => f.severity === "medium").slice(0, 5); + const lowFindings = result.findings.filter((f) => f.severity === "low").slice(0, 3); const keyFindingsToSave = [ ...criticalFindings, ...highFindings, ...mediumFindings, ...lowFindings, - ].map(f => ({ + ].map((f) => ({ severity: f.severity, category: f.category, title: f.title, @@ -233,21 +257,25 @@ async function savePRReviewToMemory( })); // Extract gotchas: security issues, critical bugs, and common mistakes - const gotchaCategories = ['security', 'error_handling', 'data_validation', 'race_condition']; + const gotchaCategories = ["security", "error_handling", "data_validation", "race_condition"]; const gotchasToSave = result.findings - .filter(f => - f.severity === 'critical' || - f.severity === 'high' || - gotchaCategories.includes(f.category?.toLowerCase() || '') + .filter( + (f) => + f.severity === "critical" || + f.severity === "high" || + gotchaCategories.includes(f.category?.toLowerCase() || "") ) - .map(f => `[${f.category}] ${f.title}: ${f.description.substring(0, 300)}`); + .map((f) => `[${f.category}] ${f.title}: ${f.description.substring(0, 300)}`); // Extract patterns: group findings by category to identify recurring issues - const categoryGroups = result.findings.reduce((acc, f) => { - const cat = f.category || 'general'; - acc[cat] = (acc[cat] || 0) + 1; - return acc; - }, {} as Record); + const categoryGroups = result.findings.reduce( + (acc, f) => { + const cat = f.category || "general"; + acc[cat] = (acc[cat] || 0) + 1; + return acc; + }, + {} as Record + ); // Patterns are categories that appear multiple times (indicates a systematic issue) const patternsToSave = Object.entries(categoryGroups) @@ -257,15 +285,15 @@ async function savePRReviewToMemory( const memoryContent: PRReviewMemory = { prNumber: result.prNumber, repo, - verdict: result.overallStatus || 'unknown', + verdict: result.overallStatus || "unknown", timestamp: new Date().toISOString(), summary: { - verdict: result.overallStatus || 'unknown', + verdict: result.overallStatus || "unknown", finding_counts: { critical: criticalFindings.length, high: highFindings.length, - medium: result.findings.filter(f => f.severity === 'medium').length, - low: result.findings.filter(f => f.severity === 'low').length, + medium: result.findings.filter((f) => f.severity === "medium").length, + low: result.findings.filter((f) => f.severity === "low").length, }, total_findings: result.findings.length, }, @@ -277,29 +305,30 @@ async function savePRReviewToMemory( // Add follow-up specific info if applicable if (isFollowup && result.resolvedFindings && result.unresolvedFindings) { - memoryContent.summary.verdict_reasoning = - `Resolved: ${result.resolvedFindings.length}, Unresolved: ${result.unresolvedFindings.length}`; + memoryContent.summary.verdict_reasoning = `Resolved: ${result.resolvedFindings.length}, Unresolved: ${result.unresolvedFindings.length}`; } // Save to memory as a pr_review episode - const episodeName = `PR #${result.prNumber} ${isFollowup ? 'Follow-up ' : ''}Review - ${repo}`; + const episodeName = `PR #${result.prNumber} ${isFollowup ? "Follow-up " : ""}Review - ${repo}`; const saveResult = await memoryService.addEpisode( episodeName, memoryContent, - 'pr_review', - `pr_review_${repo.replace('/', '_')}` + "pr_review", + `pr_review_${repo.replace("/", "_")}` ); if (saveResult.success) { - debugLog('PR review saved to memory', { prNumber: result.prNumber, episodeId: saveResult.id }); + debugLog("PR review saved to memory", { + prNumber: result.prNumber, + episodeId: saveResult.id, + }); } else { - debugLog('Failed to save PR review to memory', { error: saveResult.error }); + debugLog("Failed to save PR review to memory", { error: saveResult.error }); } - } catch (error) { // Don't fail the review if memory save fails - debugLog('Error saving PR review to memory', { - error: error instanceof Error ? error.message : error + debugLog("Error saving PR review to memory", { + error: error instanceof Error ? error.message : error, }); } } @@ -334,7 +363,7 @@ export interface PRData { * PR review progress status */ export interface PRReviewProgress { - phase: 'fetching' | 'analyzing' | 'generating' | 'posting' | 'complete'; + phase: "fetching" | "analyzing" | "generating" | "posting" | "complete"; prNumber: number; progress: number; message: string; @@ -344,18 +373,26 @@ export interface PRReviewProgress { * Get the GitHub directory for a project */ function getGitHubDir(project: Project): string { - return path.join(project.path, '.auto-claude', 'github'); + return path.join(project.path, ".auto-claude", "github"); } /** * PR log phase type */ -type PRLogPhase = 'context' | 'analysis' | 'synthesis'; +type PRLogPhase = "context" | "analysis" | "synthesis"; /** * PR log entry type */ -type PRLogEntryType = 'text' | 'tool_start' | 'tool_end' | 'phase_start' | 'phase_end' | 'error' | 'success' | 'info'; +type PRLogEntryType = + | "text" + | "tool_start" + | "tool_end" + | "phase_start" + | "phase_end" + | "error" + | "success" + | "info"; /** * Single PR log entry @@ -375,7 +412,7 @@ interface PRLogEntry { */ interface PRPhaseLog { phase: PRLogPhase; - status: 'pending' | 'active' | 'completed' | 'failed'; + status: "pending" | "active" | "completed" | "failed"; started_at: string | null; completed_at: string | null; entries: PRLogEntry[]; @@ -429,7 +466,7 @@ function parseLogLine(line: string): { source: string; content: string; isError: for (const pattern of patterns) { const match = line.match(pattern); if (match) { - const isDebugOrError = pattern.source.includes('DEBUG') || pattern.source.includes('ERROR'); + const isDebugOrError = pattern.source.includes("DEBUG") || pattern.source.includes("ERROR"); if (isDebugOrError && match.length >= 3) { // Skip debug messages that only show message types (not useful) if (match[2].match(/^Message #\d+: \w+Message/)) { @@ -438,10 +475,10 @@ function parseLogLine(line: string): { source: string; content: string; isError: return { source: match[1], content: match[2], - isError: pattern.source.includes('ERROR'), + isError: pattern.source.includes("ERROR"), }; } - const source = line.match(/^\[(\w+(?:\s+\w+)*)\]/)?.[1] || 'Unknown'; + const source = line.match(/^\[(\w+(?:\s+\w+)*)\]/)?.[1] || "Unknown"; return { source, content: match[1] || line, @@ -454,7 +491,7 @@ function parseLogLine(line: string): { source: string; content: string; isError: const prProgressMatch = line.match(/^\[PR #\d+\]\s*\[\s*(\d+)%\]\s*(.*)$/); if (prProgressMatch) { return { - source: 'Progress', + source: "Progress", content: `[${prProgressMatch[1]}%] ${prProgressMatch[2]}`, isError: false, }; @@ -464,7 +501,7 @@ function parseLogLine(line: string): { source: string; content: string; isError: const progressMatch = line.match(/^\[(\d+)%\]\s*(.*)$/); if (progressMatch) { return { - source: 'Progress', + source: "Progress", content: `[${progressMatch[1]}%] ${progressMatch[2]}`, isError: false, }; @@ -493,7 +530,7 @@ function parseLogLine(line: string): { source: string; content: string; isError: const match = line.match(pattern); if (match) { return { - source: 'Summary', + source: "Summary", content: line, isError: false, }; @@ -507,16 +544,23 @@ function parseLogLine(line: string): { source: string; content: string; isError: * Determine the phase from source */ function getPhaseFromSource(source: string): PRLogPhase { - const contextSources = ['Context', 'BotDetector']; - const analysisSources = ['AI', 'Orchestrator', 'ParallelOrchestrator', 'ParallelFollowup', 'Followup', 'orchestrator']; - const synthesisSources = ['PR Review Engine', 'Summary', 'Progress']; + const contextSources = ["Context", "BotDetector"]; + const analysisSources = [ + "AI", + "Orchestrator", + "ParallelOrchestrator", + "ParallelFollowup", + "Followup", + "orchestrator", + ]; + const synthesisSources = ["PR Review Engine", "Summary", "Progress"]; - if (contextSources.includes(source)) return 'context'; - if (analysisSources.includes(source)) return 'analysis'; + if (contextSources.includes(source)) return "context"; + if (analysisSources.includes(source)) return "analysis"; // Specialist agents (Agent:xxx) are part of analysis phase - if (source.startsWith('Agent:')) return 'analysis'; - if (synthesisSources.includes(source)) return 'synthesis'; - return 'synthesis'; // Default to synthesis for unknown sources + if (source.startsWith("Agent:")) return "analysis"; + if (synthesisSources.includes(source)) return "synthesis"; + return "synthesis"; // Default to synthesis for unknown sources } /** @@ -526,7 +570,7 @@ function createEmptyPRLogs(prNumber: number, repo: string, isFollowup: boolean): const now = new Date().toISOString(); const createEmptyPhase = (phase: PRLogPhase): PRPhaseLog => ({ phase, - status: 'pending', + status: "pending", started_at: null, completed_at: null, entries: [], @@ -539,9 +583,9 @@ function createEmptyPRLogs(prNumber: number, repo: string, isFollowup: boolean): updated_at: now, is_followup: isFollowup, phases: { - context: createEmptyPhase('context'), - analysis: createEmptyPhase('analysis'), - synthesis: createEmptyPhase('synthesis'), + context: createEmptyPhase("context"), + analysis: createEmptyPhase("analysis"), + synthesis: createEmptyPhase("synthesis"), }, }; } @@ -550,7 +594,7 @@ function createEmptyPRLogs(prNumber: number, repo: string, isFollowup: boolean): * Get PR logs file path */ function getPRLogsPath(project: Project, prNumber: number): string { - return path.join(getGitHubDir(project), 'pr', `logs_${prNumber}.json`); + return path.join(getGitHubDir(project), "pr", `logs_${prNumber}.json`); } /** @@ -560,7 +604,7 @@ function loadPRLogs(project: Project, prNumber: number): PRLogs | null { const logsPath = getPRLogsPath(project, prNumber); try { - const rawData = fs.readFileSync(logsPath, 'utf-8'); + const rawData = fs.readFileSync(logsPath, "utf-8"); const sanitizedData = sanitizeNetworkData(rawData); return JSON.parse(sanitizedData) as PRLogs; } catch { @@ -580,7 +624,7 @@ function savePRLogs(project: Project, logs: PRLogs): void { } logs.updated_at = new Date().toISOString(); - fs.writeFileSync(logsPath, JSON.stringify(logs, null, 2), 'utf-8'); + fs.writeFileSync(logsPath, JSON.stringify(logs, null, 2), "utf-8"); } /** @@ -592,8 +636,8 @@ function addLogEntry(logs: PRLogs, entry: PRLogEntry): boolean { let statusChanged = false; // Start the phase if it was pending - if (phase.status === 'pending') { - phase.status = 'active'; + if (phase.status === "pending") { + phase.status = "active"; phase.started_at = entry.timestamp; statusChanged = true; } @@ -609,7 +653,7 @@ function addLogEntry(logs: PRLogs, entry: PRLogEntry): boolean { class PRLogCollector { private logs: PRLogs; private project: Project; - private currentPhase: PRLogPhase = 'context'; + private currentPhase: PRLogPhase = "context"; private entryCount: number = 0; private saveInterval: number = 3; // Save every N entries for real-time streaming @@ -631,14 +675,14 @@ class PRLogCollector { // When moving to a new phase, mark the previous phase as complete // Only mark complete if the phase was actually active (received log entries) // This prevents marking phases as "completed" if they were skipped - if (this.currentPhase === 'context' && (phase === 'analysis' || phase === 'synthesis')) { - if (this.logs.phases.context.status === 'active') { - this.markPhaseComplete('context', true); + if (this.currentPhase === "context" && (phase === "analysis" || phase === "synthesis")) { + if (this.logs.phases.context.status === "active") { + this.markPhaseComplete("context", true); } } - if (this.currentPhase === 'analysis' && phase === 'synthesis') { - if (this.logs.phases.analysis.status === 'active') { - this.markPhaseComplete('analysis', true); + if (this.currentPhase === "analysis" && phase === "synthesis") { + if (this.logs.phases.analysis.status === "active") { + this.markPhaseComplete("analysis", true); } } this.currentPhase = phase; @@ -646,7 +690,7 @@ class PRLogCollector { const entry: PRLogEntry = { timestamp: new Date().toISOString(), - type: parsed.isError ? 'error' : 'text', + type: parsed.isError ? "error" : "text", content: parsed.content, phase, source: parsed.source, @@ -664,7 +708,7 @@ class PRLogCollector { markPhaseComplete(phase: PRLogPhase, success: boolean): void { const phaseLog = this.logs.phases[phase]; - phaseLog.status = success ? 'completed' : 'failed'; + phaseLog.status = success ? "completed" : "failed"; phaseLog.completed_at = new Date().toISOString(); // Save immediately so frontend sees the status change this.save(); @@ -677,9 +721,9 @@ class PRLogCollector { finalize(success: boolean): void { // Mark active phases as completed based on success status // Pending phases with no entries should stay pending (they never ran) - for (const phase of ['context', 'analysis', 'synthesis'] as PRLogPhase[]) { + for (const phase of ["context", "analysis", "synthesis"] as PRLogPhase[]) { const phaseLog = this.logs.phases[phase]; - if (phaseLog.status === 'active') { + if (phaseLog.status === "active") { this.markPhaseComplete(phase, success); } // Note: Pending phases stay pending - they never received any log entries @@ -693,30 +737,31 @@ class PRLogCollector { * Get saved PR review result */ function getReviewResult(project: Project, prNumber: number): PRReviewResult | null { - const reviewPath = path.join(getGitHubDir(project), 'pr', `review_${prNumber}.json`); + const reviewPath = path.join(getGitHubDir(project), "pr", `review_${prNumber}.json`); try { - const rawData = fs.readFileSync(reviewPath, 'utf-8'); + const rawData = fs.readFileSync(reviewPath, "utf-8"); const sanitizedData = sanitizeNetworkData(rawData); const data = JSON.parse(sanitizedData); return { prNumber: data.pr_number, repo: data.repo, success: data.success, - findings: data.findings?.map((f: Record) => ({ - id: f.id, - severity: f.severity, - category: f.category, - title: f.title, - description: f.description, - file: f.file, - line: f.line, - endLine: f.end_line, - suggestedFix: f.suggested_fix, - fixable: f.fixable ?? false, - })) ?? [], - summary: data.summary ?? '', - overallStatus: data.overall_status ?? 'comment', + findings: + data.findings?.map((f: Record) => ({ + id: f.id, + severity: f.severity, + category: f.category, + title: f.title, + description: f.description, + file: f.file, + line: f.line, + endLine: f.end_line, + suggestedFix: f.suggested_fix, + fixable: f.fixable ?? false, + })) ?? [], + summary: data.summary ?? "", + overallStatus: data.overall_status ?? "comment", reviewId: data.review_id, reviewedAt: data.reviewed_at ?? new Date().toISOString(), error: data.error, @@ -756,9 +801,9 @@ function getGitHubPRSettings(): { model: string; thinkingLevel: string } { const thinkingLevel = featureThinking.githubPrs ?? DEFAULT_FEATURE_THINKING.githubPrs; // Convert model short name to full model ID - const model = MODEL_ID_MAP[modelShort] ?? MODEL_ID_MAP['opus']; + const model = MODEL_ID_MAP[modelShort] ?? MODEL_ID_MAP["opus"]; - debugLog('GitHub PR settings', { modelShort, model, thinkingLevel }); + debugLog("GitHub PR settings", { modelShort, model, thinkingLevel }); return { model, thinkingLevel }; } @@ -796,22 +841,20 @@ async function runPRReview( const args = buildRunnerArgs( getRunnerPath(backendPath), project.path, - 'review-pr', + "review-pr", [prNumber.toString()], { model, thinkingLevel } ); - debugLog('Spawning PR review process', { args, model, thinkingLevel }); + debugLog("Spawning PR review process", { args, model, thinkingLevel }); // Create log collector for this review const config = getGitHubConfig(project); - const repo = config?.repo || project.name || 'unknown'; + const repo = config?.repo || project.name || "unknown"; const logCollector = new PRLogCollector(project, prNumber, repo, false); // Build environment with project settings - const subprocessEnv = await getRunnerEnv( - getClaudeMdEnv(project) - ); + const subprocessEnv = await getRunnerEnv(getClaudeMdEnv(project)); const { process: childProcess, promise } = runPythonSubprocess({ pythonPath: getPythonPath(backendPath), @@ -819,27 +862,27 @@ async function runPRReview( cwd: backendPath, env: subprocessEnv, onProgress: (percent, message) => { - debugLog('Progress update', { percent, message }); + debugLog("Progress update", { percent, message }); sendProgress({ - phase: 'analyzing', + phase: "analyzing", prNumber, progress: percent, message, }); }, onStdout: (line) => { - debugLog('STDOUT:', line); + debugLog("STDOUT:", line); // Collect log entries logCollector.processLine(line); }, - onStderr: (line) => debugLog('STDERR:', line), + onStderr: (line) => debugLog("STDERR:", line), onComplete: () => { // Load the result from disk const reviewResult = getReviewResult(project, prNumber); if (!reviewResult) { - throw new Error('Review completed but result not found'); + throw new Error("Review completed but result not found"); } - debugLog('Review result loaded', { findingsCount: reviewResult.findings.length }); + debugLog("Review result loaded", { findingsCount: reviewResult.findings.length }); return reviewResult; }, }); @@ -847,7 +890,7 @@ async function runPRReview( // Register the running process const reviewKey = getReviewKey(project.id, prNumber); runningReviews.set(reviewKey, childProcess); - debugLog('Registered review process', { reviewKey, pid: childProcess.pid }); + debugLog("Registered review process", { reviewKey, pid: childProcess.pid }); try { // Wait for the process to complete @@ -856,51 +899,49 @@ async function runPRReview( if (!result.success) { // Finalize logs with failure logCollector.finalize(false); - throw new Error(result.error ?? 'Review failed'); + throw new Error(result.error ?? "Review failed"); } // Finalize logs with success logCollector.finalize(true); // Save PR review insights to memory (async, non-blocking) - savePRReviewToMemory(result.data!, repo, false).catch(err => { - debugLog('Failed to save PR review to memory', { error: err.message }); + savePRReviewToMemory(result.data!, repo, false).catch((err) => { + debugLog("Failed to save PR review to memory", { error: err.message }); }); return result.data!; } finally { // Clean up the registry when done (success or error) runningReviews.delete(reviewKey); - debugLog('Unregistered review process', { reviewKey }); + debugLog("Unregistered review process", { reviewKey }); } } /** * Register PR-related handlers */ -export function registerPRHandlers( - getMainWindow: () => BrowserWindow | null -): void { - debugLog('Registering PR handlers'); +export function registerPRHandlers(getMainWindow: () => BrowserWindow | null): void { + debugLog("Registering PR handlers"); // List open PRs with pagination support ipcMain.handle( IPC_CHANNELS.GITHUB_PR_LIST, async (_, projectId: string, page: number = 1): Promise => { - debugLog('listPRs handler called', { projectId, page }); + debugLog("listPRs handler called", { projectId, page }); const result = await withProjectOrNull(projectId, async (project) => { const config = getGitHubConfig(project); if (!config) { - debugLog('No GitHub config found for project'); + debugLog("No GitHub config found for project"); return []; } try { // Use pagination: per_page=100 (GitHub max), page=1,2,3... - const prs = await githubFetch( + const prs = (await githubFetch( config.token, `/repos/${config.repo}/pulls?state=open&per_page=100&page=${page}` - ) as Array<{ + )) as Array<{ number: number; title: string; body?: string; @@ -917,18 +958,18 @@ export function registerPRHandlers( html_url: string; }>; - debugLog('Fetched PRs', { count: prs.length, page }); - return prs.map(pr => ({ + debugLog("Fetched PRs", { count: prs.length, page, samplePr: prs[0] }); + return prs.map((pr) => ({ number: pr.number, title: pr.title, - body: pr.body ?? '', + body: pr.body ?? "", state: pr.state, author: { login: pr.user.login }, headRefName: pr.head.ref, baseRefName: pr.base.ref, - additions: pr.additions, - deletions: pr.deletions, - changedFiles: pr.changed_files, + additions: pr.additions ?? 0, + deletions: pr.deletions ?? 0, + changedFiles: pr.changed_files ?? 0, assignees: pr.assignees?.map((a: { login: string }) => ({ login: a.login })) ?? [], files: [], createdAt: pr.created_at, @@ -936,7 +977,9 @@ export function registerPRHandlers( htmlUrl: pr.html_url, })); } catch (error) { - debugLog('Failed to fetch PRs', { error: error instanceof Error ? error.message : error }); + debugLog("Failed to fetch PRs", { + error: error instanceof Error ? error.message : error, + }); return []; } }); @@ -948,16 +991,16 @@ export function registerPRHandlers( ipcMain.handle( IPC_CHANNELS.GITHUB_PR_GET, async (_, projectId: string, prNumber: number): Promise => { - debugLog('getPR handler called', { projectId, prNumber }); + debugLog("getPR handler called", { projectId, prNumber }); return withProjectOrNull(projectId, async (project) => { const config = getGitHubConfig(project); if (!config) return null; try { - const pr = await githubFetch( + const pr = (await githubFetch( config.token, `/repos/${config.repo}/pulls/${prNumber}` - ) as { + )) as { number: number; title: string; body?: string; @@ -974,10 +1017,10 @@ export function registerPRHandlers( html_url: string; }; - const files = await githubFetch( + const files = (await githubFetch( config.token, `/repos/${config.repo}/pulls/${prNumber}/files` - ) as Array<{ + )) as Array<{ filename: string; additions: number; deletions: number; @@ -987,19 +1030,19 @@ export function registerPRHandlers( return { number: pr.number, title: pr.title, - body: pr.body ?? '', + body: pr.body ?? "", state: pr.state, author: { login: pr.user.login }, headRefName: pr.head.ref, baseRefName: pr.base.ref, - additions: pr.additions, - deletions: pr.deletions, - changedFiles: pr.changed_files, + additions: pr.additions ?? 0, + deletions: pr.deletions ?? 0, + changedFiles: pr.changed_files ?? 0, assignees: pr.assignees?.map((a: { login: string }) => ({ login: a.login })) ?? [], - files: files.map(f => ({ + files: files.map((f) => ({ path: f.filename, - additions: f.additions, - deletions: f.deletions, + additions: f.additions ?? 0, + deletions: f.deletions ?? 0, status: f.status, })), createdAt: pr.created_at, @@ -1022,15 +1065,15 @@ export function registerPRHandlers( if (!config) return null; try { - const { execFileSync } = await import('child_process'); + const { execFileSync } = await import("child_process"); // Validate prNumber to prevent command injection if (!Number.isInteger(prNumber) || prNumber <= 0) { - throw new Error('Invalid PR number'); + throw new Error("Invalid PR number"); } // Use execFileSync with arguments array to prevent command injection - const diff = execFileSync('gh', ['pr', 'diff', String(prNumber)], { + const diff = execFileSync("gh", ["pr", "diff", String(prNumber)], { cwd: project.path, - encoding: 'utf-8', + encoding: "utf-8", env: getAugmentedEnv(), }); return diff; @@ -1054,14 +1097,20 @@ export function registerPRHandlers( // Batch get saved reviews - more efficient than individual calls ipcMain.handle( IPC_CHANNELS.GITHUB_PR_GET_REVIEWS_BATCH, - async (_, projectId: string, prNumbers: number[]): Promise> => { - debugLog('getReviewsBatch handler called', { projectId, count: prNumbers.length }); + async ( + _, + projectId: string, + prNumbers: number[] + ): Promise> => { + debugLog("getReviewsBatch handler called", { projectId, count: prNumbers.length }); const result = await withProjectOrNull(projectId, async (project) => { const reviews: Record = {}; for (const prNumber of prNumbers) { reviews[prNumber] = getReviewResult(project, prNumber); } - debugLog('Batch loaded reviews', { count: Object.values(reviews).filter(r => r !== null).length }); + debugLog("Batch loaded reviews", { + count: Object.values(reviews).filter((r) => r !== null).length, + }); return reviews; }); return result ?? {}; @@ -1079,82 +1128,20 @@ export function registerPRHandlers( ); // Run AI review - ipcMain.on( - IPC_CHANNELS.GITHUB_PR_REVIEW, - async (_, projectId: string, prNumber: number) => { - debugLog('runPRReview handler called', { projectId, prNumber }); - const mainWindow = getMainWindow(); - if (!mainWindow) { - debugLog('No main window available'); - return; - } + ipcMain.on(IPC_CHANNELS.GITHUB_PR_REVIEW, async (_, projectId: string, prNumber: number) => { + debugLog("runPRReview handler called", { projectId, prNumber }); + const mainWindow = getMainWindow(); + if (!mainWindow) { + debugLog("No main window available"); + return; + } - try { - await withProjectOrNull(projectId, async (project) => { - const { sendProgress, sendComplete } = createIPCCommunicators( - mainWindow, - { - progress: IPC_CHANNELS.GITHUB_PR_REVIEW_PROGRESS, - error: IPC_CHANNELS.GITHUB_PR_REVIEW_ERROR, - complete: IPC_CHANNELS.GITHUB_PR_REVIEW_COMPLETE, - }, - projectId - ); - - debugLog('Starting PR review', { prNumber }); - sendProgress({ - phase: 'fetching', - prNumber, - progress: 5, - message: 'Assigning you to PR...', - }); - - // Auto-assign current user to PR - const config = getGitHubConfig(project); - if (config) { - try { - // Get current user - const user = await githubFetch(config.token, '/user') as { login: string }; - debugLog('Auto-assigning user to PR', { prNumber, username: user.login }); - - // Assign to PR - await githubFetch( - config.token, - `/repos/${config.repo}/issues/${prNumber}/assignees`, - { - method: 'POST', - body: JSON.stringify({ assignees: [user.login] }), - } - ); - debugLog('User assigned successfully', { prNumber, username: user.login }); - } catch (assignError) { - // Don't fail the review if assignment fails, just log it - debugLog('Failed to auto-assign user', { prNumber, error: assignError instanceof Error ? assignError.message : assignError }); - } - } - - sendProgress({ - phase: 'fetching', - prNumber, - progress: 10, - message: 'Fetching PR data...', - }); - - const result = await runPRReview(project, prNumber, mainWindow); - - debugLog('PR review completed', { prNumber, findingsCount: result.findings.length }); - sendProgress({ - phase: 'complete', - prNumber, - progress: 100, - message: 'Review complete!', - }); - - sendComplete(result); - }); - } catch (error) { - debugLog('PR review failed', { prNumber, error: error instanceof Error ? error.message : error }); - const { sendError } = createIPCCommunicators( + try { + await withProjectOrNull(projectId, async (project) => { + const { sendProgress, sendComplete } = createIPCCommunicators< + PRReviewProgress, + PRReviewResult + >( mainWindow, { progress: IPC_CHANNELS.GITHUB_PR_REVIEW_PROGRESS, @@ -1163,26 +1150,101 @@ export function registerPRHandlers( }, projectId ); - sendError(error instanceof Error ? error.message : 'Failed to run PR review'); - } + + debugLog("Starting PR review", { prNumber }); + sendProgress({ + phase: "fetching", + prNumber, + progress: 5, + message: "Assigning you to PR...", + }); + + // Auto-assign current user to PR + const config = getGitHubConfig(project); + if (config) { + try { + // Get current user + const user = (await githubFetch(config.token, "/user")) as { login: string }; + debugLog("Auto-assigning user to PR", { prNumber, username: user.login }); + + // Assign to PR + await githubFetch(config.token, `/repos/${config.repo}/issues/${prNumber}/assignees`, { + method: "POST", + body: JSON.stringify({ assignees: [user.login] }), + }); + debugLog("User assigned successfully", { prNumber, username: user.login }); + } catch (assignError) { + // Don't fail the review if assignment fails, just log it + debugLog("Failed to auto-assign user", { + prNumber, + error: assignError instanceof Error ? assignError.message : assignError, + }); + } + } + + sendProgress({ + phase: "fetching", + prNumber, + progress: 10, + message: "Fetching PR data...", + }); + + const result = await runPRReview(project, prNumber, mainWindow); + + debugLog("PR review completed", { prNumber, findingsCount: result.findings.length }); + sendProgress({ + phase: "complete", + prNumber, + progress: 100, + message: "Review complete!", + }); + + sendComplete(result); + }); + } catch (error) { + debugLog("PR review failed", { + prNumber, + error: error instanceof Error ? error.message : error, + }); + const { sendError } = createIPCCommunicators( + mainWindow, + { + progress: IPC_CHANNELS.GITHUB_PR_REVIEW_PROGRESS, + error: IPC_CHANNELS.GITHUB_PR_REVIEW_ERROR, + complete: IPC_CHANNELS.GITHUB_PR_REVIEW_COMPLETE, + }, + projectId + ); + sendError(error instanceof Error ? error.message : "Failed to run PR review"); } - ); + }); // Post review to GitHub ipcMain.handle( IPC_CHANNELS.GITHUB_PR_POST_REVIEW, - async (_, projectId: string, prNumber: number, selectedFindingIds?: string[], options?: { forceApprove?: boolean }): Promise => { - debugLog('postPRReview handler called', { projectId, prNumber, selectedCount: selectedFindingIds?.length, forceApprove: options?.forceApprove }); + async ( + _, + projectId: string, + prNumber: number, + selectedFindingIds?: string[], + options?: { forceApprove?: boolean } + ): Promise => { + debugLog("postPRReview handler called", { + projectId, + prNumber, + selectedCount: selectedFindingIds?.length, + forceApprove: options?.forceApprove, + }); const postResult = await withProjectOrNull(projectId, async (project) => { const result = getReviewResult(project, prNumber); if (!result) { - debugLog('No review result found', { prNumber }); + debugLog("No review result found", { prNumber }); return false; } const config = getGitHubConfig(project); if (!config) { - debugLog('No GitHub config found'); + debugLog("No GitHub config found"); return false; } @@ -1190,10 +1252,13 @@ export function registerPRHandlers( // Filter findings if selection provided const selectedSet = selectedFindingIds ? new Set(selectedFindingIds) : null; const findings = selectedSet - ? result.findings.filter(f => selectedSet.has(f.id)) + ? result.findings.filter((f) => selectedSet.has(f.id)) : result.findings; - debugLog('Posting findings', { total: result.findings.length, selected: findings.length }); + debugLog("Posting findings", { + total: result.findings.length, + selected: findings.length, + }); // Build review body - different format for auto-approve with suggestions let body: string; @@ -1210,7 +1275,8 @@ export function registerPRHandlers( body += `*These are non-blocking suggestions for consideration:*\n\n`; for (const f of findings) { - const emoji = { critical: '🔴', high: '🟠', medium: '🟡', low: '🔵' }[f.severity] || '⚪'; + const emoji = + { critical: "🔴", high: "🟠", medium: "🟡", low: "🔵" }[f.severity] || "⚪"; body += `#### ${emoji} [${f.id}] [${f.severity.toUpperCase()}] ${f.title}\n`; body += `📁 \`${f.file}:${f.line}\`\n\n`; body += `${f.description}\n\n`; @@ -1235,7 +1301,8 @@ export function registerPRHandlers( body += `### Findings (${countText})\n\n`; for (const f of findings) { - const emoji = { critical: '🔴', high: '🟠', medium: '🟡', low: '🔵' }[f.severity] || '⚪'; + const emoji = + { critical: "🔴", high: "🟠", medium: "🟡", low: "🔵" }[f.severity] || "⚪"; body += `#### ${emoji} [${f.id}] [${f.severity.toUpperCase()}] ${f.title}\n`; body += `📁 \`${f.file}:${f.line}\`\n\n`; body += `${f.description}\n\n`; @@ -1256,82 +1323,108 @@ export function registerPRHandlers( let overallStatus = result.overallStatus; if (options?.forceApprove) { // Force approve regardless of findings - overallStatus = 'approve'; + overallStatus = "approve"; } else if (selectedSet) { - const hasBlocker = findings.some(f => f.severity === 'critical' || f.severity === 'high'); - overallStatus = hasBlocker ? 'request_changes' : (findings.length > 0 ? 'comment' : 'approve'); + const hasBlocker = findings.some( + (f) => f.severity === "critical" || f.severity === "high" + ); + overallStatus = hasBlocker + ? "request_changes" + : findings.length > 0 + ? "comment" + : "approve"; } // Map to GitHub API event type - const event = overallStatus === 'approve' ? 'APPROVE' : - overallStatus === 'request_changes' ? 'REQUEST_CHANGES' : 'COMMENT'; + const event = + overallStatus === "approve" + ? "APPROVE" + : overallStatus === "request_changes" + ? "REQUEST_CHANGES" + : "COMMENT"; - debugLog('Posting review to GitHub', { prNumber, status: overallStatus, event, findingsCount: findings.length }); + debugLog("Posting review to GitHub", { + prNumber, + status: overallStatus, + event, + findingsCount: findings.length, + }); // Post review via GitHub API to capture review ID let reviewId: number; try { - const reviewResponse = await githubFetch( + const reviewResponse = (await githubFetch( config.token, `/repos/${config.repo}/pulls/${prNumber}/reviews`, { - method: 'POST', + method: "POST", body: JSON.stringify({ body, event, }), } - ) as { id: number }; + )) as { id: number }; reviewId = reviewResponse.id; } catch (error) { // GitHub doesn't allow REQUEST_CHANGES or APPROVE on your own PR // Fall back to COMMENT if that's the error const errorMsg = error instanceof Error ? error.message : String(error); - if (errorMsg.includes('Can not request changes on your own pull request') || - errorMsg.includes('Can not approve your own pull request')) { - debugLog('Cannot use REQUEST_CHANGES/APPROVE on own PR, falling back to COMMENT', { prNumber }); - const fallbackResponse = await githubFetch( + if ( + errorMsg.includes("Can not request changes on your own pull request") || + errorMsg.includes("Can not approve your own pull request") + ) { + debugLog("Cannot use REQUEST_CHANGES/APPROVE on own PR, falling back to COMMENT", { + prNumber, + }); + const fallbackResponse = (await githubFetch( config.token, `/repos/${config.repo}/pulls/${prNumber}/reviews`, { - method: 'POST', + method: "POST", body: JSON.stringify({ body, - event: 'COMMENT', + event: "COMMENT", }), } - ) as { id: number }; + )) as { id: number }; reviewId = fallbackResponse.id; } else { throw error; } } - debugLog('Review posted successfully', { prNumber, reviewId }); + debugLog("Review posted successfully", { prNumber, reviewId }); // Update the stored review result with the review ID and posted findings - const reviewPath = path.join(getGitHubDir(project), 'pr', `review_${prNumber}.json`); + const reviewPath = path.join(getGitHubDir(project), "pr", `review_${prNumber}.json`); try { - const rawData = fs.readFileSync(reviewPath, 'utf-8'); + const rawData = fs.readFileSync(reviewPath, "utf-8"); // Sanitize network data before parsing (review may contain data from GitHub API) const sanitizedData = sanitizeNetworkData(rawData); const data = JSON.parse(sanitizedData); data.review_id = reviewId; // Track posted findings to enable follow-up review data.has_posted_findings = true; - const newPostedIds = findings.map(f => f.id); + const newPostedIds = findings.map((f) => f.id); const existingPostedIds = data.posted_finding_ids || []; data.posted_finding_ids = [...new Set([...existingPostedIds, ...newPostedIds])]; data.posted_at = new Date().toISOString(); - fs.writeFileSync(reviewPath, JSON.stringify(data, null, 2), 'utf-8'); - debugLog('Updated review result with review ID and posted findings', { prNumber, reviewId, postedCount: newPostedIds.length }); + fs.writeFileSync(reviewPath, JSON.stringify(data, null, 2), "utf-8"); + debugLog("Updated review result with review ID and posted findings", { + prNumber, + reviewId, + postedCount: newPostedIds.length, + }); } catch { // File doesn't exist or couldn't be read - this is expected for new reviews - debugLog('Review result file not found or unreadable, skipping update', { prNumber }); + debugLog("Review result file not found or unreadable, skipping update", { prNumber }); } return true; } catch (error) { - debugLog('Failed to post review', { prNumber, error: error instanceof Error ? error.message : error }); + debugLog("Failed to post review", { + prNumber, + error: error instanceof Error ? error.message : error, + }); return false; } }); @@ -1343,41 +1436,46 @@ export function registerPRHandlers( ipcMain.handle( IPC_CHANNELS.GITHUB_PR_POST_COMMENT, async (_, projectId: string, prNumber: number, body: string): Promise => { - debugLog('postPRComment handler called', { projectId, prNumber }); + debugLog("postPRComment handler called", { projectId, prNumber }); const postResult = await withProjectOrNull(projectId, async (project) => { try { - const { execFileSync } = await import('child_process'); - const { writeFileSync, unlinkSync } = await import('fs'); - const { join } = await import('path'); + const { execFileSync } = await import("child_process"); + const { writeFileSync, unlinkSync } = await import("fs"); + const { join } = await import("path"); - debugLog('Posting comment to PR', { prNumber }); + debugLog("Posting comment to PR", { prNumber }); // Validate prNumber to prevent command injection if (!Number.isInteger(prNumber) || prNumber <= 0) { - throw new Error('Invalid PR number'); + throw new Error("Invalid PR number"); } // Use temp file to avoid shell escaping issues - const tmpFile = join(project.path, '.auto-claude', 'tmp_comment_body.txt'); + const tmpFile = join(project.path, ".auto-claude", "tmp_comment_body.txt"); try { - writeFileSync(tmpFile, body, 'utf-8'); + writeFileSync(tmpFile, body, "utf-8"); // Use execFileSync with arguments array to prevent command injection - execFileSync('gh', ['pr', 'comment', String(prNumber), '--body-file', tmpFile], { + execFileSync("gh", ["pr", "comment", String(prNumber), "--body-file", tmpFile], { cwd: project.path, env: getAugmentedEnv(), }); unlinkSync(tmpFile); } catch (error) { - try { unlinkSync(tmpFile); } catch { + try { + unlinkSync(tmpFile); + } catch { // Ignore cleanup errors } throw error; } - debugLog('Comment posted successfully', { prNumber }); + debugLog("Comment posted successfully", { prNumber }); return true; } catch (error) { - debugLog('Failed to post comment', { prNumber, error: error instanceof Error ? error.message : error }); + debugLog("Failed to post comment", { + prNumber, + error: error instanceof Error ? error.message : error, + }); return false; } }); @@ -1389,51 +1487,54 @@ export function registerPRHandlers( ipcMain.handle( IPC_CHANNELS.GITHUB_PR_DELETE_REVIEW, async (_, projectId: string, prNumber: number): Promise => { - debugLog('deletePRReview handler called', { projectId, prNumber }); + debugLog("deletePRReview handler called", { projectId, prNumber }); const deleteResult = await withProjectOrNull(projectId, async (project) => { const result = getReviewResult(project, prNumber); if (!result || !result.reviewId) { - debugLog('No review ID found for deletion', { prNumber }); + debugLog("No review ID found for deletion", { prNumber }); return false; } const config = getGitHubConfig(project); if (!config) { - debugLog('No GitHub config found'); + debugLog("No GitHub config found"); return false; } try { - debugLog('Deleting review from GitHub', { prNumber, reviewId: result.reviewId }); + debugLog("Deleting review from GitHub", { prNumber, reviewId: result.reviewId }); // Delete review via GitHub API await githubFetch( config.token, `/repos/${config.repo}/pulls/${prNumber}/reviews/${result.reviewId}`, { - method: 'DELETE', + method: "DELETE", } ); - debugLog('Review deleted successfully', { prNumber, reviewId: result.reviewId }); + debugLog("Review deleted successfully", { prNumber, reviewId: result.reviewId }); // Clear the review ID from the stored result - const reviewPath = path.join(getGitHubDir(project), 'pr', `review_${prNumber}.json`); + const reviewPath = path.join(getGitHubDir(project), "pr", `review_${prNumber}.json`); try { - const rawData = fs.readFileSync(reviewPath, 'utf-8'); + const rawData = fs.readFileSync(reviewPath, "utf-8"); const sanitizedData = sanitizeNetworkData(rawData); const data = JSON.parse(sanitizedData); delete data.review_id; - fs.writeFileSync(reviewPath, JSON.stringify(data, null, 2), 'utf-8'); - debugLog('Cleared review ID from result file', { prNumber }); + fs.writeFileSync(reviewPath, JSON.stringify(data, null, 2), "utf-8"); + debugLog("Cleared review ID from result file", { prNumber }); } catch { // File doesn't exist or couldn't be read - this is expected if review wasn't saved - debugLog('Review result file not found or unreadable, skipping update', { prNumber }); + debugLog("Review result file not found or unreadable, skipping update", { prNumber }); } return true; } catch (error) { - debugLog('Failed to delete review', { prNumber, error: error instanceof Error ? error.message : error }); + debugLog("Failed to delete review", { + prNumber, + error: error instanceof Error ? error.message : error, + }); return false; } }); @@ -1444,33 +1545,41 @@ export function registerPRHandlers( // Merge PR ipcMain.handle( IPC_CHANNELS.GITHUB_PR_MERGE, - async (_, projectId: string, prNumber: number, mergeMethod: 'merge' | 'squash' | 'rebase' = 'squash'): Promise => { - debugLog('mergePR handler called', { projectId, prNumber, mergeMethod }); + async ( + _, + projectId: string, + prNumber: number, + mergeMethod: "merge" | "squash" | "rebase" = "squash" + ): Promise => { + debugLog("mergePR handler called", { projectId, prNumber, mergeMethod }); const mergeResult = await withProjectOrNull(projectId, async (project) => { try { - const { execFileSync } = await import('child_process'); - debugLog('Merging PR', { prNumber, method: mergeMethod }); + const { execFileSync } = await import("child_process"); + debugLog("Merging PR", { prNumber, method: mergeMethod }); // Validate prNumber to prevent command injection if (!Number.isInteger(prNumber) || prNumber <= 0) { - throw new Error('Invalid PR number'); + throw new Error("Invalid PR number"); } // Validate mergeMethod to prevent command injection - const validMethods = ['merge', 'squash', 'rebase']; + const validMethods = ["merge", "squash", "rebase"]; if (!validMethods.includes(mergeMethod)) { - throw new Error('Invalid merge method'); + throw new Error("Invalid merge method"); } // Use execFileSync with arguments array to prevent command injection - execFileSync('gh', ['pr', 'merge', String(prNumber), `--${mergeMethod}`], { + execFileSync("gh", ["pr", "merge", String(prNumber), `--${mergeMethod}`], { cwd: project.path, env: getAugmentedEnv(), }); - debugLog('PR merged successfully', { prNumber }); + debugLog("PR merged successfully", { prNumber }); return true; } catch (error) { - debugLog('Failed to merge PR', { prNumber, error: error instanceof Error ? error.message : error }); + debugLog("Failed to merge PR", { + prNumber, + error: error instanceof Error ? error.message : error, + }); return false; } }); @@ -1482,25 +1591,25 @@ export function registerPRHandlers( ipcMain.handle( IPC_CHANNELS.GITHUB_PR_ASSIGN, async (_, projectId: string, prNumber: number, username: string): Promise => { - debugLog('assignPR handler called', { projectId, prNumber, username }); + debugLog("assignPR handler called", { projectId, prNumber, username }); const assignResult = await withProjectOrNull(projectId, async (project) => { const config = getGitHubConfig(project); if (!config) return false; try { // Use GitHub API to add assignee - await githubFetch( - config.token, - `/repos/${config.repo}/issues/${prNumber}/assignees`, - { - method: 'POST', - body: JSON.stringify({ assignees: [username] }), - } - ); - debugLog('User assigned successfully', { prNumber, username }); + await githubFetch(config.token, `/repos/${config.repo}/issues/${prNumber}/assignees`, { + method: "POST", + body: JSON.stringify({ assignees: [username] }), + }); + debugLog("User assigned successfully", { prNumber, username }); return true; } catch (error) { - debugLog('Failed to assign user', { prNumber, username, error: error instanceof Error ? error.message : error }); + debugLog("Failed to assign user", { + prNumber, + username, + error: error instanceof Error ? error.message : error, + }); return false; } }); @@ -1512,33 +1621,36 @@ export function registerPRHandlers( ipcMain.handle( IPC_CHANNELS.GITHUB_PR_REVIEW_CANCEL, async (_, projectId: string, prNumber: number): Promise => { - debugLog('cancelPRReview handler called', { projectId, prNumber }); + debugLog("cancelPRReview handler called", { projectId, prNumber }); const reviewKey = getReviewKey(projectId, prNumber); const childProcess = runningReviews.get(reviewKey); if (!childProcess) { - debugLog('No running review found to cancel', { reviewKey }); + debugLog("No running review found to cancel", { reviewKey }); return false; } try { - debugLog('Killing review process', { reviewKey, pid: childProcess.pid }); - childProcess.kill('SIGTERM'); + debugLog("Killing review process", { reviewKey, pid: childProcess.pid }); + childProcess.kill("SIGTERM"); // Give it a moment to terminate gracefully, then force kill if needed setTimeout(() => { if (!childProcess.killed) { - debugLog('Force killing review process', { reviewKey, pid: childProcess.pid }); - childProcess.kill('SIGKILL'); + debugLog("Force killing review process", { reviewKey, pid: childProcess.pid }); + childProcess.kill("SIGKILL"); } }, 1000); // Clean up the registry runningReviews.delete(reviewKey); - debugLog('Review process cancelled', { reviewKey }); + debugLog("Review process cancelled", { reviewKey }); return true; } catch (error) { - debugLog('Failed to cancel review', { reviewKey, error: error instanceof Error ? error.message : error }); + debugLog("Failed to cancel review", { + reviewKey, + error: error instanceof Error ? error.message : error, + }); return false; } } @@ -1548,16 +1660,16 @@ export function registerPRHandlers( ipcMain.handle( IPC_CHANNELS.GITHUB_PR_CHECK_NEW_COMMITS, async (_, projectId: string, prNumber: number): Promise => { - debugLog('checkNewCommits handler called', { projectId, prNumber }); + debugLog("checkNewCommits handler called", { projectId, prNumber }); const result = await withProjectOrNull(projectId, async (project) => { // Check if review exists and has reviewed_commit_sha - const githubDir = path.join(project.path, '.auto-claude', 'github'); - const reviewPath = path.join(githubDir, 'pr', `review_${prNumber}.json`); + const githubDir = path.join(project.path, ".auto-claude", "github"); + const reviewPath = path.join(githubDir, "pr", `review_${prNumber}.json`); let review: PRReviewResult; try { - const rawData = fs.readFileSync(reviewPath, 'utf-8'); + const rawData = fs.readFileSync(reviewPath, "utf-8"); const sanitizedData = sanitizeNetworkData(rawData); review = JSON.parse(sanitizedData); } catch { @@ -1565,10 +1677,10 @@ export function registerPRHandlers( return { hasNewCommits: false, newCommitCount: 0 }; } - // Convert snake_case to camelCase for the field - const reviewedCommitSha = review.reviewedCommitSha || (review as any).reviewed_commit_sha; + // Normalize snake_case to camelCase for backwards compatibility with old saved files + const reviewedCommitSha = review.reviewedCommitSha ?? (review as any).reviewed_commit_sha; if (!reviewedCommitSha) { - debugLog('No reviewedCommitSha in review', { prNumber }); + debugLog("No reviewedCommitSha in review", { prNumber }); return { hasNewCommits: false, newCommitCount: 0 }; } @@ -1587,7 +1699,10 @@ export function registerPRHandlers( )) as { head: { sha: string }; commits: number }; currentHeadSha = prData.head.sha; } catch (error) { - debugLog('Error fetching PR data', { prNumber, error: error instanceof Error ? error.message : error }); + debugLog("Error fetching PR data", { + prNumber, + error: error instanceof Error ? error.message : error, + }); return { hasNewCommits: false, newCommitCount: 0 }; } @@ -1608,7 +1723,11 @@ export function registerPRHandlers( const comparison = (await githubFetch( config.token, `/repos/${config.repo}/compare/${reviewedCommitSha}...${currentHeadSha}` - )) as { ahead_by?: number; total_commits?: number; commits?: Array<{ commit: { committer: { date: string } } }> }; + )) as { + ahead_by?: number; + total_commits?: number; + commits?: Array<{ commit: { committer: { date: string } } }>; + }; // Check if findings have been posted and if new commits are after the posting date const postedAt = review.postedAt || (review as any).posted_at; @@ -1617,14 +1736,15 @@ export function registerPRHandlers( if (postedAt && comparison.commits && comparison.commits.length > 0) { const postedAtDate = new Date(postedAt); // Check if any commit is newer than when findings were posted - hasCommitsAfterPosting = comparison.commits.some(c => { + hasCommitsAfterPosting = comparison.commits.some((c) => { const commitDate = new Date(c.commit.committer.date); return commitDate > postedAtDate; }); - debugLog('Comparing commit dates with posted_at', { + debugLog("Comparing commit dates with posted_at", { prNumber, postedAt, - latestCommitDate: comparison.commits[comparison.commits.length - 1]?.commit.committer.date, + latestCommitDate: + comparison.commits[comparison.commits.length - 1]?.commit.committer.date, hasCommitsAfterPosting, }); } else if (!postedAt) { @@ -1642,12 +1762,15 @@ export function registerPRHandlers( } catch (error) { // Comparison failed (e.g., force push made old commit unreachable) // Since we already verified SHAs differ, treat as having new commits - debugLog('Comparison failed but SHAs differ - likely force push, treating as new commits', { - prNumber, - reviewedCommitSha, - currentHeadSha, - error: error instanceof Error ? error.message : error - }); + debugLog( + "Comparison failed but SHAs differ - likely force push, treating as new commits", + { + prNumber, + reviewedCommitSha, + currentHeadSha, + error: error instanceof Error ? error.message : error, + } + ); return { hasNewCommits: true, newCommitCount: 1, // Unknown count due to force push @@ -1666,29 +1789,29 @@ export function registerPRHandlers( ipcMain.handle( IPC_CHANNELS.GITHUB_PR_CHECK_MERGE_READINESS, async (_, projectId: string, prNumber: number): Promise => { - debugLog('checkMergeReadiness handler called', { projectId, prNumber }); + debugLog("checkMergeReadiness handler called", { projectId, prNumber }); const defaultResult: MergeReadiness = { isDraft: false, - mergeable: 'UNKNOWN', + mergeable: "UNKNOWN", isBehind: false, - ciStatus: 'none', + ciStatus: "none", blockers: [], }; const result = await withProjectOrNull(projectId, async (project) => { const config = getGitHubConfig(project); if (!config) { - debugLog('No GitHub config found for checkMergeReadiness'); + debugLog("No GitHub config found for checkMergeReadiness"); return defaultResult; } try { // Fetch PR data including mergeable status - const pr = await githubFetch( + const pr = (await githubFetch( config.token, `/repos/${config.repo}/pulls/${prNumber}` - ) as { + )) as { draft: boolean; mergeable: boolean | null; mergeable_state: string; @@ -1696,85 +1819,86 @@ export function registerPRHandlers( }; // Determine mergeable status - let mergeable: MergeReadiness['mergeable'] = 'UNKNOWN'; + let mergeable: MergeReadiness["mergeable"] = "UNKNOWN"; if (pr.mergeable === true) { - mergeable = 'MERGEABLE'; - } else if (pr.mergeable === false || pr.mergeable_state === 'dirty') { - mergeable = 'CONFLICTING'; + mergeable = "MERGEABLE"; + } else if (pr.mergeable === false || pr.mergeable_state === "dirty") { + mergeable = "CONFLICTING"; } // Check if branch is behind base (out of date) // GitHub's mergeable_state can be: 'behind', 'blocked', 'clean', 'dirty', 'has_hooks', 'unknown', 'unstable' - const isBehind = pr.mergeable_state === 'behind'; + const isBehind = pr.mergeable_state === "behind"; // Fetch combined commit status for CI - let ciStatus: MergeReadiness['ciStatus'] = 'none'; + let ciStatus: MergeReadiness["ciStatus"] = "none"; try { - const status = await githubFetch( + const status = (await githubFetch( config.token, `/repos/${config.repo}/commits/${pr.head.sha}/status` - ) as { - state: 'success' | 'pending' | 'failure' | 'error'; + )) as { + state: "success" | "pending" | "failure" | "error"; total_count: number; }; if (status.total_count === 0) { // No status checks, check for check runs (GitHub Actions) - const checkRuns = await githubFetch( + const checkRuns = (await githubFetch( config.token, `/repos/${config.repo}/commits/${pr.head.sha}/check-runs` - ) as { + )) as { total_count: number; check_runs: Array<{ conclusion: string | null; status: string }>; }; if (checkRuns.total_count > 0) { const hasFailing = checkRuns.check_runs.some( - cr => cr.conclusion === 'failure' || cr.conclusion === 'cancelled' - ); - const hasPending = checkRuns.check_runs.some( - cr => cr.status !== 'completed' + (cr) => cr.conclusion === "failure" || cr.conclusion === "cancelled" ); + const hasPending = checkRuns.check_runs.some((cr) => cr.status !== "completed"); if (hasFailing) { - ciStatus = 'failing'; + ciStatus = "failing"; } else if (hasPending) { - ciStatus = 'pending'; + ciStatus = "pending"; } else { - ciStatus = 'passing'; + ciStatus = "passing"; } } } else { // Use combined status - if (status.state === 'success') { - ciStatus = 'passing'; - } else if (status.state === 'pending') { - ciStatus = 'pending'; + if (status.state === "success") { + ciStatus = "passing"; + } else if (status.state === "pending") { + ciStatus = "pending"; } else { - ciStatus = 'failing'; + ciStatus = "failing"; } } } catch (err) { - debugLog('Failed to fetch CI status', { prNumber, error: err instanceof Error ? err.message : err }); + debugLog("Failed to fetch CI status", { + prNumber, + error: err instanceof Error ? err.message : err, + }); // Continue without CI status } // Build blockers list const blockers: string[] = []; if (pr.draft) { - blockers.push('PR is in draft mode'); + blockers.push("PR is in draft mode"); } - if (mergeable === 'CONFLICTING') { - blockers.push('Merge conflicts detected'); + if (mergeable === "CONFLICTING") { + blockers.push("Merge conflicts detected"); } if (isBehind) { - blockers.push('Branch is out of date with base branch. Update to check for conflicts.'); + blockers.push("Branch is out of date with base branch. Update to check for conflicts."); } - if (ciStatus === 'failing') { - blockers.push('CI checks are failing'); + if (ciStatus === "failing") { + blockers.push("CI checks are failing"); } - debugLog('checkMergeReadiness result', { + debugLog("checkMergeReadiness result", { prNumber, isDraft: pr.draft, mergeable, @@ -1791,7 +1915,7 @@ export function registerPRHandlers( blockers, }; } catch (error) { - debugLog('Failed to check merge readiness', { + debugLog("Failed to check merge readiness", { prNumber, error: error instanceof Error ? error.message : error, }); @@ -1807,16 +1931,19 @@ export function registerPRHandlers( ipcMain.on( IPC_CHANNELS.GITHUB_PR_FOLLOWUP_REVIEW, async (_, projectId: string, prNumber: number) => { - debugLog('followupReview handler called', { projectId, prNumber }); + debugLog("followupReview handler called", { projectId, prNumber }); const mainWindow = getMainWindow(); if (!mainWindow) { - debugLog('No main window available'); + debugLog("No main window available"); return; } try { await withProjectOrNull(projectId, async (project) => { - const { sendProgress, sendError, sendComplete } = createIPCCommunicators( + const { sendProgress, sendError, sendComplete } = createIPCCommunicators< + PRReviewProgress, + PRReviewResult + >( mainWindow, { progress: IPC_CHANNELS.GITHUB_PR_REVIEW_PROGRESS, @@ -1829,7 +1956,7 @@ export function registerPRHandlers( // Comprehensive validation of GitHub module const validation = await validateGitHubModule(project); if (!validation.valid) { - sendError({ prNumber, error: validation.error || 'GitHub module validation failed' }); + sendError({ prNumber, error: validation.error || "GitHub module validation failed" }); return; } @@ -1838,38 +1965,36 @@ export function registerPRHandlers( // Check if already running if (runningReviews.has(reviewKey)) { - debugLog('Follow-up review already running', { reviewKey }); + debugLog("Follow-up review already running", { reviewKey }); return; } - debugLog('Starting follow-up review', { prNumber }); + debugLog("Starting follow-up review", { prNumber }); sendProgress({ - phase: 'fetching', + phase: "fetching", prNumber, progress: 5, - message: 'Starting follow-up review...', + message: "Starting follow-up review...", }); const { model, thinkingLevel } = getGitHubPRSettings(); const args = buildRunnerArgs( getRunnerPath(backendPath), project.path, - 'followup-review-pr', + "followup-review-pr", [prNumber.toString()], { model, thinkingLevel } ); - debugLog('Spawning follow-up review process', { args, model, thinkingLevel }); + debugLog("Spawning follow-up review process", { args, model, thinkingLevel }); // Create log collector for this follow-up review const config = getGitHubConfig(project); - const repo = config?.repo || project.name || 'unknown'; + const repo = config?.repo || project.name || "unknown"; const logCollector = new PRLogCollector(project, prNumber, repo, true); // Build environment with project settings - const followupEnv = await getRunnerEnv( - getClaudeMdEnv(project) - ); + const followupEnv = await getRunnerEnv(getClaudeMdEnv(project)); const { process: childProcess, promise } = runPythonSubprocess({ pythonPath: getPythonPath(backendPath), @@ -1877,34 +2002,36 @@ export function registerPRHandlers( cwd: backendPath, env: followupEnv, onProgress: (percent, message) => { - debugLog('Progress update', { percent, message }); + debugLog("Progress update", { percent, message }); sendProgress({ - phase: 'analyzing', + phase: "analyzing", prNumber, progress: percent, message, }); }, onStdout: (line) => { - debugLog('STDOUT:', line); + debugLog("STDOUT:", line); // Collect log entries logCollector.processLine(line); }, - onStderr: (line) => debugLog('STDERR:', line), + onStderr: (line) => debugLog("STDERR:", line), onComplete: () => { // Load the result from disk const reviewResult = getReviewResult(project, prNumber); if (!reviewResult) { - throw new Error('Follow-up review completed but result not found'); + throw new Error("Follow-up review completed but result not found"); } - debugLog('Follow-up review result loaded', { findingsCount: reviewResult.findings.length }); + debugLog("Follow-up review result loaded", { + findingsCount: reviewResult.findings.length, + }); return reviewResult; }, }); // Register the running process runningReviews.set(reviewKey, childProcess); - debugLog('Registered follow-up review process', { reviewKey, pid: childProcess.pid }); + debugLog("Registered follow-up review process", { reviewKey, pid: childProcess.pid }); try { const result = await promise; @@ -1912,33 +2039,39 @@ export function registerPRHandlers( if (!result.success) { // Finalize logs with failure logCollector.finalize(false); - throw new Error(result.error ?? 'Follow-up review failed'); + throw new Error(result.error ?? "Follow-up review failed"); } // Finalize logs with success logCollector.finalize(true); // Save follow-up PR review insights to memory (async, non-blocking) - savePRReviewToMemory(result.data!, repo, true).catch(err => { - debugLog('Failed to save follow-up PR review to memory', { error: err.message }); + savePRReviewToMemory(result.data!, repo, true).catch((err) => { + debugLog("Failed to save follow-up PR review to memory", { error: err.message }); }); - debugLog('Follow-up review completed', { prNumber, findingsCount: result.data?.findings.length }); + debugLog("Follow-up review completed", { + prNumber, + findingsCount: result.data?.findings.length, + }); sendProgress({ - phase: 'complete', + phase: "complete", prNumber, progress: 100, - message: 'Follow-up review complete!', + message: "Follow-up review complete!", }); sendComplete(result.data!); } finally { runningReviews.delete(reviewKey); - debugLog('Unregistered follow-up review process', { reviewKey }); + debugLog("Unregistered follow-up review process", { reviewKey }); } }); } catch (error) { - debugLog('Follow-up review failed', { prNumber, error: error instanceof Error ? error.message : error }); + debugLog("Follow-up review failed", { + prNumber, + error: error instanceof Error ? error.message : error, + }); const { sendError } = createIPCCommunicators( mainWindow, { @@ -1948,7 +2081,10 @@ export function registerPRHandlers( }, projectId ); - sendError({ prNumber, error: error instanceof Error ? error.message : 'Failed to run follow-up review' }); + sendError({ + prNumber, + error: error instanceof Error ? error.message : "Failed to run follow-up review", + }); } } ); @@ -1956,25 +2092,34 @@ export function registerPRHandlers( // Get workflows awaiting approval for a PR (fork PRs) ipcMain.handle( IPC_CHANNELS.GITHUB_WORKFLOWS_AWAITING_APPROVAL, - async (_, projectId: string, prNumber: number): Promise<{ + async ( + _, + projectId: string, + prNumber: number + ): Promise<{ awaiting_approval: number; workflow_runs: Array<{ id: number; name: string; html_url: string; workflow_name: string }>; can_approve: boolean; error?: string; }> => { - debugLog('getWorkflowsAwaitingApproval handler called', { projectId, prNumber }); + debugLog("getWorkflowsAwaitingApproval handler called", { projectId, prNumber }); const result = await withProjectOrNull(projectId, async (project) => { const config = getGitHubConfig(project); if (!config) { - return { awaiting_approval: 0, workflow_runs: [], can_approve: false, error: 'No GitHub config' }; + return { + awaiting_approval: 0, + workflow_runs: [], + can_approve: false, + error: "No GitHub config", + }; } try { // First get the PR's head SHA - const prData = await githubFetch( + const prData = (await githubFetch( config.token, `/repos/${config.repo}/pulls/${prNumber}` - ) as { head?: { sha?: string } }; + )) as { head?: { sha?: string } }; const headSha = prData?.head?.sha; if (!headSha) { @@ -1982,24 +2127,32 @@ export function registerPRHandlers( } // Query workflow runs with action_required status - const runsData = await githubFetch( + const runsData = (await githubFetch( config.token, `/repos/${config.repo}/actions/runs?status=action_required&per_page=100` - ) as { workflow_runs?: Array<{ id: number; name: string; html_url: string; head_sha: string; workflow?: { name?: string } }> }; + )) as { + workflow_runs?: Array<{ + id: number; + name: string; + html_url: string; + head_sha: string; + workflow?: { name?: string }; + }>; + }; const allRuns = runsData?.workflow_runs || []; // Filter to only runs for this PR's head SHA const prRuns = allRuns - .filter(run => run.head_sha === headSha) - .map(run => ({ + .filter((run) => run.head_sha === headSha) + .map((run) => ({ id: run.id, name: run.name, html_url: run.html_url, - workflow_name: run.workflow?.name || 'Unknown', + workflow_name: run.workflow?.name || "Unknown", })); - debugLog('Found workflows awaiting approval', { prNumber, count: prRuns.length }); + debugLog("Found workflows awaiting approval", { prNumber, count: prRuns.length }); return { awaiting_approval: prRuns.length, @@ -2007,12 +2160,15 @@ export function registerPRHandlers( can_approve: true, // Assume token has permission; will fail if not }; } catch (error) { - debugLog('Failed to get workflows awaiting approval', { prNumber, error: error instanceof Error ? error.message : error }); + debugLog("Failed to get workflows awaiting approval", { + prNumber, + error: error instanceof Error ? error.message : error, + }); return { awaiting_approval: 0, workflow_runs: [], can_approve: false, - error: error instanceof Error ? error.message : 'Unknown error', + error: error instanceof Error ? error.message : "Unknown error", }; } }); @@ -2025,26 +2181,27 @@ export function registerPRHandlers( ipcMain.handle( IPC_CHANNELS.GITHUB_WORKFLOW_APPROVE, async (_, projectId: string, runId: number): Promise => { - debugLog('approveWorkflow handler called', { projectId, runId }); + debugLog("approveWorkflow handler called", { projectId, runId }); const result = await withProjectOrNull(projectId, async (project) => { const config = getGitHubConfig(project); if (!config) { - debugLog('No GitHub config found'); + debugLog("No GitHub config found"); return false; } try { // Approve the workflow run - await githubFetch( - config.token, - `/repos/${config.repo}/actions/runs/${runId}/approve`, - { method: 'POST' } - ); + await githubFetch(config.token, `/repos/${config.repo}/actions/runs/${runId}/approve`, { + method: "POST", + }); - debugLog('Workflow approved successfully', { runId }); + debugLog("Workflow approved successfully", { runId }); return true; } catch (error) { - debugLog('Failed to approve workflow', { runId, error: error instanceof Error ? error.message : error }); + debugLog("Failed to approve workflow", { + runId, + error: error instanceof Error ? error.message : error, + }); return false; } }); @@ -2057,20 +2214,20 @@ export function registerPRHandlers( ipcMain.handle( IPC_CHANNELS.GITHUB_PR_MEMORY_GET, async (_, projectId: string, limit: number = 10): Promise => { - debugLog('getPRReviewMemories handler called', { projectId, limit }); + debugLog("getPRReviewMemories handler called", { projectId, limit }); const result = await withProjectOrNull(projectId, async (project) => { - const memoryDir = path.join(getGitHubDir(project), 'memory', project.name || 'unknown'); + const memoryDir = path.join(getGitHubDir(project), "memory", project.name || "unknown"); const memories: PRReviewMemory[] = []; // Try to load from file-based storage try { - const indexPath = path.join(memoryDir, 'reviews_index.json'); + const indexPath = path.join(memoryDir, "reviews_index.json"); if (!fs.existsSync(indexPath)) { - debugLog('No PR review memories found', { projectId }); + debugLog("No PR review memories found", { projectId }); return []; } - const indexContent = fs.readFileSync(indexPath, 'utf-8'); + const indexContent = fs.readFileSync(indexPath, "utf-8"); const index = JSON.parse(sanitizeNetworkData(indexContent)); const reviews = index.reviews || []; @@ -2079,12 +2236,12 @@ export function registerPRHandlers( try { const reviewPath = path.join(memoryDir, `pr_${entry.pr_number}_review.json`); if (fs.existsSync(reviewPath)) { - const reviewContent = fs.readFileSync(reviewPath, 'utf-8'); + const reviewContent = fs.readFileSync(reviewPath, "utf-8"); const memory = JSON.parse(sanitizeNetworkData(reviewContent)); memories.push({ prNumber: memory.pr_number, repo: memory.repo, - verdict: memory.summary?.verdict || 'unknown', + verdict: memory.summary?.verdict || "unknown", timestamp: memory.timestamp, summary: memory.summary, keyFindings: memory.key_findings || [], @@ -2094,14 +2251,19 @@ export function registerPRHandlers( }); } } catch (err) { - debugLog('Failed to load PR review memory', { prNumber: entry.pr_number, error: err instanceof Error ? err.message : err }); + debugLog("Failed to load PR review memory", { + prNumber: entry.pr_number, + error: err instanceof Error ? err.message : err, + }); } } - debugLog('Loaded PR review memories', { count: memories.length }); + debugLog("Loaded PR review memories", { count: memories.length }); return memories; } catch (error) { - debugLog('Failed to load PR review memories', { error: error instanceof Error ? error.message : error }); + debugLog("Failed to load PR review memories", { + error: error instanceof Error ? error.message : error, + }); return []; } }); @@ -2113,20 +2275,20 @@ export function registerPRHandlers( ipcMain.handle( IPC_CHANNELS.GITHUB_PR_MEMORY_SEARCH, async (_, projectId: string, query: string, limit: number = 10): Promise => { - debugLog('searchPRReviewMemories handler called', { projectId, query, limit }); + debugLog("searchPRReviewMemories handler called", { projectId, query, limit }); const result = await withProjectOrNull(projectId, async (project) => { - const memoryDir = path.join(getGitHubDir(project), 'memory', project.name || 'unknown'); + const memoryDir = path.join(getGitHubDir(project), "memory", project.name || "unknown"); const memories: PRReviewMemory[] = []; const queryLower = query.toLowerCase(); // Search through file-based storage try { - const indexPath = path.join(memoryDir, 'reviews_index.json'); + const indexPath = path.join(memoryDir, "reviews_index.json"); if (!fs.existsSync(indexPath)) { return []; } - const indexContent = fs.readFileSync(indexPath, 'utf-8'); + const indexContent = fs.readFileSync(indexPath, "utf-8"); const index = JSON.parse(sanitizeNetworkData(indexContent)); const reviews = index.reviews || []; @@ -2135,7 +2297,7 @@ export function registerPRHandlers( try { const reviewPath = path.join(memoryDir, `pr_${entry.pr_number}_review.json`); if (fs.existsSync(reviewPath)) { - const reviewContent = fs.readFileSync(reviewPath, 'utf-8'); + const reviewContent = fs.readFileSync(reviewPath, "utf-8"); // Check if content matches query if (reviewContent.toLowerCase().includes(queryLower)) { @@ -2143,7 +2305,7 @@ export function registerPRHandlers( memories.push({ prNumber: memory.pr_number, repo: memory.repo, - verdict: memory.summary?.verdict || 'unknown', + verdict: memory.summary?.verdict || "unknown", timestamp: memory.timestamp, summary: memory.summary, keyFindings: memory.key_findings || [], @@ -2159,14 +2321,19 @@ export function registerPRHandlers( break; } } catch (err) { - debugLog('Failed to search PR review memory', { prNumber: entry.pr_number, error: err instanceof Error ? err.message : err }); + debugLog("Failed to search PR review memory", { + prNumber: entry.pr_number, + error: err instanceof Error ? err.message : err, + }); } } - debugLog('Found matching PR review memories', { count: memories.length, query }); + debugLog("Found matching PR review memories", { count: memories.length, query }); return memories; } catch (error) { - debugLog('Failed to search PR review memories', { error: error instanceof Error ? error.message : error }); + debugLog("Failed to search PR review memories", { + error: error instanceof Error ? error.message : error, + }); return []; } }); @@ -2174,5 +2341,5 @@ export function registerPRHandlers( } ); - debugLog('PR handlers registered'); + debugLog("PR handlers registered"); } diff --git a/apps/frontend/src/renderer/components/GitHubIssues.tsx b/apps/frontend/src/renderer/components/GitHubIssues.tsx index f42c98ed..cd43ac75 100644 --- a/apps/frontend/src/renderer/components/GitHubIssues.tsx +++ b/apps/frontend/src/renderer/components/GitHubIssues.tsx @@ -1,8 +1,13 @@ -import { useState, useCallback, useMemo, useEffect } from 'react'; -import { useProjectStore } from '../stores/project-store'; -import { useTaskStore } from '../stores/task-store'; -import { useGitHubIssues, useGitHubInvestigation, useIssueFiltering, useAutoFix } from './github-issues/hooks'; -import { useAnalyzePreview } from './github-issues/hooks/useAnalyzePreview'; +import { useState, useCallback, useMemo, useEffect } from "react"; +import { useProjectStore } from "../stores/project-store"; +import { useTaskStore } from "../stores/task-store"; +import { + useGitHubIssues, + useGitHubInvestigation, + useIssueFiltering, + useAutoFix, +} from "./github-issues/hooks"; +import { useAnalyzePreview } from "./github-issues/hooks/useAnalyzePreview"; import { NotConnectedState, EmptyState, @@ -10,11 +15,11 @@ import { IssueList, IssueDetail, InvestigationDialog, - BatchReviewWizard -} from './github-issues/components'; -import { GitHubSetupModal } from './GitHubSetupModal'; -import type { GitHubIssue } from '../../shared/types'; -import type { GitHubIssuesProps } from './github-issues/types'; + BatchReviewWizard, +} from "./github-issues/components"; +import { GitHubSetupModal } from "./GitHubSetupModal"; +import type { GitHubIssue } from "../../shared/types"; +import type { GitHubIssuesProps } from "./github-issues/types"; export function GitHubIssues({ onOpenSettings, onNavigateToTask }: GitHubIssuesProps) { const projects = useProjectStore((state) => state.projects); @@ -28,19 +33,20 @@ export function GitHubIssues({ onOpenSettings, onNavigateToTask }: GitHubIssuesP isLoading, error, selectedIssueNumber, + selectedIssue, filterState, selectIssue, getFilteredIssues, getOpenIssuesCount, handleRefresh, - handleFilterChange + handleFilterChange, } = useGitHubIssues(selectedProject?.id); const { investigationStatus, lastInvestigationResult, startInvestigation, - resetInvestigationStatus + resetInvestigationStatus, } = useGitHubInvestigation(selectedProject?.id); const { searchQuery, setSearchQuery, filteredIssues } = useIssueFiltering(getFilteredIssues()); @@ -66,15 +72,16 @@ export function GitHubIssues({ onOpenSettings, onNavigateToTask }: GitHubIssuesP closeWizard, startAnalysis, approveBatches, - } = useAnalyzePreview({ projectId: selectedProject?.id || '' }); + } = useAnalyzePreview({ projectId: selectedProject?.id || "" }); const [showInvestigateDialog, setShowInvestigateDialog] = useState(false); - const [selectedIssueForInvestigation, setSelectedIssueForInvestigation] = useState(null); + const [selectedIssueForInvestigation, setSelectedIssueForInvestigation] = + useState(null); const [showGitHubSetup, setShowGitHubSetup] = useState(false); // Show GitHub setup modal when module is not installed useEffect(() => { - if (analysisError?.includes('GitHub automation module not installed')) { + if (analysisError?.includes("GitHub automation module not installed")) { setShowGitHubSetup(true); } }, [analysisError]); @@ -104,34 +111,30 @@ export function GitHubIssues({ onOpenSettings, onNavigateToTask }: GitHubIssuesP setShowInvestigateDialog(true); }, []); - const handleStartInvestigation = useCallback((selectedCommentIds: number[]) => { - if (selectedIssueForInvestigation) { - startInvestigation(selectedIssueForInvestigation, selectedCommentIds); - } - }, [selectedIssueForInvestigation, startInvestigation]); + const handleStartInvestigation = useCallback( + (selectedCommentIds: number[]) => { + if (selectedIssueForInvestigation) { + startInvestigation(selectedIssueForInvestigation, selectedCommentIds); + } + }, + [selectedIssueForInvestigation, startInvestigation] + ); const handleCloseDialog = useCallback(() => { setShowInvestigateDialog(false); resetInvestigationStatus(); }, [resetInvestigationStatus]); - const selectedIssue = issues.find(i => i.number === selectedIssueNumber); - // Not connected state if (!syncStatus?.connected) { - return ( - - ); + return ; } return (
{/* Header */} state.projects); const selectedProjectId = useProjectStore((state) => state.selectedProjectId); const selectedProject = projects.find((p) => p.id === selectedProjectId); @@ -27,25 +27,27 @@ export function GitLabIssues({ onOpenSettings, onNavigateToTask }: GitLabIssuesP isLoading, error, selectedIssueIid, + selectedIssue, filterState, selectIssue, getFilteredIssues, getOpenIssuesCount, handleRefresh, - handleFilterChange + handleFilterChange, } = useGitLabIssues(selectedProject?.id); const { investigationStatus, lastInvestigationResult, startInvestigation, - resetInvestigationStatus + resetInvestigationStatus, } = useGitLabInvestigation(selectedProject?.id); const { searchQuery, setSearchQuery, filteredIssues } = useIssueFiltering(getFilteredIssues()); const [showInvestigateDialog, setShowInvestigateDialog] = useState(false); - const [selectedIssueForInvestigation, setSelectedIssueForInvestigation] = useState(null); + const [selectedIssueForInvestigation, setSelectedIssueForInvestigation] = + useState(null); // Build a map of GitLab issue IIDs to task IDs for quick lookup const issueToTaskMap = useMemo(() => { @@ -63,34 +65,30 @@ export function GitLabIssues({ onOpenSettings, onNavigateToTask }: GitLabIssuesP setShowInvestigateDialog(true); }, []); - const handleStartInvestigation = useCallback((selectedNoteIds: number[]) => { - if (selectedIssueForInvestigation) { - startInvestigation(selectedIssueForInvestigation, selectedNoteIds); - } - }, [selectedIssueForInvestigation, startInvestigation]); + const handleStartInvestigation = useCallback( + (selectedNoteIds: number[]) => { + if (selectedIssueForInvestigation) { + startInvestigation(selectedIssueForInvestigation, selectedNoteIds); + } + }, + [selectedIssueForInvestigation, startInvestigation] + ); const handleCloseDialog = useCallback(() => { setShowInvestigateDialog(false); resetInvestigationStatus(); }, [resetInvestigationStatus]); - const selectedIssue = issues.find(i => i.iid === selectedIssueIid); - // Not connected state if (!syncStatus?.connected) { - return ( - - ); + return ; } return (
{/* Header */} ) : ( - + )}
diff --git a/apps/frontend/src/renderer/components/github-issues/hooks/useGitHubIssues.ts b/apps/frontend/src/renderer/components/github-issues/hooks/useGitHubIssues.ts index ae2d064a..18cd136c 100644 --- a/apps/frontend/src/renderer/components/github-issues/hooks/useGitHubIssues.ts +++ b/apps/frontend/src/renderer/components/github-issues/hooks/useGitHubIssues.ts @@ -1,12 +1,12 @@ -import { useEffect, useCallback, useRef } from 'react'; +import { useEffect, useCallback, useRef, useMemo } from "react"; import { useIssuesStore, useSyncStatusStore, loadGitHubIssues, checkGitHubConnection, - type IssueFilterState -} from '../../../stores/github'; -import type { FilterState } from '../types'; + type IssueFilterState, +} from "../../../stores/github"; +import type { FilterState } from "../types"; export function useGitHubIssues(projectId: string | undefined) { const { @@ -18,7 +18,7 @@ export function useGitHubIssues(projectId: string | undefined) { selectIssue, setFilterState, getFilteredIssues, - getOpenIssuesCount + getOpenIssuesCount, } = useIssuesStore(); const { syncStatus } = useSyncStatusStore(); @@ -50,12 +50,20 @@ export function useGitHubIssues(projectId: string | undefined) { } }, [projectId, filterState]); - const handleFilterChange = useCallback((state: FilterState) => { - setFilterState(state); - if (projectId) { - loadGitHubIssues(projectId, state); - } - }, [projectId, setFilterState]); + const handleFilterChange = useCallback( + (state: FilterState) => { + setFilterState(state); + if (projectId) { + loadGitHubIssues(projectId, state); + } + }, + [projectId, setFilterState] + ); + + // Compute selectedIssue from issues array + const selectedIssue = useMemo(() => { + return issues.find((i) => i.number === selectedIssueNumber) || null; + }, [issues, selectedIssueNumber]); return { issues, @@ -63,11 +71,12 @@ export function useGitHubIssues(projectId: string | undefined) { isLoading, error, selectedIssueNumber, + selectedIssue, filterState, selectIssue, getFilteredIssues, getOpenIssuesCount, handleRefresh, - handleFilterChange + handleFilterChange, }; } diff --git a/apps/frontend/src/renderer/components/github-prs/GitHubPRs.tsx b/apps/frontend/src/renderer/components/github-prs/GitHubPRs.tsx index c4460f8b..b5c1f2aa 100644 --- a/apps/frontend/src/renderer/components/github-prs/GitHubPRs.tsx +++ b/apps/frontend/src/renderer/components/github-prs/GitHubPRs.tsx @@ -1,11 +1,11 @@ -import { useCallback } from 'react'; -import { GitPullRequest, RefreshCw, ExternalLink, Settings } from 'lucide-react'; -import { useTranslation } from 'react-i18next'; -import { useProjectStore } from '../../stores/project-store'; -import { useGitHubPRs, usePRFiltering } from './hooks'; -import { PRList, PRDetail, PRFilterBar } from './components'; -import { Button } from '../ui/button'; -import { ResizablePanels } from '../ui/resizable-panels'; +import { useCallback } from "react"; +import { GitPullRequest, RefreshCw, ExternalLink, Settings } from "lucide-react"; +import { useTranslation } from "react-i18next"; +import { useProjectStore } from "../../stores/project-store"; +import { useGitHubPRs, usePRFiltering } from "./hooks"; +import { PRList, PRDetail, PRFilterBar } from "./components"; +import { Button } from "../ui/button"; +import { ResizablePanels } from "../ui/resizable-panels"; interface GitHubPRsProps { onOpenSettings?: () => void; @@ -15,7 +15,7 @@ interface GitHubPRsProps { function NotConnectedState({ error, onOpenSettings, - t + t, }: { error: string | null; onOpenSettings?: () => void; @@ -25,14 +25,12 @@ function NotConnectedState({
-

{t('prReview.notConnected')}

-

- {error || t('prReview.connectPrompt')} -

+

{t("prReview.notConnected")}

+

{error || t("prReview.connectPrompt")}

{onOpenSettings && ( )}
@@ -52,7 +50,7 @@ function EmptyState({ message }: { message: string }) { } export function GitHubPRs({ onOpenSettings, isActive = false }: GitHubPRsProps) { - const { t } = useTranslation('common'); + const { t } = useTranslation("common"); const projects = useProjectStore((state) => state.projects); const selectedProjectId = useProjectStore((state) => state.selectedProjectId); const selectedProject = projects.find((p) => p.id === selectedProjectId); @@ -82,14 +80,11 @@ export function GitHubPRs({ onOpenSettings, isActive = false }: GitHubPRsProps) isConnected, repoFullName, getReviewStateForPR, + selectedPR, } = useGitHubPRs(selectedProject?.id, { isActive }); - const selectedPR = prs.find(pr => pr.number === selectedPRNumber); - // Get previousResult and newCommitsCheck for follow-up review continuity - const selectedPRReviewState = selectedPRNumber - ? getReviewStateForPR(selectedPRNumber) - : null; + const selectedPRReviewState = selectedPRNumber ? getReviewStateForPR(selectedPRNumber) : null; const previousReviewResult = selectedPRReviewState?.previousResult ?? null; const storedNewCommitsCheck = selectedPRReviewState?.newCommitsCheck ?? null; @@ -130,30 +125,45 @@ export function GitHubPRs({ onOpenSettings, isActive = false }: GitHubPRsProps) } }, [selectedPRNumber, cancelReview]); - const handlePostReview = useCallback(async (selectedFindingIds?: string[], options?: { forceApprove?: boolean }): Promise => { - if (selectedPRNumber && reviewResult) { - return await postReview(selectedPRNumber, selectedFindingIds, options); - } - return false; - }, [selectedPRNumber, reviewResult, postReview]); + const handlePostReview = useCallback( + async ( + selectedFindingIds?: string[], + options?: { forceApprove?: boolean } + ): Promise => { + if (selectedPRNumber && reviewResult) { + return await postReview(selectedPRNumber, selectedFindingIds, options); + } + return false; + }, + [selectedPRNumber, reviewResult, postReview] + ); - const handlePostComment = useCallback(async (body: string) => { - if (selectedPRNumber) { - await postComment(selectedPRNumber, body); - } - }, [selectedPRNumber, postComment]); + const handlePostComment = useCallback( + async (body: string) => { + if (selectedPRNumber) { + await postComment(selectedPRNumber, body); + } + }, + [selectedPRNumber, postComment] + ); - const handleMergePR = useCallback(async (mergeMethod?: 'merge' | 'squash' | 'rebase') => { - if (selectedPRNumber) { - await mergePR(selectedPRNumber, mergeMethod); - } - }, [selectedPRNumber, mergePR]); + const handleMergePR = useCallback( + async (mergeMethod?: "merge" | "squash" | "rebase") => { + if (selectedPRNumber) { + await mergePR(selectedPRNumber, mergeMethod); + } + }, + [selectedPRNumber, mergePR] + ); - const handleAssignPR = useCallback(async (username: string) => { - if (selectedPRNumber) { - await assignPR(selectedPRNumber, username); - } - }, [selectedPRNumber, assignPR]); + const handleAssignPR = useCallback( + async (username: string) => { + if (selectedPRNumber) { + await assignPR(selectedPRNumber, username); + } + }, + [selectedPRNumber, assignPR] + ); const handleGetLogs = useCallback(async () => { if (selectedProjectId && selectedPRNumber) { @@ -174,7 +184,7 @@ export function GitHubPRs({ onOpenSettings, isActive = false }: GitHubPRsProps)

- {t('prReview.pullRequests')} + {t("prReview.pullRequests")}

{repoFullName && ( )} - {prs.length} {t('prReview.open')} + {prs.length} {t("prReview.open")}
-
@@ -235,7 +240,7 @@ export function GitHubPRs({ onOpenSettings, isActive = false }: GitHubPRsProps) selectedPR ? ( ) : ( - + ) } /> diff --git a/apps/frontend/src/renderer/components/github-prs/hooks/useGitHubPRs.ts b/apps/frontend/src/renderer/components/github-prs/hooks/useGitHubPRs.ts index 556eb0d9..756d32d6 100644 --- a/apps/frontend/src/renderer/components/github-prs/hooks/useGitHubPRs.ts +++ b/apps/frontend/src/renderer/components/github-prs/hooks/useGitHubPRs.ts @@ -1,15 +1,19 @@ -import { useState, useEffect, useCallback, useMemo, useRef } from 'react'; +import { useState, useEffect, useCallback, useMemo, useRef } from "react"; import type { PRData, PRReviewResult, PRReviewProgress, - NewCommitsCheck -} from '../../../../preload/api/modules/github-api'; -import { usePRReviewStore, startPRReview as storeStartPRReview, startFollowupReview as storeStartFollowupReview } from '../../../stores/github'; + NewCommitsCheck, +} from "../../../../preload/api/modules/github-api"; +import { + usePRReviewStore, + startPRReview as storeStartPRReview, + startFollowupReview as storeStartFollowupReview, +} from "../../../stores/github"; // Re-export types for consumers export type { PRData, PRReviewResult, PRReviewProgress }; -export type { PRReviewFinding } from '../../../../preload/api/modules/github-api'; +export type { PRReviewFinding } from "../../../../preload/api/modules/github-api"; interface UseGitHubPRsOptions { /** Whether the component is currently active/visible */ @@ -34,18 +38,32 @@ interface UseGitHubPRsResult { selectPR: (prNumber: number | null) => void; refresh: () => Promise; loadMore: () => Promise; - runReview: (prNumber: number) => Promise; - runFollowupReview: (prNumber: number) => Promise; + runReview: (prNumber: number) => void; + runFollowupReview: (prNumber: number) => void; checkNewCommits: (prNumber: number) => Promise; cancelReview: (prNumber: number) => Promise; - postReview: (prNumber: number, selectedFindingIds?: string[], options?: { forceApprove?: boolean }) => Promise; + postReview: ( + prNumber: number, + selectedFindingIds?: string[], + options?: { forceApprove?: boolean } + ) => Promise; postComment: (prNumber: number, body: string) => Promise; - mergePR: (prNumber: number, mergeMethod?: 'merge' | 'squash' | 'rebase') => Promise; + mergePR: (prNumber: number, mergeMethod?: "merge" | "squash" | "rebase") => Promise; assignPR: (prNumber: number, username: string) => Promise; - getReviewStateForPR: (prNumber: number) => { isReviewing: boolean; progress: PRReviewProgress | null; result: PRReviewResult | null; previousResult: PRReviewResult | null; error: string | null; newCommitsCheck?: NewCommitsCheck | null } | null; + getReviewStateForPR: (prNumber: number) => { + isReviewing: boolean; + progress: PRReviewProgress | null; + result: PRReviewResult | null; + previousResult: PRReviewResult | null; + error: string | null; + newCommitsCheck?: NewCommitsCheck | null; + } | null; } -export function useGitHubPRs(projectId?: string, options: UseGitHubPRsOptions = {}): UseGitHubPRsResult { +export function useGitHubPRs( + projectId?: string, + options: UseGitHubPRsOptions = {} +): UseGitHubPRsResult { const { isActive = true } = options; const [prs, setPrs] = useState([]); const [isLoading, setIsLoading] = useState(false); @@ -63,6 +81,8 @@ export function useGitHubPRs(projectId?: string, options: UseGitHubPRsOptions = const wasActiveRef = useRef(isActive); // Track if initial load has happened const hasLoadedRef = useRef(false); + // Track the current PR being fetched (for race condition prevention) + const currentFetchPRNumberRef = useRef(null); // Get PR review state from the global store const prReviews = usePRReviewStore((state) => state.prReviews); @@ -84,99 +104,115 @@ export function useGitHubPRs(projectId?: string, options: UseGitHubPRsOptions = // Get list of PR numbers currently being reviewed const activePRReviews = useMemo(() => { if (!projectId) return []; - return getActivePRReviews(projectId).map(review => review.prNumber); + return getActivePRReviews(projectId).map((review) => review.prNumber); }, [projectId, prReviews, getActivePRReviews]); // Helper to get review state for any PR - const getReviewStateForPR = useCallback((prNumber: number) => { - if (!projectId) return null; - const state = getPRReviewState(projectId, prNumber); - if (!state) return null; - return { - isReviewing: state.isReviewing, - progress: state.progress, - result: state.result, - previousResult: state.previousResult, - error: state.error, - newCommitsCheck: state.newCommitsCheck - }; - }, [projectId, prReviews, getPRReviewState]); + const getReviewStateForPR = useCallback( + (prNumber: number) => { + if (!projectId) return null; + const state = getPRReviewState(projectId, prNumber); + if (!state) return null; + return { + isReviewing: state.isReviewing, + progress: state.progress, + result: state.result, + previousResult: state.previousResult, + error: state.error, + newCommitsCheck: state.newCommitsCheck, + }; + }, + [projectId, prReviews, getPRReviewState] + ); // Use detailed PR data if available (includes files), otherwise fall back to list data - const selectedPR = selectedPRDetails || prs.find(pr => pr.number === selectedPRNumber) || null; + // Validate that selectedPRDetails matches selectedPRNumber to avoid showing stale data + const selectedPR = useMemo(() => { + const matchingDetails = + selectedPRDetails?.number === selectedPRNumber ? selectedPRDetails : null; + return matchingDetails || prs.find((pr) => pr.number === selectedPRNumber) || null; + }, [selectedPRDetails, prs, selectedPRNumber]); // Check connection and fetch PRs - const fetchPRs = useCallback(async (page: number = 1, append: boolean = false) => { - if (!projectId) return; + const fetchPRs = useCallback( + async (page: number = 1, append: boolean = false) => { + if (!projectId) return; - if (append) { - setIsLoadingMore(true); - } else { - setIsLoading(true); - } - setError(null); + if (append) { + setIsLoadingMore(true); + } else { + setIsLoading(true); + } + setError(null); - try { - // First check connection - const connectionResult = await window.electronAPI.github.checkGitHubConnection(projectId); - if (connectionResult.success && connectionResult.data) { - setIsConnected(connectionResult.data.connected); - setRepoFullName(connectionResult.data.repoFullName || null); + try { + // First check connection + const connectionResult = await window.electronAPI.github.checkGitHubConnection(projectId); + if (connectionResult.success && connectionResult.data) { + setIsConnected(connectionResult.data.connected); + setRepoFullName(connectionResult.data.repoFullName || null); - if (connectionResult.data.connected) { - // Fetch PRs with pagination - const result = await window.electronAPI.github.listPRs(projectId, page); - if (result) { - // Check if there are more PRs to load (GitHub returns up to 100 per page) - setHasMore(result.length === 100); - setCurrentPage(page); + if (connectionResult.data.connected) { + // Fetch PRs with pagination + const result = await window.electronAPI.github.listPRs(projectId, page); + if (result) { + // Check if there are more PRs to load (GitHub returns up to 100 per page) + setHasMore(result.length === 100); + setCurrentPage(page); - if (append) { - // Append to existing PRs, deduplicating by PR number - setPrs(prevPrs => { - const existingNumbers = new Set(prevPrs.map(pr => pr.number)); - const newPrs = result.filter(pr => !existingNumbers.has(pr.number)); - return [...prevPrs, ...newPrs]; + if (append) { + // Append to existing PRs, deduplicating by PR number + setPrs((prevPrs) => { + const existingNumbers = new Set(prevPrs.map((pr) => pr.number)); + const newPrs = result.filter((pr) => !existingNumbers.has(pr.number)); + return [...prevPrs, ...newPrs]; + }); + } else { + setPrs(result); + } + + // Batch preload review results for PRs not in store (single IPC call) + const prsNeedingPreload = result.filter((pr) => { + const existingState = getPRReviewState(projectId, pr.number); + return !existingState?.result; }); - } else { - setPrs(result); - } - // Batch preload review results for PRs not in store (single IPC call) - const prsNeedingPreload = result.filter(pr => { - const existingState = getPRReviewState(projectId, pr.number); - return !existingState?.result; - }); + if (prsNeedingPreload.length > 0) { + const prNumbers = prsNeedingPreload.map((pr) => pr.number); + const batchReviews = await window.electronAPI.github.getPRReviewsBatch( + projectId, + prNumbers + ); - if (prsNeedingPreload.length > 0) { - const prNumbers = prsNeedingPreload.map(pr => pr.number); - const batchReviews = await window.electronAPI.github.getPRReviewsBatch(projectId, prNumbers); - - // Update store with loaded results - for (const reviewResult of Object.values(batchReviews)) { - if (reviewResult) { - usePRReviewStore.getState().setPRReviewResult(projectId, reviewResult, { preserveNewCommitsCheck: true }); + // Update store with loaded results + for (const reviewResult of Object.values(batchReviews)) { + if (reviewResult) { + usePRReviewStore.getState().setPRReviewResult(projectId, reviewResult, { + preserveNewCommitsCheck: true, + }); + } } } - } - // Note: New commits check is now lazy - only done when user selects a PR - // or explicitly triggers a check. This significantly speeds up list loading. + // Note: New commits check is now lazy - only done when user selects a PR + // or explicitly triggers a check. This significantly speeds up list loading. + } } + } else { + setIsConnected(false); + setRepoFullName(null); + setError(connectionResult.error || "Failed to check connection"); } - } else { + } catch (err) { + setError(err instanceof Error ? err.message : "Failed to fetch PRs"); setIsConnected(false); - setRepoFullName(null); - setError(connectionResult.error || 'Failed to check connection'); + } finally { + setIsLoading(false); + setIsLoadingMore(false); } - } catch (err) { - setError(err instanceof Error ? err.message : 'Failed to fetch PRs'); - setIsConnected(false); - } finally { - setIsLoading(false); - setIsLoadingMore(false); - } - }, [projectId, getPRReviewState, setNewCommitsCheckAction]); + }, + [projectId, getPRReviewState] + ); // Initial load useEffect(() => { @@ -210,68 +246,88 @@ export function useGitHubPRs(projectId?: string, options: UseGitHubPRsOptions = // No need for local IPC listeners - they're handled globally in github-store - const selectPR = useCallback((prNumber: number | null) => { - setSelectedPRNumber(prNumber); - // Note: Don't reset review result - it comes from the store now - // and persists across navigation + const selectPR = useCallback( + (prNumber: number | null) => { + setSelectedPRNumber(prNumber); + // Note: Don't reset review result - it comes from the store now + // and persists across navigation - // Clear previous detailed PR data when deselecting - if (prNumber === null) { - setSelectedPRDetails(null); - return; - } + // Clear previous detailed PR data when deselecting + if (prNumber === null) { + setSelectedPRDetails(null); + currentFetchPRNumberRef.current = null; + return; + } - if (prNumber && projectId) { - // Fetch full PR details including files - setIsLoadingPRDetails(true); - window.electronAPI.github.getPR(projectId, prNumber) - .then(prDetails => { - if (prDetails) { - setSelectedPRDetails(prDetails); - } - }) - .catch(err => { - console.warn(`Failed to fetch PR details for #${prNumber}:`, err); - }) - .finally(() => { - setIsLoadingPRDetails(false); - }); + if (prNumber && projectId) { + // Track the current PR being fetched (for race condition prevention) + currentFetchPRNumberRef.current = prNumber; - // Load existing review from disk if not already in store - const existingState = getPRReviewState(projectId, prNumber); - // Only fetch from disk if we don't have a result in the store - if (!existingState?.result) { - window.electronAPI.github.getPRReview(projectId, prNumber).then(result => { - if (result) { - // Update store with the loaded result - // Preserve newCommitsCheck when loading existing review from disk - usePRReviewStore.getState().setPRReviewResult(projectId, result, { preserveNewCommitsCheck: true }); + // Fetch full PR details including files + setIsLoadingPRDetails(true); + window.electronAPI.github + .getPR(projectId, prNumber) + .then((prDetails) => { + // Only update if this response is still for the current PR (prevents race condition) + if (prDetails && prNumber === currentFetchPRNumberRef.current) { + setSelectedPRDetails(prDetails); + } + }) + .catch((err) => { + console.warn(`Failed to fetch PR details for #${prNumber}:`, err); + }) + .finally(() => { + // Only clear loading state if this was the last fetch + if (prNumber === currentFetchPRNumberRef.current) { + setIsLoadingPRDetails(false); + } + }); - // Always check for new commits when selecting a reviewed PR - // This ensures fresh data even if we have a cached check from earlier in the session - const reviewedCommitSha = result.reviewedCommitSha || (result as any).reviewed_commit_sha; - if (reviewedCommitSha) { - window.electronAPI.github.checkNewCommits(projectId, prNumber).then(newCommitsResult => { + // Load existing review from disk if not already in store + const existingState = getPRReviewState(projectId, prNumber); + // Only fetch from disk if we don't have a result in the store + if (!existingState?.result) { + window.electronAPI.github.getPRReview(projectId, prNumber).then((result) => { + if (result) { + // Update store with the loaded result + // Preserve newCommitsCheck when loading existing review from disk + usePRReviewStore + .getState() + .setPRReviewResult(projectId, result, { preserveNewCommitsCheck: true }); + + // Always check for new commits when selecting a reviewed PR + // This ensures fresh data even if we have a cached check from earlier in the session + const reviewedCommitSha = result.reviewedCommitSha; + if (reviewedCommitSha) { + window.electronAPI.github + .checkNewCommits(projectId, prNumber) + .then((newCommitsResult) => { + setNewCommitsCheckAction(projectId, prNumber, newCommitsResult); + }) + .catch((err) => { + console.warn(`Failed to check new commits for PR #${prNumber}:`, err); + }); + } + } + }); + } else if (existingState?.result) { + // Review already in store - always check for new commits to get fresh status + const reviewedCommitSha = existingState.result.reviewedCommitSha; + if (reviewedCommitSha) { + window.electronAPI.github + .checkNewCommits(projectId, prNumber) + .then((newCommitsResult) => { setNewCommitsCheckAction(projectId, prNumber, newCommitsResult); - }).catch(err => { + }) + .catch((err) => { console.warn(`Failed to check new commits for PR #${prNumber}:`, err); }); - } } - }); - } else if (existingState?.result) { - // Review already in store - always check for new commits to get fresh status - const reviewedCommitSha = existingState.result.reviewedCommitSha || (existingState.result as any).reviewed_commit_sha; - if (reviewedCommitSha) { - window.electronAPI.github.checkNewCommits(projectId, prNumber).then(newCommitsResult => { - setNewCommitsCheckAction(projectId, prNumber, newCommitsResult); - }).catch(err => { - console.warn(`Failed to check new commits for PR #${prNumber}:`, err); - }); } } - } - }, [projectId, getPRReviewState, setNewCommitsCheckAction]); + }, + [projectId, getPRReviewState, setNewCommitsCheckAction] + ); const refresh = useCallback(async () => { setCurrentPage(1); @@ -284,115 +340,155 @@ export function useGitHubPRs(projectId?: string, options: UseGitHubPRsOptions = await fetchPRs(currentPage + 1, true); }, [fetchPRs, hasMore, isLoadingMore, isLoading, currentPage]); - const runReview = useCallback(async (prNumber: number) => { - if (!projectId) return; + const runReview = useCallback( + (prNumber: number) => { + if (!projectId) return; - // Use the store function which handles both state and IPC - storeStartPRReview(projectId, prNumber); - }, [projectId]); + // Use the store function which handles both state and IPC + storeStartPRReview(projectId, prNumber); + }, + [projectId] + ); - const runFollowupReview = useCallback(async (prNumber: number) => { - if (!projectId) return; + const runFollowupReview = useCallback( + (prNumber: number) => { + if (!projectId) return; - // Use the store function which handles both state and IPC - storeStartFollowupReview(projectId, prNumber); - }, [projectId]); + // Use the store function which handles both state and IPC + storeStartFollowupReview(projectId, prNumber); + }, + [projectId] + ); - const checkNewCommits = useCallback(async (prNumber: number): Promise => { - if (!projectId) { - return { hasNewCommits: false, newCommitCount: 0 }; - } - - try { - const result = await window.electronAPI.github.checkNewCommits(projectId, prNumber); - // Cache the result in the store so the list view can use it - // Use the action from the hook subscription to ensure proper React re-renders - setNewCommitsCheckAction(projectId, prNumber, result); - return result; - } catch (err) { - setError(err instanceof Error ? err.message : 'Failed to check for new commits'); - return { hasNewCommits: false, newCommitCount: 0 }; - } - }, [projectId, setNewCommitsCheckAction]); - - const cancelReview = useCallback(async (prNumber: number): Promise => { - if (!projectId) return false; - - try { - const success = await window.electronAPI.github.cancelPRReview(projectId, prNumber); - if (success) { - // Update store to mark review as cancelled - usePRReviewStore.getState().setPRReviewError(projectId, prNumber, 'Review cancelled by user'); + const checkNewCommits = useCallback( + async (prNumber: number): Promise => { + if (!projectId) { + return { hasNewCommits: false, newCommitCount: 0 }; } - return success; - } catch (err) { - setError(err instanceof Error ? err.message : 'Failed to cancel review'); - return false; - } - }, [projectId]); - const postReview = useCallback(async (prNumber: number, selectedFindingIds?: string[], options?: { forceApprove?: boolean }): Promise => { - if (!projectId) return false; + try { + const result = await window.electronAPI.github.checkNewCommits(projectId, prNumber); + // Cache the result in the store so the list view can use it + // Use the action from the hook subscription to ensure proper React re-renders + setNewCommitsCheckAction(projectId, prNumber, result); + return result; + } catch (err) { + setError(err instanceof Error ? err.message : "Failed to check for new commits"); + return { hasNewCommits: false, newCommitCount: 0 }; + } + }, + [projectId, setNewCommitsCheckAction] + ); - try { - const success = await window.electronAPI.github.postPRReview(projectId, prNumber, selectedFindingIds, options); - if (success) { - // Reload review result to get updated postedAt and finding status - const result = await window.electronAPI.github.getPRReview(projectId, prNumber); - if (result) { - // Preserve newCommitsCheck - posting doesn't change whether there are new commits - usePRReviewStore.getState().setPRReviewResult(projectId, result, { preserveNewCommitsCheck: true }); + const cancelReview = useCallback( + async (prNumber: number): Promise => { + if (!projectId) return false; + + try { + const success = await window.electronAPI.github.cancelPRReview(projectId, prNumber); + if (success) { + // Update store to mark review as cancelled + usePRReviewStore + .getState() + .setPRReviewError(projectId, prNumber, "Review cancelled by user"); } + return success; + } catch (err) { + setError(err instanceof Error ? err.message : "Failed to cancel review"); + return false; } - return success; - } catch (err) { - setError(err instanceof Error ? err.message : 'Failed to post review'); - return false; - } - }, [projectId]); + }, + [projectId] + ); - const postComment = useCallback(async (prNumber: number, body: string): Promise => { - if (!projectId) return false; + const postReview = useCallback( + async ( + prNumber: number, + selectedFindingIds?: string[], + options?: { forceApprove?: boolean } + ): Promise => { + if (!projectId) return false; - try { - return await window.electronAPI.github.postPRComment(projectId, prNumber, body); - } catch (err) { - setError(err instanceof Error ? err.message : 'Failed to post comment'); - return false; - } - }, [projectId]); - - const mergePR = useCallback(async (prNumber: number, mergeMethod: 'merge' | 'squash' | 'rebase' = 'squash'): Promise => { - if (!projectId) return false; - - try { - const success = await window.electronAPI.github.mergePR(projectId, prNumber, mergeMethod); - if (success) { - // Refresh PR list after merge - await fetchPRs(); + try { + const success = await window.electronAPI.github.postPRReview( + projectId, + prNumber, + selectedFindingIds, + options + ); + if (success) { + // Reload review result to get updated postedAt and finding status + const result = await window.electronAPI.github.getPRReview(projectId, prNumber); + if (result) { + // Preserve newCommitsCheck - posting doesn't change whether there are new commits + usePRReviewStore + .getState() + .setPRReviewResult(projectId, result, { preserveNewCommitsCheck: true }); + } + } + return success; + } catch (err) { + setError(err instanceof Error ? err.message : "Failed to post review"); + return false; } - return success; - } catch (err) { - setError(err instanceof Error ? err.message : 'Failed to merge PR'); - return false; - } - }, [projectId, fetchPRs]); + }, + [projectId] + ); - const assignPR = useCallback(async (prNumber: number, username: string): Promise => { - if (!projectId) return false; + const postComment = useCallback( + async (prNumber: number, body: string): Promise => { + if (!projectId) return false; - try { - const success = await window.electronAPI.github.assignPR(projectId, prNumber, username); - if (success) { - // Refresh PR list to update assignees - await fetchPRs(); + try { + return await window.electronAPI.github.postPRComment(projectId, prNumber, body); + } catch (err) { + setError(err instanceof Error ? err.message : "Failed to post comment"); + return false; } - return success; - } catch (err) { - setError(err instanceof Error ? err.message : 'Failed to assign user'); - return false; - } - }, [projectId, fetchPRs]); + }, + [projectId] + ); + + const mergePR = useCallback( + async ( + prNumber: number, + mergeMethod: "merge" | "squash" | "rebase" = "squash" + ): Promise => { + if (!projectId) return false; + + try { + const success = await window.electronAPI.github.mergePR(projectId, prNumber, mergeMethod); + if (success) { + // Refresh PR list after merge + await fetchPRs(); + } + return success; + } catch (err) { + setError(err instanceof Error ? err.message : "Failed to merge PR"); + return false; + } + }, + [projectId, fetchPRs] + ); + + const assignPR = useCallback( + async (prNumber: number, username: string): Promise => { + if (!projectId) return false; + + try { + const success = await window.electronAPI.github.assignPR(projectId, prNumber, username); + if (success) { + // Refresh PR list to update assignees + await fetchPRs(); + } + return success; + } catch (err) { + setError(err instanceof Error ? err.message : "Failed to assign user"); + return false; + } + }, + [projectId, fetchPRs] + ); return { prs, diff --git a/apps/frontend/src/renderer/components/gitlab-issues/hooks/useGitLabIssues.ts b/apps/frontend/src/renderer/components/gitlab-issues/hooks/useGitLabIssues.ts index 91802ce8..c7f80fcf 100644 --- a/apps/frontend/src/renderer/components/gitlab-issues/hooks/useGitLabIssues.ts +++ b/apps/frontend/src/renderer/components/gitlab-issues/hooks/useGitLabIssues.ts @@ -1,6 +1,10 @@ -import { useEffect, useCallback } from 'react'; -import { useGitLabStore, loadGitLabIssues, checkGitLabConnection } from '../../../stores/gitlab-store'; -import type { FilterState } from '../types'; +import { useEffect, useCallback, useMemo } from "react"; +import { + useGitLabStore, + loadGitLabIssues, + checkGitLabConnection, +} from "../../../stores/gitlab-store"; +import type { FilterState } from "../types"; export function useGitLabIssues(projectId: string | undefined) { const { @@ -13,7 +17,7 @@ export function useGitLabIssues(projectId: string | undefined) { selectIssue, setFilterState, getFilteredIssues, - getOpenIssuesCount + getOpenIssuesCount, } = useGitLabStore(); // Always check connection when component mounts or projectId changes @@ -39,12 +43,20 @@ export function useGitLabIssues(projectId: string | undefined) { } }, [projectId, filterState]); - const handleFilterChange = useCallback((state: FilterState) => { - setFilterState(state); - if (projectId) { - loadGitLabIssues(projectId, state); - } - }, [projectId, setFilterState]); + const handleFilterChange = useCallback( + (state: FilterState) => { + setFilterState(state); + if (projectId) { + loadGitLabIssues(projectId, state); + } + }, + [projectId, setFilterState] + ); + + // Compute selectedIssue from issues array + const selectedIssue = useMemo(() => { + return issues.find((i) => i.iid === selectedIssueIid) || null; + }, [issues, selectedIssueIid]); return { issues, @@ -52,11 +64,12 @@ export function useGitLabIssues(projectId: string | undefined) { isLoading, error, selectedIssueIid, + selectedIssue, filterState, selectIssue, getFilteredIssues, getOpenIssuesCount, handleRefresh, - handleFilterChange + handleFilterChange, }; }