From d98f6c8029bd5af0f43a44f51a5b37e43edce747 Mon Sep 17 00:00:00 2001 From: Colin McDonnell Date: Tue, 20 Jan 2026 23:59:52 +0000 Subject: [PATCH] Switch back to grpahql for review threads --- entry | 189 +++++++++++++++++++++++------ mcp/README.md | 42 ++++++- mcp/reviewComments.ts | 275 +++++++++++++++++++++++++++++++++++------- modes.ts | 3 +- utils/errorReport.ts | 15 ++- 5 files changed, 431 insertions(+), 93 deletions(-) diff --git a/entry b/entry index 0221b17..85877db 100755 --- a/entry +++ b/entry @@ -121223,6 +121223,67 @@ function CreatePullRequestReviewTool(ctx) { // mcp/reviewComments.ts import { writeFileSync as writeFileSync3 } from "node:fs"; import { join as join5 } from "node:path"; +var REVIEW_THREADS_QUERY = ` +query ($owner: String!, $repo: String!, $pullNumber: Int!) { + repository(owner: $owner, name: $repo) { + pullRequest(number: $pullNumber) { + reviewThreads(first: 100) { + nodes { + diffSide + startDiffSide + comments(first: 100) { + nodes { + id + databaseId + body + path + line + startLine + url + author { + login + } + createdAt + updatedAt + pullRequestReview { + databaseId + } + replyTo { + databaseId + } + reactionGroups { + content + reactors(first: 10) { + nodes { + ... on Actor { + login + } + } + } + } + } + } + } + } + } + } +} +`; +var MAX_BODY_PREVIEW = 80; +function truncateBody(body) { + const oneLine = body.replace(/\n/g, " ").trim(); + if (oneLine.length <= MAX_BODY_PREVIEW) return oneLine; + return oneLine.slice(0, MAX_BODY_PREVIEW - 3) + "..."; +} +function escapeXml(text) { + return text.replace(/&/g, "&").replace(//g, ">").replace(/"/g, """); +} +function hasThumbsUpFrom(comment, username) { + if (!comment.reactionGroups) return false; + const thumbsUp = comment.reactionGroups.find((g) => g.content === "THUMBS_UP"); + if (!thumbsUp?.reactors?.nodes) return false; + return thumbsUp.reactors.nodes.some((r) => r?.login === username); +} var GetReviewComments = type({ pull_number: type.number.describe("The pull request number"), review_id: type.number.describe("The review ID to get comments for"), @@ -121231,71 +121292,112 @@ var GetReviewComments = type({ function GetReviewCommentsTool(ctx) { return tool({ name: "get_review_comments", - description: "Get review comments for a pull request review, including diff context. When approved_by is provided, only returns comments that user approved with \u{1F44D}. Returns commentsPath pointing to a file with full comment details.", + description: "Get review comments for a pull request review, including thread context. When approved_by is provided, only returns comments that user approved with \u{1F44D}. Returns commentsPath pointing to a file with full comment details in XML format.", parameters: GetReviewComments, execute: execute(async ({ pull_number, review_id, approved_by }) => { - const allComments = await ctx.octokit.paginate(ctx.octokit.rest.pulls.listReviewComments, { + const review = await ctx.octokit.rest.pulls.getReview({ owner: ctx.repo.owner, repo: ctx.repo.name, - pull_number + pull_number, + review_id }); - let reviewComments = allComments.filter((c) => c.pull_request_review_id === review_id); - if (approved_by) { - const approvedIds = /* @__PURE__ */ new Set(); - for (const comment of reviewComments) { - const reactions = await ctx.octokit.rest.reactions.listForPullRequestReviewComment({ - owner: ctx.repo.owner, - repo: ctx.repo.name, - comment_id: comment.id - }); - const hasThumbsUp = reactions.data.some( - (r) => r.content === "+1" && r.user?.login === approved_by - ); - if (hasThumbsUp) approvedIds.add(comment.id); - } - reviewComments = reviewComments.filter((c) => approvedIds.has(c.id)); - } - if (reviewComments.length === 0) { + const reviewer = review.data.user?.login ?? "unknown"; + const response = await ctx.octokit.graphql(REVIEW_THREADS_QUERY, { + owner: ctx.repo.owner, + repo: ctx.repo.name, + pullNumber: pull_number + }); + const pullRequest = response.repository?.pullRequest; + if (!pullRequest?.reviewThreads?.nodes) { return { review_id, pull_number, + reviewer, + count: 0, + commentsPath: null, + message: "No review threads found" + }; + } + const leafComments = []; + for (const thread of pullRequest.reviewThreads.nodes) { + if (!thread?.comments?.nodes) continue; + const threadComments = thread.comments.nodes.filter( + (c) => c !== null + ); + if (threadComments.length === 0) continue; + for (const comment of threadComments) { + if (comment.pullRequestReview?.databaseId !== review_id) continue; + if (approved_by && !hasThumbsUpFrom(comment, approved_by)) continue; + const threadContext = threadComments.filter( + (c) => c.createdAt < comment.createdAt || c.createdAt === comment.createdAt && c.databaseId < comment.databaseId + ); + leafComments.push({ + comment, + thread: threadContext, + side: thread.diffSide + }); + } + } + if (leafComments.length === 0) { + return { + review_id, + pull_number, + reviewer, count: 0, commentsPath: null, message: approved_by ? `No comments with \u{1F44D} from ${approved_by}` : "No comments found for this review" }; } const lines = []; - for (const comment of reviewComments) { - lines.push(`${"=".repeat(60)}`); - lines.push(`COMMENT #${comment.id} by @${comment.user?.login ?? "unknown"}`); - lines.push(`File: ${comment.path}:${comment.line ?? comment.original_line ?? "?"}`); - if (comment.in_reply_to_id) { - lines.push(`Reply to: #${comment.in_reply_to_id}`); + lines.push( + `` + ); + lines.push(""); + lines.push(""); + for (const leaf of leafComments) { + const line = leaf.comment.line ?? leaf.comment.startLine ?? 0; + const preview = escapeXml(truncateBody(leaf.comment.body)); + lines.push( + ` ${preview}` + ); + } + lines.push(""); + lines.push(""); + for (const leaf of leafComments) { + const line = leaf.comment.line ?? leaf.comment.startLine ?? 0; + const author = leaf.comment.author?.login ?? "unknown"; + lines.push( + `` + ); + if (leaf.thread.length > 0) { + lines.push(" "); + for (const msg of leaf.thread) { + const msgAuthor = msg.author?.login ?? "unknown"; + lines.push( + ` ${escapeXml(msg.body)}` + ); + } + lines.push(" "); } - lines.push(""); - if (comment.diff_hunk) { - lines.push("```diff"); - lines.push(comment.diff_hunk); - lines.push("```"); - lines.push(""); - } - lines.push("Comment:"); - lines.push(comment.body); + lines.push(` ${escapeXml(leaf.comment.body)}`); + lines.push(""); lines.push(""); } + lines.push(""); const content = lines.join("\n"); const tempDir = process.env.PULLFROG_TEMP_DIR; if (!tempDir) { throw new Error("PULLFROG_TEMP_DIR not set"); } - const filename = approved_by ? `review-${review_id}-approved-by-${approved_by}.txt` : `review-${review_id}-comments.txt`; + const filename = approved_by ? `review-${review_id}-approved-by-${approved_by}.xml` : `review-${review_id}-comments.xml`; const commentsPath = join5(tempDir, filename); writeFileSync3(commentsPath, content); - log.debug(`wrote ${reviewComments.length} comments to ${commentsPath}`); + log.debug(`wrote ${leafComments.length} comments to ${commentsPath}`); return { review_id, pull_number, - count: reviewComments.length, + reviewer, + count: leafComments.length, commentsPath }; }) @@ -121534,7 +121636,8 @@ function computeModes() { 3. Fetch review comments using ${ghPullfrogMcpName}/get_review_comments with \`pull_number\` and \`review_id\` from EVENT DATA. This returns \`commentsPath\` - read that file for full comment details with diff context. If EVENT DATA contains a \`triggerer\` field (indicating who requested fixes), you can pass \`approved_by\` to filter to only comments they approved with \u{1F44D}. -4. Review the feedback provided. Understand each review comment and what changes are being requested. +4. Review the feedback provided. Understand each review comment and what changes are being requested. + - **If there are no actionable comments** (e.g., the review is an approval with no specific feedback to address), do NOT post a progress comment. Simply exit without taking action. 5. If the request requires understanding the codebase structure or conventions, gather relevant context. Read AGENTS.md if it exists. @@ -139997,12 +140100,18 @@ ${ctx.error}` : ctx.error; } const repoContext = parseRepoContext(); const octokit = createOctokit(getGitHubInstallationToken()); + const runId = process.env.GITHUB_RUN_ID; + const footer = buildPullfrogFooter({ + triggeredBy: true, + workflowRun: runId ? { owner: repoContext.owner, repo: repoContext.name, runId } : void 0 + }); await octokit.rest.issues.updateComment({ owner: repoContext.owner, repo: repoContext.name, comment_id: commentId, - body: formattedError + body: `${formattedError}${footer}` }); + ctx.toolState.progressComment.wasUpdated = true; } // utils/instructions.ts diff --git a/mcp/README.md b/mcp/README.md index ddb6beb..0d3db06 100644 --- a/mcp/README.md +++ b/mcp/README.md @@ -30,20 +30,50 @@ await mcp.call("gh_pullfrog/get_check_suite_logs", { ### review tools #### `get_review_comments` -get all line-by-line comments and their replies for a specific pull request review. +get all line-by-line comments for a specific pull request review, including full thread context for replies. **parameters:** - `pull_number` (number): the pull request number - `review_id` (number): the id from review.id in the webhook payload +- `approved_by` (string, optional): only return comments this user gave a 👍 to **replaces:** `gh api repos/{owner}/{repo}/pulls/{pull_number}/reviews/{review_id}/comments` **returns:** -array of review comments including threaded replies: -- file path, line number, comment body -- side (LEFT/RIGHT) and position in diff -- user, timestamps, html_url -- in_reply_to_id for threaded comments (replies have this set to the parent comment id) +- `commentsPath`: path to XML file with full comment details +- `reviewer`: github username of the review author +- `count`: number of comments to address + +**output format (XML):** +```xml + + + + Actually, can you use a type guard... + This should handle the error case + + + + + Please add null checking here + What about using optional chaining? + + +@@ -40,7 +40,7 @@ + const user = getUser(id); +- return user.name; ++ return user?.name; + + Actually, can you use a type guard instead? + + + +``` + +- `` lists all comments to address with truncated preview +- `` shows parent comments (when replying to existing thread) +- `` contains the diff hunk around the commented line +- `` is the actual comment text to address **example:** ```typescript diff --git a/mcp/reviewComments.ts b/mcp/reviewComments.ts index 7e0eaa2..da71366 100644 --- a/mcp/reviewComments.ts +++ b/mcp/reviewComments.ts @@ -5,6 +5,124 @@ import { log } from "../utils/log.ts"; import type { ToolContext } from "./server.ts"; import { execute, tool } from "./shared.ts"; +// graphql query to fetch all review threads with comments, replies, and reactions +const REVIEW_THREADS_QUERY = ` +query ($owner: String!, $repo: String!, $pullNumber: Int!) { + repository(owner: $owner, name: $repo) { + pullRequest(number: $pullNumber) { + reviewThreads(first: 100) { + nodes { + diffSide + startDiffSide + comments(first: 100) { + nodes { + id + databaseId + body + path + line + startLine + url + author { + login + } + createdAt + updatedAt + pullRequestReview { + databaseId + } + replyTo { + databaseId + } + reactionGroups { + content + reactors(first: 10) { + nodes { + ... on Actor { + login + } + } + } + } + } + } + } + } + } + } +} +`; + +type GraphQLReviewComment = { + id: string; + databaseId: number; + body: string; + path: string; + line: number | null; + startLine: number | null; + url: string; + author: { + login: string; + } | null; + createdAt: string; + updatedAt: string; + pullRequestReview: { + databaseId: number; + } | null; + replyTo: { + databaseId: number; + } | null; + reactionGroups: + | { + content: string; + reactors: { + nodes: ({ login: string } | null)[] | null; + }; + }[] + | null; +}; + +type GraphQLReviewThread = { + diffSide: "LEFT" | "RIGHT"; + startDiffSide: "LEFT" | "RIGHT" | null; + comments: { + nodes: (GraphQLReviewComment | null)[] | null; + } | null; +} | null; + +type GraphQLResponse = { + repository: { + pullRequest: { + reviewThreads: { + nodes: (GraphQLReviewThread | null)[] | null; + } | null; + } | null; + } | null; +}; + +const MAX_BODY_PREVIEW = 80; + +function truncateBody(body: string): string { + const oneLine = body.replace(/\n/g, " ").trim(); + if (oneLine.length <= MAX_BODY_PREVIEW) return oneLine; + return oneLine.slice(0, MAX_BODY_PREVIEW - 3) + "..."; +} + +function escapeXml(text: string): string { + return text + .replace(/&/g, "&") + .replace(//g, ">") + .replace(/"/g, """); +} + +function hasThumbsUpFrom(comment: GraphQLReviewComment, username: string): boolean { + if (!comment.reactionGroups) return false; + const thumbsUp = comment.reactionGroups.find((g) => g.content === "THUMBS_UP"); + if (!thumbsUp?.reactors?.nodes) return false; + return thumbsUp.reactors.nodes.some((r) => r?.login === username); +} + export const GetReviewComments = type({ pull_number: type.number.describe("The pull request number"), review_id: type.number.describe("The review ID to get comments for"), @@ -17,42 +135,82 @@ export function GetReviewCommentsTool(ctx: ToolContext) { return tool({ name: "get_review_comments", description: - "Get review comments for a pull request review, including diff context. " + + "Get review comments for a pull request review, including thread context. " + "When approved_by is provided, only returns comments that user approved with 👍. " + - "Returns commentsPath pointing to a file with full comment details.", + "Returns commentsPath pointing to a file with full comment details in XML format.", parameters: GetReviewComments, execute: execute(async ({ pull_number, review_id, approved_by }) => { - // fetch all review comments via REST API (includes diff_hunk) - const allComments = await ctx.octokit.paginate(ctx.octokit.rest.pulls.listReviewComments, { + // fetch the review to get reviewer username + const review = await ctx.octokit.rest.pulls.getReview({ owner: ctx.repo.owner, repo: ctx.repo.name, pull_number, + review_id, + }); + const reviewer = review.data.user?.login ?? "unknown"; + + // fetch all review threads using graphql (single API call) + const response = await ctx.octokit.graphql(REVIEW_THREADS_QUERY, { + owner: ctx.repo.owner, + repo: ctx.repo.name, + pullNumber: pull_number, }); - // filter to target review - let reviewComments = allComments.filter((c) => c.pull_request_review_id === review_id); - - // filter by thumbs up if approved_by is specified - if (approved_by) { - const approvedIds = new Set(); - for (const comment of reviewComments) { - const reactions = await ctx.octokit.rest.reactions.listForPullRequestReviewComment({ - owner: ctx.repo.owner, - repo: ctx.repo.name, - comment_id: comment.id, - }); - const hasThumbsUp = reactions.data.some( - (r) => r.content === "+1" && r.user?.login === approved_by - ); - if (hasThumbsUp) approvedIds.add(comment.id); - } - reviewComments = reviewComments.filter((c) => approvedIds.has(c.id)); - } - - if (reviewComments.length === 0) { + const pullRequest = response.repository?.pullRequest; + if (!pullRequest?.reviewThreads?.nodes) { return { review_id, pull_number, + reviewer, + count: 0, + commentsPath: null, + message: "No review threads found", + }; + } + + // collect leaf comments (from target review) with their thread context + type LeafComment = { + comment: GraphQLReviewComment; + thread: GraphQLReviewComment[]; // parent comments in order (oldest first) + side: "LEFT" | "RIGHT"; + }; + const leafComments: LeafComment[] = []; + + for (const thread of pullRequest.reviewThreads.nodes) { + if (!thread?.comments?.nodes) continue; + + const threadComments = thread.comments.nodes.filter( + (c): c is GraphQLReviewComment => c !== null + ); + if (threadComments.length === 0) continue; + + // find comments from the target review (these are the "leaf" comments to address) + for (const comment of threadComments) { + if (comment.pullRequestReview?.databaseId !== review_id) continue; + + // filter by approved_by if specified + if (approved_by && !hasThumbsUpFrom(comment, approved_by)) continue; + + // get thread context (all comments before this one in the thread) + const threadContext = threadComments.filter( + (c) => + c.createdAt < comment.createdAt || + (c.createdAt === comment.createdAt && c.databaseId < comment.databaseId) + ); + + leafComments.push({ + comment, + thread: threadContext, + side: thread.diffSide, + }); + } + } + + if (leafComments.length === 0) { + return { + review_id, + pull_number, + reviewer, count: 0, commentsPath: null, message: approved_by @@ -61,27 +219,53 @@ export function GetReviewCommentsTool(ctx: ToolContext) { }; } - // format comments with diff context + // build XML output const lines: string[] = []; - for (const comment of reviewComments) { - lines.push(`${"=".repeat(60)}`); - lines.push(`COMMENT #${comment.id} by @${comment.user?.login ?? "unknown"}`); - lines.push(`File: ${comment.path}:${comment.line ?? comment.original_line ?? "?"}`); - if (comment.in_reply_to_id) { - lines.push(`Reply to: #${comment.in_reply_to_id}`); + lines.push( + `` + ); + lines.push(""); + + // summary section + lines.push(""); + for (const leaf of leafComments) { + const line = leaf.comment.line ?? leaf.comment.startLine ?? 0; + const preview = escapeXml(truncateBody(leaf.comment.body)); + lines.push( + ` ${preview}` + ); + } + lines.push(""); + lines.push(""); + + // detailed comments with thread context + for (const leaf of leafComments) { + const line = leaf.comment.line ?? leaf.comment.startLine ?? 0; + const author = leaf.comment.author?.login ?? "unknown"; + lines.push( + `` + ); + + // thread history (parent comments) + if (leaf.thread.length > 0) { + lines.push(" "); + for (const msg of leaf.thread) { + const msgAuthor = msg.author?.login ?? "unknown"; + lines.push( + ` ${escapeXml(msg.body)}` + ); + } + lines.push(" "); } - lines.push(""); - if (comment.diff_hunk) { - lines.push("```diff"); - lines.push(comment.diff_hunk); - lines.push("```"); - lines.push(""); - } - lines.push("Comment:"); - lines.push(comment.body); + + // the actual comment body to address + lines.push(` ${escapeXml(leaf.comment.body)}`); + lines.push(""); lines.push(""); } + lines.push(""); + const content = lines.join("\n"); // write to temp file @@ -90,16 +274,17 @@ export function GetReviewCommentsTool(ctx: ToolContext) { throw new Error("PULLFROG_TEMP_DIR not set"); } const filename = approved_by - ? `review-${review_id}-approved-by-${approved_by}.txt` - : `review-${review_id}-comments.txt`; + ? `review-${review_id}-approved-by-${approved_by}.xml` + : `review-${review_id}-comments.xml`; const commentsPath = join(tempDir, filename); writeFileSync(commentsPath, content); - log.debug(`wrote ${reviewComments.length} comments to ${commentsPath}`); + log.debug(`wrote ${leafComments.length} comments to ${commentsPath}`); return { review_id, pull_number, - count: reviewComments.length, + reviewer, + count: leafComments.length, commentsPath, }; }), diff --git a/modes.ts b/modes.ts index 2e5a708..c189aa7 100644 --- a/modes.ts +++ b/modes.ts @@ -75,7 +75,8 @@ export function computeModes(): Mode[] { 3. Fetch review comments using ${ghPullfrogMcpName}/get_review_comments with \`pull_number\` and \`review_id\` from EVENT DATA. This returns \`commentsPath\` - read that file for full comment details with diff context. If EVENT DATA contains a \`triggerer\` field (indicating who requested fixes), you can pass \`approved_by\` to filter to only comments they approved with 👍. -4. Review the feedback provided. Understand each review comment and what changes are being requested. +4. Review the feedback provided. Understand each review comment and what changes are being requested. + - **If there are no actionable comments** (e.g., the review is an approval with no specific feedback to address), do NOT post a progress comment. Simply exit without taking action. 5. If the request requires understanding the codebase structure or conventions, gather relevant context. Read AGENTS.md if it exists. diff --git a/utils/errorReport.ts b/utils/errorReport.ts index 62e55cc..6d0bbe0 100644 --- a/utils/errorReport.ts +++ b/utils/errorReport.ts @@ -1,4 +1,5 @@ import type { ToolState } from "../mcp/server.ts"; +import { buildPullfrogFooter } from "./buildPullfrogFooter.ts"; import { createOctokit, parseRepoContext } from "./github.ts"; import { getGitHubInstallationToken } from "./token.ts"; @@ -18,11 +19,23 @@ export async function reportErrorToComment(ctx: ReportErrorParams): Promise