Fixes

2026-01-12 13:55:43 -08:00
parent c6572f0987
commit 45f837cedb
12 changed files with 1731 additions and 1391 deletions
@@ -5,7 +5,12 @@ import { agentsManifest } from "../external.ts";
 import type { ToolContext } from "../main.ts";
 import { fetchWorkflowRunInfo } from "../utils/api.ts";
 import { buildPullfrogFooter, stripExistingFooter } from "../utils/buildPullfrogFooter.ts";
-import { createOctokit, getGitHubInstallationToken, parseRepoContext, type OctokitWithPlugins } from "../utils/github.ts";
+import {
+  createOctokit,
+  getGitHubInstallationToken,
+  type OctokitWithPlugins,
+  parseRepoContext,
+} from "../utils/github.ts";
 import { execute, tool } from "./shared.ts";

 /**
@@ -82,7 +87,11 @@ function buildImplementPlanLink(
  return `[Implement plan ➔](${apiUrl}/trigger/${owner}/${repo}/${issueNumber}?action=implement&comment_id=${commentId})`;
 }

-async function addFooter(body: string, payload: Payload, octokit?: OctokitWithPlugins): Promise<string> {
+async function addFooter(
+  body: string,
+  payload: Payload,
+  octokit?: OctokitWithPlugins
+): Promise<string> {
  const bodyWithoutFooter = stripExistingFooter(body);
  const footer = await buildCommentFooter({ payload, octokit });
  return `${bodyWithoutFooter}${footer}`;
@@ -494,6 +503,6 @@ export function ReplyToReviewCommentTool(ctx: ToolContext) {
        body: result.data.body,
        in_reply_to_id: result.data.in_reply_to_id,
      };
-    }),
+    }, "reply_to_review_comment"),
  });
 }
@@ -93,95 +93,95 @@ export function GetReviewCommentsTool(ctx: ToolContext) {
      "Get all review comments and their replies for a specific pull request review. Returns line-by-line comments that were left on specific code locations, including any threaded replies.",
    parameters: GetReviewComments,
    execute: execute(async ({ pull_number, review_id }) => {
-      // fetch all review threads using graphql
-      const response = await ctx.octokit.graphql<GraphQLResponse>(REVIEW_THREADS_QUERY, {
-        owner: ctx.owner,
-        repo: ctx.name,
-        pullNumber: pull_number,
-      });
+        // fetch all review threads using graphql
+        const response = await ctx.octokit.graphql<GraphQLResponse>(REVIEW_THREADS_QUERY, {
+          owner: ctx.owner,
+          repo: ctx.name,
+          pullNumber: pull_number,
+        });

-      const pullRequest = response.repository?.pullRequest;
-      if (!pullRequest) {
-        return {
-          review_id,
-          pull_number,
-          comments: [],
-          count: 0,
-        };
-      }
-
-      const threadNodes = pullRequest.reviewThreads?.nodes;
-      if (!threadNodes) {
-        return {
-          review_id,
-          pull_number,
-          comments: [],
-          count: 0,
-        };
-      }
-
-      const allComments: {
-        id: number;
-        body: string;
-        path: string;
-        line: number | null;
-        side: "LEFT" | "RIGHT";
-        start_line: number | null;
-        start_side: "LEFT" | "RIGHT" | null;
-        user: string | null;
-        created_at: string;
-        updated_at: string;
-        html_url: string;
-        in_reply_to_id: number | null;
-        pull_request_review_id: number | null;
-      }[] = [];
-
-      // iterate through all threads (filter out nulls)
-      for (const thread of threadNodes) {
-        if (!thread?.comments?.nodes) continue;
-
-        // filter out null comments
-        const threadComments = thread.comments.nodes.filter(
-          (c): c is GraphQLReviewComment => c !== null
-        );
-        if (threadComments.length === 0) continue;
-
-        // find the root comment (the one with replyTo == null) to determine thread ownership
-        const rootComment = threadComments.find((c) => c.replyTo === null);
-        if (!rootComment) continue;
-
-        // check if this thread belongs to the target review using the root comment
-        const threadBelongsToReview = rootComment.pullRequestReview?.databaseId === review_id;
-        if (!threadBelongsToReview) continue;
-
-        // include all comments from this thread (original + replies)
-        // side info comes from thread level, not comment level
-        for (const comment of threadComments) {
-          allComments.push({
-            id: comment.databaseId,
-            body: comment.body,
-            path: comment.path,
-            line: comment.line,
-            start_line: comment.startLine,
-            side: thread.diffSide,
-            start_side: thread.startDiffSide,
-            user: comment.author?.login ?? null,
-            created_at: comment.createdAt,
-            updated_at: comment.updatedAt,
-            html_url: comment.url,
-            in_reply_to_id: comment.replyTo?.databaseId ?? null,
-            pull_request_review_id: comment.pullRequestReview?.databaseId ?? null,
-          });
+        const pullRequest = response.repository?.pullRequest;
+        if (!pullRequest) {
+          return {
+            review_id,
+            pull_number,
+            comments: [],
+            count: 0,
+          };
        }
-      }

-      return {
-        review_id,
-        pull_number,
-        comments: allComments,
-        count: allComments.length,
-      };
-    }),
+        const threadNodes = pullRequest.reviewThreads?.nodes;
+        if (!threadNodes) {
+          return {
+            review_id,
+            pull_number,
+            comments: [],
+            count: 0,
+          };
+        }
+
+        const allComments: {
+          id: number;
+          body: string;
+          path: string;
+          line: number | null;
+          side: "LEFT" | "RIGHT";
+          start_line: number | null;
+          start_side: "LEFT" | "RIGHT" | null;
+          user: string | null;
+          created_at: string;
+          updated_at: string;
+          html_url: string;
+          in_reply_to_id: number | null;
+          pull_request_review_id: number | null;
+        }[] = [];
+
+        // iterate through all threads (filter out nulls)
+        for (const thread of threadNodes) {
+          if (!thread?.comments?.nodes) continue;
+
+          // filter out null comments
+          const threadComments = thread.comments.nodes.filter(
+            (c): c is GraphQLReviewComment => c !== null
+          );
+          if (threadComments.length === 0) continue;
+
+          // find the root comment (the one with replyTo == null) to determine thread ownership
+          const rootComment = threadComments.find((c) => c.replyTo === null);
+          if (!rootComment) continue;
+
+          // check if this thread belongs to the target review using the root comment
+          const threadBelongsToReview = rootComment.pullRequestReview?.databaseId === review_id;
+          if (!threadBelongsToReview) continue;
+
+          // include all comments from this thread (original + replies)
+          // side info comes from thread level, not comment level
+          for (const comment of threadComments) {
+            allComments.push({
+              id: comment.databaseId,
+              body: comment.body,
+              path: comment.path,
+              line: comment.line,
+              start_line: comment.startLine,
+              side: thread.diffSide,
+              start_side: thread.startDiffSide,
+              user: comment.author?.login ?? null,
+              created_at: comment.createdAt,
+              updated_at: comment.updatedAt,
+              html_url: comment.url,
+              in_reply_to_id: comment.replyTo?.databaseId ?? null,
+              pull_request_review_id: comment.pullRequestReview?.databaseId ?? null,
+            });
+          }
+        }
+
+        return {
+          review_id,
+          pull_number,
+          comments: allComments,
+          count: allComments.length,
+        };
+      }),
  });
 }

@@ -2,6 +2,7 @@ import type { StandardSchemaV1 } from "@standard-schema/spec";
 import { encode as toonEncode } from "@toon-format/toon";
 import type { FastMCP, Tool } from "fastmcp";
 import type { ToolContext } from "../main.ts";
+import { formatJsonValue, log } from "../utils/cli.ts";

 export const tool = <const params>(toolDef: Tool<any, StandardSchemaV1<params>>) => toolDef;

@@ -36,13 +37,22 @@ export const handleToolError = (error: unknown): ToolResult => {
 /**
 * Helper to wrap a tool execute function with error handling.
 * Captures ctx in closure so tools don't need to handle try/catch.
+ * @param fn - the function to execute
+ * @param toolName - optional tool name for error logging
 */
-export const execute = <T>(fn: (params: T) => Promise<Record<string, any> | string>) => {
+export const execute = <T>(
+  fn: (params: T) => Promise<Record<string, any> | string>,
+  toolName?: string
+) => {
  return async (params: T): Promise<ToolResult> => {
    try {
      const result = await fn(params);
      return handleToolSuccess(result);
    } catch (error) {
+      const errorMessage = error instanceof Error ? error.message : String(error);
+      const prefix = toolName ? `[${toolName}]` : "tool";
+      log.error(`${prefix} error: ${errorMessage}`);
+      log.debug(`${prefix} params: ${formatJsonValue(params)}`);
      return handleToolError(error);
    }
  };