Switch back to one-shot reviews

This commit is contained in:
Colin McDonnell
2025-12-22 14:55:19 -08:00
parent 5353d80388
commit 25896559f0
6 changed files with 276 additions and 404 deletions
+7 -1
View File
@@ -180,7 +180,13 @@ export function CheckoutPrTool(ctx: ToolContext) {
// 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`);
const tempDir = process.env.PULLFROG_TEMP_DIR;
if (!tempDir) {
throw new Error(
"PULLFROG_TEMP_DIR not set - checkout_pr must run in pullfrog action context"
);
}
const diffPath = join(tempDir, `pr-${pull_number}.diff`);
writeFileSync(diffPath, diffContent);
log.debug(`wrote diff to ${diffPath} (${diffContent.length} bytes)`);
+138 -129
View File
@@ -6,16 +6,150 @@ import { log } from "../utils/cli.ts";
import { deleteProgressComment } from "./comment.ts";
import { execute, tool } from "./shared.ts";
// one-shot review tool
export const CreatePullRequestReview = type({
pull_number: type.number.describe("The pull request number to review"),
body: type.string
.describe(
"1-2 sentence high-level summary ONLY. Include urgency level and critical callouts (e.g., API key leak). ALL specific feedback MUST go in 'comments' array instead."
)
.optional(),
commit_id: type.string
.describe("Optional SHA of the commit being reviewed. Defaults to latest.")
.optional(),
comments: type({
path: type.string.describe("The file path to comment on (relative to repo root)"),
line: type.number.describe(
"The line number in the file (use line numbers from the diff - usually the RIGHT side/new code)"
),
side: type
.enumerated("LEFT", "RIGHT")
.describe(
"Side of the diff: LEFT (old code) or RIGHT (new code). Defaults to RIGHT if not provided."
)
.optional(),
body: type.string.describe(
"The comment text for this specific line. For issues appearing multiple times, comment on the first occurrence and reference others."
),
start_line: type.number
.describe("Start line for multi-line comments (optional, for commenting on ranges)")
.optional(),
})
.array()
.describe(
// FORK PR NOTE: checkout_pr returns the diff via GitHub API - use that for line numbers
"PRIMARY location for ALL feedback. 95%+ of review content should be here. Use the diff returned from checkout_pr to find correct line numbers (RIGHT side for new code, LEFT for old)."
)
.optional(),
});
export function CreatePullRequestReviewTool(ctx: ToolContext) {
return tool({
name: "create_pull_request_review",
description:
"Submit a review for an existing pull request. " +
"IMPORTANT: 95%+ of feedback should be in 'comments' array with file paths and line numbers. " +
"Only use 'body' for a 1-2 sentence summary with urgency and critical callouts.",
parameters: CreatePullRequestReview,
execute: execute(async ({ pull_number, body, commit_id, comments = [] }) => {
// set PR context
ctx.toolState.prNumber = pull_number;
// get the PR to determine the head commit if commit_id not provided
const pr = await ctx.octokit.rest.pulls.get({
owner: ctx.owner,
repo: ctx.name,
pull_number,
});
// compose the request
const params: RestEndpointMethodTypes["pulls"]["createReview"]["parameters"] = {
owner: ctx.owner,
repo: ctx.name,
pull_number,
event: "COMMENT",
};
if (body) params.body = body;
if (commit_id) {
params.commit_id = commit_id;
} else {
params.commit_id = pr.data.head.sha;
}
if (comments.length > 0) {
type ReviewComment = (typeof params.comments & {})[number];
// convert comments to the format expected by GitHub API
params.comments = comments.map((comment) => {
const reviewComment: ReviewComment = {
...comment,
};
reviewComment.side = comment.side || "RIGHT";
if (comment.start_line) {
reviewComment.start_line = comment.start_line;
reviewComment.start_side = comment.side || "RIGHT";
}
return reviewComment;
});
}
const result = await ctx.octokit.rest.pulls.createReview(params);
log.debug(`createReview 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
const apiUrl = process.env.API_URL || "https://pullfrog.com";
const fixAllUrl = `${apiUrl}/trigger/${ctx.owner}/${ctx.name}/${pull_number}?action=fix&review_id=${reviewId}`;
const fixApprovedUrl = `${apiUrl}/trigger/${ctx.owner}/${ctx.name}/${pull_number}?action=fix-approved&review_id=${reviewId}`;
const footer = buildPullfrogFooter({
workflowRun: { owner: ctx.owner, repo: ctx.name, runId: ctx.runId, jobId: ctx.jobId },
customParts: [`[Fix all ➔](${fixAllUrl})`, `[Fix 👍s ➔](${fixApprovedUrl})`],
});
const updatedBody = (body || "") + footer;
// update the review with the footer
await ctx.octokit.rest.pulls.updateReview({
owner: ctx.owner,
repo: ctx.name,
pull_number,
review_id: reviewId,
body: updatedBody,
});
await deleteProgressComment(ctx);
return {
success: true,
reviewId,
html_url: result.data.html_url,
state: result.data.state,
user: result.data.user?.login,
submitted_at: result.data.submitted_at,
};
}),
});
}
// =============================================================================
// COMMENTED OUT: Three-step review flow (start_review, add_review_comment, submit_review)
// This approach used GraphQL to add comments to a pending review one-by-one,
// but GitHub's API was returning null for valid lines. Keeping for reference.
// =============================================================================
/*
// graphql mutation to add a comment thread to a pending review
// note: REST API doesn't support adding comments to an existing pending review
const ADD_PULL_REQUEST_REVIEW_THREAD = `
mutation AddPullRequestReviewThread($pullRequestReviewId: ID!, $path: String!, $line: Int!, $body: String!, $side: DiffSide) {
mutation AddPullRequestReviewThread($pullRequestReviewId: ID!, $path: String!, $line: Int!, $body: String!, $side: DiffSide, $subjectType: PullRequestReviewThreadSubjectType) {
addPullRequestReviewThread(input: {
pullRequestReviewId: $pullRequestReviewId,
path: $path,
line: $line,
body: $body,
side: $side
side: $side,
subjectType: $subjectType
}) {
thread {
id
@@ -180,6 +314,7 @@ export function AddReviewCommentTool(ctx: ToolContext) {
line,
body,
side: side || "RIGHT",
subjectType: "LINE",
}
);
log.debug(`addPullRequestReviewThread response: ${JSON.stringify(result)}`);
@@ -293,130 +428,4 @@ export function SubmitReviewTool(ctx: ToolContext) {
}),
});
}
// legacy tool - kept for backwards compatibility
export const Review = type({
pull_number: type.number.describe("The pull request number to review"),
body: type.string
.describe(
"1-2 sentence high-level summary ONLY. Include urgency level and critical callouts (e.g., API key leak). ALL specific feedback MUST go in 'comments' array instead."
)
.optional(),
commit_id: type.string
.describe("Optional SHA of the commit being reviewed. Defaults to latest.")
.optional(),
comments: type({
path: type.string.describe("The file path to comment on (relative to repo root)"),
line: type.number.describe(
"The line number in the file (use line numbers from the diff - usually the RIGHT side/new code)"
),
side: type
.enumerated("LEFT", "RIGHT")
.describe(
"Side of the diff: LEFT (old code) or RIGHT (new code). Defaults to RIGHT if not provided."
)
.optional(),
body: type.string.describe(
"The comment text for this specific line. For issues appearing multiple times, comment on the first occurrence and reference others."
),
start_line: type.number
.describe("Start line for multi-line comments (optional, for commenting on ranges)")
.optional(),
})
.array()
.describe(
// FORK PR NOTE: checkout_pr returns the diff via GitHub API - use that for line numbers
"PRIMARY location for ALL feedback. 95%+ of review content should be here. Use the diff returned from checkout_pr to find correct line numbers (RIGHT side for new code, LEFT for old)."
)
.optional(),
});
export function ReviewTool(ctx: ToolContext) {
return tool({
name: "submit_pull_request_review",
description:
"DEPRECATED: Use start_review, add_review_comment, and submit_review instead for iterative review workflow. " +
"Submit a review for an existing pull request. " +
"IMPORTANT: 95%+ of feedback should be in 'comments' array with file paths and line numbers. " +
"Only use 'body' for a 1-2 sentence summary with urgency and critical callouts.",
parameters: Review,
execute: execute(async ({ pull_number, body, commit_id, comments = [] }) => {
// set PR context
ctx.toolState.prNumber = pull_number;
// get the PR to determine the head commit if commit_id not provided
const pr = await ctx.octokit.rest.pulls.get({
owner: ctx.owner,
repo: ctx.name,
pull_number,
});
// compose the request
const params: RestEndpointMethodTypes["pulls"]["createReview"]["parameters"] = {
owner: ctx.owner,
repo: ctx.name,
pull_number,
event: "COMMENT",
};
if (body) params.body = body;
if (commit_id) {
params.commit_id = commit_id;
} else {
params.commit_id = pr.data.head.sha;
}
if (comments.length > 0) {
type ReviewComment = (typeof params.comments & {})[number];
// convert comments to the format expected by GitHub API
params.comments = comments.map((comment) => {
const reviewComment: ReviewComment = {
...comment,
};
reviewComment.side = comment.side || "RIGHT";
if (comment.start_line) {
reviewComment.start_line = comment.start_line;
reviewComment.start_side = comment.side || "RIGHT";
}
return reviewComment;
});
}
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
const apiUrl = process.env.API_URL || "https://pullfrog.com";
const fixAllUrl = `${apiUrl}/trigger/${ctx.owner}/${ctx.name}/${pull_number}?action=fix&review_id=${reviewId}`;
const fixApprovedUrl = `${apiUrl}/trigger/${ctx.owner}/${ctx.name}/${pull_number}?action=fix-approved&review_id=${reviewId}`;
const footer = buildPullfrogFooter({
workflowRun: { owner: ctx.owner, repo: ctx.name, runId: ctx.runId, jobId: ctx.jobId },
customParts: [`[Fix all ➔](${fixAllUrl})`, `[Fix 👍s ➔](${fixApprovedUrl})`],
});
const updatedBody = (body || "") + footer;
// update the review with the footer
await ctx.octokit.rest.pulls.updateReview({
owner: ctx.owner,
repo: ctx.name,
pull_number,
review_id: reviewId,
body: updatedBody,
});
await deleteProgressComment(ctx);
return {
success: true,
reviewId,
html_url: result.data.html_url,
state: result.data.state,
user: result.data.user?.login,
submitted_at: result.data.submitted_at,
};
}),
});
}
*/
+2 -5
View File
@@ -26,7 +26,7 @@ import { IssueInfoTool } from "./issueInfo.ts";
import { AddLabelsTool } from "./labels.ts";
import { PullRequestTool } from "./pr.ts";
import { PullRequestInfoTool } from "./prInfo.ts";
import { AddReviewCommentTool, StartReviewTool, SubmitReviewTool } from "./review.ts";
import { CreatePullRequestReviewTool } from "./review.ts";
import { GetReviewCommentsTool, ListPullRequestReviewsTool } from "./reviewComments.ts";
import { SelectModeTool } from "./selectMode.ts";
import { addTools } from "./shared.ts";
@@ -84,10 +84,7 @@ export async function startMcpHttpServer(
GetIssueCommentsTool(ctx),
GetIssueEventsTool(ctx),
PullRequestTool(ctx),
// ReviewTool(ctx),
StartReviewTool(ctx),
AddReviewCommentTool(ctx),
SubmitReviewTool(ctx),
CreatePullRequestReviewTool(ctx),
PullRequestInfoTool(ctx),
CheckoutPrTool(ctx),
GetReviewCommentsTool(ctx),