wip

sayansisodiya · sayansisodiya · commit 4aafbb95f3b6 · 2025-08-26T16:49:28.000-07:00
diff --git a/src/github/process-review.ts b/src/github/process-review.ts
@@ -2,6 +2,7 @@ import { getConfig } from "../config.js";
 import { reviewDiff } from "../review/reviewer.js";
 import { GitHubClient } from "./client.js";
 import { CHECK_STATUS, CHECK_CONCLUSION, PRDetails, GitHubPullRequestEvent } from "./types.js";
+import { startReviewSession, endReviewSession } from "../review/comment-collector.js";
 
 export async function processReview(
     jobId: string, 
@@ -61,24 +62,61 @@ export async function processReview(
 
       const prDetailsContent = `Repository: ${payload.repository.full_name}, PR Number: ${prDetails.pr_number}, Commit SHA: ${prDetails.commit_sha}, PR URL: ${prDetails.pr_url}`;
 
+      console.log(`Starting review session for job ${jobId}`);
+      const sessionId = `session-${jobId}`;
+      startReviewSession(sessionId);
+
       console.log(`Calling reviewDiff() for job ${jobId}`);
-      const reviewResult = await reviewDiff(diffContent, prDetailsContent, installationId);
+      await reviewDiff(diffContent, prDetailsContent, installationId, sessionId);
       console.log(`Review completed for job ${jobId}`);
 
-      // Update check run with success
+      // Collect all comments from the review session
+      const collector = endReviewSession(sessionId);
+      console.log(`Collected ${collector.getInlineComments().length} inline comments and ${collector.getGeneralComments().length} general comments`);
+
+      // Post aggregated review if there are any comments
+      if (collector.hasComments()) {
+        console.log('📋 Posting aggregated PR review...');
+        const reviewSummary = collector.getReviewSummary();
+        const inlineComments = collector.getInlineComments();
+        
+        await githubClient.createPRReview(
+          owner,
+          repo,
+          prNumber,
+          reviewSummary,
+          'COMMENT',
+          inlineComments.map(comment => ({
+            path: comment.path,
+            line: comment.line,
+            body: comment.body
+          }))
+        );
+        console.log('✅ PR review posted successfully');
+      }
+
+      // Update check run with success (simplified since review details are now in PR review)
       await githubClient.updateCheckRun(owner, repo, checkRun.id, {
         status: CHECK_STATUS.COMPLETED,
         conclusion: CHECK_CONCLUSION.SUCCESS,
         output: {
           title: 'Code Review Completed',
-          summary: 'Code review has been completed successfully.',
-          text: reviewResult.result || 'Review completed successfully.',
+          summary: 'Code review has been completed successfully. See PR conversation for details.',
         },
         details_url: prUrl,
       });
 
     } catch (error) {
       console.error(`Review job ${jobId} failed with exception:`, error);
+      
+      // End review session if it was started
+      try {
+        const sessionId = `session-${jobId}`;
+        endReviewSession(sessionId);
+      } catch {
+        console.log('No active review session to end');
+      }
+      
       // Post FAILED check run for exceptions
       await postFailureCheckRun(githubClient, payload, error instanceof Error ? error.message : String(error));
   }
diff --git a/src/mcp/server.ts b/src/mcp/server.ts
@@ -149,10 +149,7 @@ class GitHubMCPServer {
           case 'leave_general_comment': {
             console.log(`🗨️  Executing leave_general_comment...`);
             const validatedArgs = validateLeaveGeneralCommentArgs(args);
-            const result = await leaveGeneralComment(
-              validatedArgs,
-              this.config
-            );
+            const result = await leaveGeneralComment(validatedArgs);
             console.log(`✅ leave_general_comment completed in ${Date.now() - startTime}ms`);
             return {
               content: [
@@ -167,10 +164,7 @@ class GitHubMCPServer {
           case 'leave_inline_comment': {
             console.log(`📝 Executing leave_inline_comment...`);
             const validatedArgs = validateLeaveInlineCommentArgs(args);
-            const result = await leaveInlineComment(
-              validatedArgs,
-              this.config
-            );
+            const result = await leaveInlineComment(validatedArgs);
             console.log(`✅ leave_inline_comment completed in ${Date.now() - startTime}ms`);
             return {
               content: [
diff --git a/src/mcp/tools/leave_comment.ts b/src/mcp/tools/leave_comment.ts
@@ -1,5 +1,4 @@
-import { GitHubClient } from '../../github/client.js';
-import { Config } from '../../config.js';
+import { getCurrentCollector } from '../../review/comment-collector.js';
 
 export interface LeaveGeneralCommentArgs {
   message: string;
@@ -9,21 +8,27 @@ export interface LeaveGeneralCommentArgs {
 }
 
 export async function leaveGeneralComment(
-  args: LeaveGeneralCommentArgs,
-  config: Config
-): Promise<{ success: boolean; comment_id?: number; error?: string }> {
+  args: LeaveGeneralCommentArgs
+): Promise<{ success: boolean; error?: string }> {
   try {
-    const { message, owner, repo, pr_number } = args;
+    const { message } = args;
 
-    const githubClient = GitHubClient.fromEnv(config);
-    const response = await githubClient.createPRComment(owner, repo, pr_number, message);
+    console.log('📝 Collecting general comment for later review');
+    
+    // Get session ID from environment (passed by the review process)
+    const sessionId = process.env.REVIEW_SESSION_ID;
+    if (!sessionId) {
+      throw new Error('No REVIEW_SESSION_ID found in environment. Review session not properly initialized.');
+    }
+    
+    const collector = getCurrentCollector(sessionId);
+    collector.addGeneralComment(message);
     
     return {
-      success: true,
-      comment_id: response.id
+      success: true
     };
   } catch (error) {
-    console.error('Failed to leave general comment:', error);
+    console.error('Failed to collect general comment:', error);
     return {
       success: false,
       error: error instanceof Error ? error.message : 'Unknown error'
diff --git a/src/mcp/tools/leave_inline_comment.ts b/src/mcp/tools/leave_inline_comment.ts
@@ -1,5 +1,4 @@
-import { GitHubClient } from '../../github/client.js';
-import { Config } from '../../config.js';
+import { getCurrentCollector } from '../../review/comment-collector.js';
 
 export interface LeaveInlineCommentArgs {
   message: string;
@@ -12,57 +11,27 @@ export interface LeaveInlineCommentArgs {
 }
 
 export async function leaveInlineComment(
-  args: LeaveInlineCommentArgs,
-  config: Config
-): Promise<{ success: boolean; review_id?: number; error?: string }> {
+  args: LeaveInlineCommentArgs
+): Promise<{ success: boolean; error?: string }> {
   try {
-    const { message, owner, repo, pr_number, path, line, commit_sha } = args;
+    const { message, path, line } = args;
 
-    const githubClient = GitHubClient.fromEnv(config);
-
-    console.log('🎯 Creating inline comment via PR review:', { path, line });
-    
-    // Get commit SHA if not provided
-    let actualCommitSha = commit_sha;
+    console.log('📝 Collecting inline comment for later review:', { path, line });
     
-    if (!actualCommitSha) {
-      console.log('🔍 Fetching commit SHA from PR...');
-      try {
-        const prInfo = await githubClient.getPRInfo(owner, repo, pr_number);
-        actualCommitSha = prInfo.head.sha;
-        console.log('✅ Using commit SHA:', actualCommitSha?.substring(0, 8));
-      } catch (error) {
-        console.log('⚠️ Could not fetch PR info for commit SHA:', error);
-        throw new Error('Cannot create inline comment without valid commit SHA');
-      }
-    }
-
-    if (!actualCommitSha) {
-      throw new Error('Missing required commit SHA for inline comment');
+    // Get session ID from environment (passed by the review process)
+    const sessionId = process.env.REVIEW_SESSION_ID;
+    if (!sessionId) {
+      throw new Error('No REVIEW_SESSION_ID found in environment. Review session not properly initialized.');
     }
-
-    console.log('📍 Creating PR review with inline comment at:', { path, line });
-
-    // Create a review with inline comment
-    const response = await githubClient.createPRReview(
-      owner, 
-      repo, 
-      pr_number, 
-      '', // Empty body to avoid duplication
-      'COMMENT',
-      [{
-        path,
-        line,
-        body: message
-      }]
-    );
+    
+    const collector = getCurrentCollector(sessionId);
+    collector.addInlineComment(path, line, message);
     
     return {
-      success: true,
-      review_id: response.id
+      success: true
     };
   } catch (error) {
-    console.error('Failed to leave inline comment:', error);
+    console.error('Failed to collect inline comment:', error);
     return {
       success: false,
       error: error instanceof Error ? error.message : 'Unknown error'
diff --git a/src/review/comment-collector.ts b/src/review/comment-collector.ts
@@ -0,0 +1,79 @@
+// Comment collection system for aggregating MCP tool comments into a single PR review
+
+export interface InlineComment {
+  path: string;
+  line: number;
+  body: string;
+}
+
+export interface GeneralComment {
+  body: string;
+}
+
+export class ReviewCommentCollector {
+  private inlineComments: InlineComment[] = [];
+  private generalComments: GeneralComment[] = [];
+
+  addInlineComment(path: string, line: number, body: string): void {
+    this.inlineComments.push({ path, line, body });
+  }
+
+  addGeneralComment(body: string): void {
+    this.generalComments.push({ body });
+  }
+
+  getInlineComments(): InlineComment[] {
+    return [...this.inlineComments];
+  }
+
+  getGeneralComments(): GeneralComment[] {
+    return [...this.generalComments];
+  }
+
+  getReviewSummary(): string {
+    const generalText = this.generalComments.map(c => c.body).join('\n\n');
+    
+    if (!generalText) {
+      return 'Code review completed.';
+    }
+    
+    return generalText;
+  }
+
+  hasComments(): boolean {
+    return this.inlineComments.length > 0 || this.generalComments.length > 0;
+  }
+
+  clear(): void {
+    this.inlineComments = [];
+    this.generalComments = [];
+  }
+}
+
+// Global collectors keyed by session ID for concurrent reviews
+const activeCollectors = new Map<string, ReviewCommentCollector>();
+
+export function startReviewSession(sessionId: string): ReviewCommentCollector {
+  const collector = new ReviewCommentCollector();
+  activeCollectors.set(sessionId, collector);
+  console.log(`Started review session: ${sessionId}`);
+  return collector;
+}
+
+export function getCurrentCollector(sessionId: string): ReviewCommentCollector {
+  const collector = activeCollectors.get(sessionId);
+  if (!collector) {
+    throw new Error(`No active review session found for ID: ${sessionId}. Call startReviewSession() first.`);
+  }
+  return collector;
+}
+
+export function endReviewSession(sessionId: string): ReviewCommentCollector {
+  const collector = activeCollectors.get(sessionId);
+  if (!collector) {
+    throw new Error(`No active review session found for ID: ${sessionId}.`);
+  }
+  activeCollectors.delete(sessionId);
+  console.log(`Ended review session: ${sessionId}`);
+  return collector;
+}
diff --git a/src/review/reviewer.ts b/src/review/reviewer.ts
@@ -6,7 +6,7 @@ import { Config, getConfig } from "../config.js";
 import { newThread, execute } from "../amp.js";
 
 
-export const reviewDiff = async (diffContent: string, mrDetailsContent: string, installationId: number) => {
+export const reviewDiff = async (diffContent: string, mrDetailsContent: string, installationId: number, sessionId?: string) => {
 
     // Get config
     const config: Config = getConfig();
@@ -45,14 +45,15 @@ export const reviewDiff = async (diffContent: string, mrDetailsContent: string,
       // Write settings to file with installation ID
       const settings = { ...ampConfig.settings };
       
-      // Ensure GitHub MCP server environment exists and set installation ID
+      // Ensure GitHub MCP server environment exists and set installation ID and session ID
       settings['amp.mcpServers'] = {
         ...settings['amp.mcpServers'],
         github: {
           ...settings['amp.mcpServers']?.github,
           env: {
             ...settings['amp.mcpServers']?.github?.env,
             GITHUB_INSTALLATION_ID: installationId.toString(),
+            ...(sessionId && { REVIEW_SESSION_ID: sessionId }),
           }
         }
       };
diff --git a/src/routes/github.ts b/src/routes/github.ts
@@ -12,7 +12,7 @@ const github = new Hono();
 const config: Config = getConfig();
 
 // PR actions that trigger code reviews
-const REVIEW_TRIGGER_ACTIONS = ['opened', 'reopened', 'ready_for_review'];
+const REVIEW_TRIGGER_ACTIONS = ['opened', 'reopened', 'ready_for_review', 'review_requested'];
 
 // We'll need to receive the reviewQueue from the main server
 let reviewQueue: ReviewJobQueue | null = null;
@@ -58,6 +58,16 @@ async function handlePullRequestEvent(payload: GitHubPullRequestEvent) {
     return { message: 'Action ignored' };
   }
 
+  // For review_requested actions, only respond if our bot was requested
+  if (action === 'review_requested') {
+    const requestedReviewer = (payload as any).requested_reviewer;
+    if (!requestedReviewer || requestedReviewer.type !== 'Bot') {
+      return { message: 'Review not requested for this bot' };
+    }
+    // You could add additional bot name checking here if needed
+    console.log('Re-review requested for bot:', requestedReviewer.login);
+  }
+
   console.log(`Processing PR ${payload.pull_request.number} action: ${action}`);
 
   // Extract installation ID directly from webhook payload
