Write diff to file

mcp/checkout.ts

@@ -1,3 +1,5 @@
+import { writeFileSync } from "node:fs";
+import { join } from "node:path";
 import type { Octokit } from "@octokit/rest";
 import { type } from "arktype";
 import type { ToolContext } from "../main.ts";
@@ -20,6 +22,7 @@ export type CheckoutPrResult = {
   url: string;
   headRepo: string;
   diff: string;
+  diffPath: string;
 };
 
 interface CheckoutPrBranchParams {
@@ -140,7 +143,8 @@ export function CheckoutPrTool(ctx: ToolContext) {
   return tool({
     name: "checkout_pr",
     description:
-      "Checkout a pull request branch locally. This fetches the PR branch and sets up push configuration for fork PRs. Use this when you need to work on an existing PR.",
+      "Checkout a pull request branch locally. This fetches the PR branch and sets up push configuration for fork PRs. " +
+      "The PR diff is written to a file (diffPath) for grep access. For small diffs, it's also returned inline.",
     parameters: CheckoutPr,
     execute: execute(async ({ pull_number }) => {
       const result = await checkoutPrBranch({
@@ -174,6 +178,19 @@ export function CheckoutPrTool(ctx: ToolContext) {
         mediaType: { format: "diff" },
       });
 
+      // write diff to file for grep access
+      const diffContent = diffResponse.data as unknown as string;
+      const diffPath = join(process.env.PULLFROG_TEMP_DIR!, `pr-${pull_number}.diff`);
+      writeFileSync(diffPath, diffContent);
+      log.debug(`wrote diff to ${diffPath} (${diffContent.length} bytes)`);
+
+      // return diff inline only if small enough
+      const MAX_INLINE_DIFF_SIZE = 50 * 1024; // 50KB
+      const diff =
+        diffContent.length <= MAX_INLINE_DIFF_SIZE
+          ? diffContent
+          : `Diff is ${(diffContent.length / 1024).toFixed(0)}KB - use grep or read from ${diffPath}`;
+
       return {
         success: true,
         number: pr.data.number,
@@ -184,7 +201,8 @@ export function CheckoutPrTool(ctx: ToolContext) {
         maintainerCanModify: pr.data.maintainer_can_modify,
         url: pr.data.html_url,
         headRepo: headRepo.full_name,
-        diff: diffResponse.data as unknown as string,
+        diff,
+        diffPath,
       } satisfies CheckoutPrResult;
     }),
   });

mcp/review.ts

@@ -92,6 +92,13 @@ export function StartReviewTool(ctx: ToolContext) {
           commit_id: pr.data.head.sha,
           // no 'event' = PENDING review
         });
+        log.debug(`createReview response: ${JSON.stringify(result.data)}`);
+        if (!result.data.id || !result.data.node_id) {
+          log.debug(result);
+          throw new Error(
+            `createReview returned invalid data: id=${result.data.id}, node_id=${result.data.node_id}`
+          );
+        }
         reviewId = result.data.id;
         reviewNodeId = result.data.node_id;
         log.debug(`created new pending review: id=${reviewId}`);
@@ -126,10 +133,7 @@ export function StartReviewTool(ctx: ToolContext) {
       log.debug(`review session started: id=${reviewId}, nodeId=${reviewNodeId}`);
 
       return {
-        message: `Review session started for PR #${pull_number}.`,
-        instructions:
-          "Analyze: What does this PR change? Is the approach sound? What bugs, edge cases, or security issues exist? " +
-          "Before commenting: Skip nitpicks unless requested. Only comment if the codebase maintainer would care.",
+        message: `Review session started for PR #${pull_number}. Add comments with add_review_comment, then submit with submit_review.`,
       };
     }),
   });
@@ -160,28 +164,58 @@ export function AddReviewCommentTool(ctx: ToolContext) {
         throw new Error("No review session started. Call start_review first.");
       }
 
+      const reviewNodeId = ctx.toolState.review.nodeId;
       log.debug(
-        `adding review comment: reviewNodeId=${ctx.toolState.review.nodeId}, path=${path}, line=${line}, side=${side || "RIGHT"}`
+        `adding review comment: reviewNodeId=${reviewNodeId}, path=${path}, line=${line}, side=${side || "RIGHT"}`
       );
 
       // add comment thread via GraphQL (REST doesn't support adding to existing pending review)
-      const result = await ctx.octokit.graphql<AddPullRequestReviewThreadResponse>(
-        ADD_PULL_REQUEST_REVIEW_THREAD,
-        {
-          pullRequestReviewId: ctx.toolState.review.nodeId,
-          path,
-          line,
-          body,
-          side: side || "RIGHT",
-        }
-      );
+      let result: AddPullRequestReviewThreadResponse;
+      try {
+        result = await ctx.octokit.graphql<AddPullRequestReviewThreadResponse>(
+          ADD_PULL_REQUEST_REVIEW_THREAD,
+          {
+            pullRequestReviewId: reviewNodeId,
+            path,
+            line,
+            body,
+            side: side || "RIGHT",
+          }
+        );
+        log.debug(`addPullRequestReviewThread response: ${JSON.stringify(result)}`);
+      } catch (error) {
+        const errorMsg = error instanceof Error ? error.message : String(error);
+        log.debug(`addPullRequestReviewThread error: ${errorMsg}`);
+        throw new Error(
+          `Failed to add comment to ${path}:${line}. GraphQL error: ${errorMsg}. ` +
+            `Ensure the line is part of the diff and the path is correct.`
+        );
+      }
 
-      log.debug(`review comment added: threadId=${result.addPullRequestReviewThread.thread.id}`);
+      // check if the mutation succeeded - null means the line is not in the diff
+      if (!result) {
+        throw new Error(
+          `Failed to add comment to ${path}:${line}. GraphQL returned null response.`
+        );
+      }
+      if (!result.addPullRequestReviewThread) {
+        throw new Error(
+          `Failed to add comment to ${path}:${line}. addPullRequestReviewThread is null. Response: ${JSON.stringify(result)}`
+        );
+      }
+      if (!result.addPullRequestReviewThread.thread) {
+        throw new Error(
+          `Failed to add comment to ${path}:${line}. thread is null. The line must be part of the diff. Response: ${JSON.stringify(result)}`
+        );
+      }
+
+      const threadId = result.addPullRequestReviewThread.thread.id;
+      log.debug(`review comment added: threadId=${threadId}`);
 
       return {
         success: true,
         message: `Comment added to ${path}:${line}`,
-        threadId: result.addPullRequestReviewThread.thread.id,
+        threadId,
       };
     }),
   });
@@ -238,6 +272,10 @@ export function SubmitReviewTool(ctx: ToolContext) {
         body: bodyWithFooter,
       });
 
+      log.debug(`submitReview response: ${JSON.stringify(result.data)}`);
+      if (!result.data.id) {
+        throw new Error(`submitReview returned invalid data: ${JSON.stringify(result.data)}`);
+      }
       log.debug(`review submitted: reviewId=${result.data.id}, state=${result.data.state}`);
 
       // clear review state
@@ -342,6 +380,10 @@ export function ReviewTool(ctx: ToolContext) {
         });
       }
       const result = await ctx.octokit.rest.pulls.createReview(params);
+      log.debug(`createReview (legacy) response: ${JSON.stringify(result.data)}`);
+      if (!result.data.id) {
+        throw new Error(`createReview returned invalid data: ${JSON.stringify(result.data)}`);
+      }
       const reviewId = result.data.id;
 
       // build quick links footer and update the review body