diff --git a/entry b/entry index bac6a9d..cadfe95 100755 --- a/entry +++ b/entry @@ -94721,7 +94721,7 @@ var agents = { // main.ts import { mkdtemp as mkdtemp2 } from "node:fs/promises"; import { tmpdir as tmpdir2 } from "node:os"; -import { join as join10 } from "node:path"; +import { join as join11 } from "node:path"; // node_modules/.pnpm/universal-user-agent@7.0.3/node_modules/universal-user-agent/index.js function getUserAgent() { @@ -122940,6 +122940,10 @@ var FastMCP = class extends FastMCPEventEmitter { } }; +// mcp/checkout.ts +import { writeFileSync as writeFileSync4 } from "node:fs"; +import { join as join8 } from "node:path"; + // utils/shell.ts import { spawnSync as spawnSync4 } from "node:child_process"; function $(cmd, args3, options) { @@ -123046,7 +123050,7 @@ async function checkoutPrBranch(params) { function CheckoutPrTool(ctx) { 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.", + description: "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({ @@ -123072,6 +123076,12 @@ function CheckoutPrTool(ctx) { pull_number, mediaType: { format: "diff" } }); + const diffContent = diffResponse.data; + const diffPath = join8(process.env.PULLFROG_TEMP_DIR, `pr-${pull_number}.diff`); + writeFileSync4(diffPath, diffContent); + log.debug(`wrote diff to ${diffPath} (${diffContent.length} bytes)`); + const MAX_INLINE_DIFF_SIZE = 50 * 1024; + 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, @@ -123082,7 +123092,8 @@ function CheckoutPrTool(ctx) { maintainerCanModify: pr.data.maintainer_can_modify, url: pr.data.html_url, headRepo: headRepo.full_name, - diff: diffResponse.data + diff, + diffPath }; }) }); @@ -123191,7 +123202,7 @@ function DebugShellCommandTool(_ctx) { // prep/installNodeDependencies.ts import { existsSync as existsSync3, readFileSync as readFileSync2 } from "node:fs"; -import { join as join8 } from "node:path"; +import { join as join9 } from "node:path"; // node_modules/.pnpm/package-manager-detector@1.6.0/node_modules/package-manager-detector/dist/commands.mjs function dashDashArg(agent2, agentCommand) { @@ -123503,7 +123514,7 @@ async function isCommandAvailable(command) { return result.exitCode === 0; } function getPackageManagerFromPackageJson() { - const packageJsonPath = join8(process.cwd(), "package.json"); + const packageJsonPath = join9(process.cwd(), "package.json"); try { const content = readFileSync2(packageJsonPath, "utf-8"); const pkg = JSON.parse(content); @@ -123534,7 +123545,7 @@ async function installPackageManager(name, installSpec) { return result.stderr || `failed to install ${name}`; } if (name === "deno") { - const denoPath = join8(process.env.HOME || "", ".deno", "bin"); + const denoPath = join9(process.env.HOME || "", ".deno", "bin"); process.env.PATH = `${denoPath}:${process.env.PATH}`; } log.info(`\u2705 installed ${name}`); @@ -123543,7 +123554,7 @@ async function installPackageManager(name, installSpec) { var installNodeDependencies = { name: "installNodeDependencies", shouldRun: () => { - const packageJsonPath = join8(process.cwd(), "package.json"); + const packageJsonPath = join9(process.cwd(), "package.json"); return existsSync3(packageJsonPath); }, run: async () => { @@ -123606,7 +123617,7 @@ var installNodeDependencies = { // prep/installPythonDependencies.ts import { existsSync as existsSync4 } from "node:fs"; -import { join as join9 } from "node:path"; +import { join as join10 } from "node:path"; var PYTHON_CONFIGS = [ { file: "requirements.txt", @@ -123678,11 +123689,11 @@ var installPythonDependencies = { return false; } const cwd2 = process.cwd(); - return PYTHON_CONFIGS.some((config2) => existsSync4(join9(cwd2, config2.file))); + return PYTHON_CONFIGS.some((config2) => existsSync4(join10(cwd2, config2.file))); }, run: async () => { const cwd2 = process.cwd(); - const config2 = PYTHON_CONFIGS.find((c) => existsSync4(join9(cwd2, c.file))); + const config2 = PYTHON_CONFIGS.find((c) => existsSync4(join10(cwd2, c.file))); if (!config2) { return { language: "python", @@ -124484,6 +124495,13 @@ function StartReviewTool(ctx) { 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}`); @@ -124512,8 +124530,7 @@ function StartReviewTool(ctx) { }; 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.` }; }) }); @@ -124535,24 +124552,51 @@ function AddReviewCommentTool(ctx) { if (!ctx.toolState.review) { 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=${path4}, line=${line}, side=${side || "RIGHT"}` + `adding review comment: reviewNodeId=${reviewNodeId}, path=${path4}, line=${line}, side=${side || "RIGHT"}` ); - const result = await ctx.octokit.graphql( - ADD_PULL_REQUEST_REVIEW_THREAD, - { - pullRequestReviewId: ctx.toolState.review.nodeId, - path: path4, - line, - body, - side: side || "RIGHT" - } - ); - log.debug(`review comment added: threadId=${result.addPullRequestReviewThread.thread.id}`); + let result; + try { + result = await ctx.octokit.graphql( + ADD_PULL_REQUEST_REVIEW_THREAD, + { + pullRequestReviewId: reviewNodeId, + path: path4, + line, + body, + side: side || "RIGHT" + } + ); + log.debug(`addPullRequestReviewThread response: ${JSON.stringify(result)}`); + } catch (error41) { + const errorMsg = error41 instanceof Error ? error41.message : String(error41); + log.debug(`addPullRequestReviewThread error: ${errorMsg}`); + throw new Error( + `Failed to add comment to ${path4}:${line}. GraphQL error: ${errorMsg}. Ensure the line is part of the diff and the path is correct.` + ); + } + if (!result) { + throw new Error( + `Failed to add comment to ${path4}:${line}. GraphQL returned null response.` + ); + } + if (!result.addPullRequestReviewThread) { + throw new Error( + `Failed to add comment to ${path4}:${line}. addPullRequestReviewThread is null. Response: ${JSON.stringify(result)}` + ); + } + if (!result.addPullRequestReviewThread.thread) { + throw new Error( + `Failed to add comment to ${path4}:${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 ${path4}:${line}`, - threadId: result.addPullRequestReviewThread.thread.id + threadId }; }) }); @@ -124594,6 +124638,10 @@ function SubmitReviewTool(ctx) { event: "COMMENT", 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}`); delete ctx.toolState.review; await deleteProgressComment(ctx); @@ -125248,7 +125296,7 @@ function resolveAgent({ return agent2; } async function createTempDirectory() { - const sharedTempDir = await mkdtemp2(join10(tmpdir2(), "pullfrog-")); + const sharedTempDir = await mkdtemp2(join11(tmpdir2(), "pullfrog-")); process.env.PULLFROG_TEMP_DIR = sharedTempDir; log.info(`\u{1F4C2} PULLFROG_TEMP_DIR has been created at ${sharedTempDir}`); return sharedTempDir; diff --git a/mcp/checkout.ts b/mcp/checkout.ts index bf65c3d..9d39715 100644 --- a/mcp/checkout.ts +++ b/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; }), }); diff --git a/mcp/review.ts b/mcp/review.ts index 1dc496d..3a8ae7f 100644 --- a/mcp/review.ts +++ b/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( - 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( + 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 diff --git a/utils/cli.ts b/utils/cli.ts index 8aa4b53..8bd1f8e 100644 --- a/utils/cli.ts +++ b/utils/cli.ts @@ -278,7 +278,7 @@ export const log = { /** * Print debug message (only if LOG_LEVEL=debug) */ - debug: (message: string): void => { + debug: (message: string | unknown): void => { if (isDebugEnabled()) { if (isGitHubActions) { // using this instead of core.debug