diff --git a/agents/shared.ts b/agents/shared.ts index 0ee3d75..fb3c96b 100644 --- a/agents/shared.ts +++ b/agents/shared.ts @@ -31,10 +31,10 @@ export const instructions = `- use the ${ghPullfrogMcpName} MCP server to intera - if prompted by a comment to respond to create a new issue, pr or anything else, after succeeding, also respond to the original comment with a very brief message containing a link to it - if prompted to review a PR: - (1) get PR info with mcp__${ghPullfrogMcpName}__get_pull_request - (2) fetch both branches: git fetch origin --depth=20 && git fetch origin - (3) checkout the PR branch: git checkout origin/ (you MUST do this before reading any files) - (4) view diff: git diff origin/...origin/ (this shows what changed) - (5) read files from the checked-out PR branch to understand the implementation + (1) get PR info with mcp__${ghPullfrogMcpName}__get_pull_request (this automatically prepares the repository by fetching and checking out the PR branch) + (2) view diff: git diff origin/...origin/ (use line numbers from this for inline comments) + (3) read files from the checked-out PR branch to understand the implementation + (4) when submitting review: use the 'comments' array for ALL specific code issues - include the file path and line position from the diff + (5) only use the 'body' field for a brief summary (1-2 sentences) or for feedback that doesn't apply to a specific code location replace and with 'base' and 'head' from the PR info `; diff --git a/index.ts b/index.ts index 044ac94..5b2b33a 100644 --- a/index.ts +++ b/index.ts @@ -3,8 +3,7 @@ * This exports the main function for programmatic usage */ -export { ClaudeAgent } from "./agents/claude.ts"; -export type { Agent, AgentConfig, AgentResult } from "./agents/types.ts"; +export type { Agent, AgentConfig, AgentResult } from "./agents/shared.ts"; export { type Inputs as ExecutionInputs, type MainResult, diff --git a/mcp/prInfo.ts b/mcp/prInfo.ts index 8c5ee3f..22e13b6 100644 --- a/mcp/prInfo.ts +++ b/mcp/prInfo.ts @@ -1,4 +1,6 @@ +import { execSync } from "node:child_process"; import { type } from "arktype"; +import { log } from "../utils/cli.ts"; import { contextualize, tool } from "./shared.ts"; export const PullRequestInfo = type({ @@ -7,7 +9,8 @@ export const PullRequestInfo = type({ export const PullRequestInfoTool = tool({ name: "get_pull_request", - description: "Retrieve minimal information for a specific pull request by number.", + description: + "Retrieve PR information and automatically prepare the repository for review by fetching and checking out the PR branch.", parameters: PullRequestInfo, execute: contextualize(async ({ pull_number }, ctx) => { const pr = await ctx.octokit.rest.pulls.get({ @@ -21,6 +24,20 @@ export const PullRequestInfoTool = tool({ const baseBranch = data.base.ref; const headBranch = data.head.ref; + if (!baseBranch) { + throw new Error(`Base branch not found for PR #${pull_number}`); + } + + // Automatically fetch and checkout branches for review + log.info(`Fetching base branch: origin/${baseBranch}`); + execSync(`git fetch origin ${baseBranch} --depth=20`, { stdio: "inherit" }); + + log.info(`Fetching PR branch: origin/${headBranch}`); + execSync(`git fetch origin ${headBranch}`, { stdio: "inherit" }); + + log.info(`Checking out PR branch: origin/${headBranch}`); + execSync(`git checkout origin/${headBranch}`, { stdio: "inherit" }); + return { number: data.number, url: data.html_url, diff --git a/mcp/review.ts b/mcp/review.ts index 5563fc7..06d3737 100644 --- a/mcp/review.ts +++ b/mcp/review.ts @@ -8,27 +8,51 @@ export const Review = type({ .enumerated("APPROVE", "REQUEST_CHANGES", "COMMENT") .describe("'APPROVE', 'REQUEST_CHANGES', or 'COMMENT' (the review action)"), body: type.string - .describe("The body content for the review (required for REQUEST_CHANGES or COMMENT)") + .describe( + "Brief summary or general feedback that doesn't apply to specific code locations. Keep it concise - most feedback should be in the 'comments' array." + ) .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"), - position: type.number.describe("The diff position in the file"), - body: type.string.describe("The comment text"), + 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"), + start_line: type.number + .describe("Start line for multi-line comments (optional, for commenting on ranges)") + .optional(), }) .array() - .describe("Array of draft review comments for specific lines, optional.") + .describe( + "REQUIRED: Array of inline comments for specific code issues. Use this for all location-specific feedback. Use 'git diff origin/...origin/' to find the correct line numbers (typically use the line numbers shown on the RIGHT side for new code, LEFT side for old code)." + ) .optional(), }); export const ReviewTool = tool({ name: "submit_pull_request_review", description: - "Submit a review (approve, request changes, or comment) for an existing pull request.", + "Submit a review (approve, request changes, or comment) for an existing pull request. " + + "IMPORTANT: Use 'comments' array for ALL specific code issues at the line-level. " + + "Only use 'body' for a brief summary or feedback that doesn't apply to a specific location.", parameters: Review, execute: contextualize(async ({ pull_number, event, body, commit_id, comments = [] }, ctx) => { + // 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, @@ -37,8 +61,26 @@ export const ReviewTool = tool({ event, }; if (body) params.body = body; - if (commit_id) params.commit_id = commit_id; - if (comments.length > 0) params.comments = comments; + 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); return { success: true, diff --git a/package.json b/package.json index a7ec14f..93ce317 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@pullfrog/action", - "version": "0.0.67", + "version": "0.0.68", "type": "module", "files": [ "index.js", @@ -22,12 +22,12 @@ "dependencies": { "@actions/core": "^1.11.1", "@anthropic-ai/claude-agent-sdk": "^0.1.26", - "@ark/fs": "0.50.0", - "@ark/util": "0.50.0", + "@ark/fs": "0.53.0", + "@ark/util": "0.53.0", "@octokit/rest": "^22.0.0", "@octokit/webhooks-types": "^7.6.1", "@standard-schema/spec": "1.0.0", - "arktype": "^2.1.25", + "arktype": "2.1.25", "dotenv": "^17.2.3", "execa": "^9.6.0", "fastmcp": "^3.20.0", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 8e2a9bd..fe98658 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -15,11 +15,11 @@ importers: specifier: ^0.1.26 version: 0.1.30(zod@3.25.76) '@ark/fs': - specifier: 0.50.0 - version: 0.50.0 + specifier: 0.53.0 + version: 0.53.0 '@ark/util': - specifier: 0.50.0 - version: 0.50.0 + specifier: 0.53.0 + version: 0.53.0 '@octokit/rest': specifier: ^22.0.0 version: 22.0.0 @@ -30,7 +30,7 @@ importers: specifier: 1.0.0 version: 1.0.0 arktype: - specifier: ^2.1.25 + specifier: 2.1.25 version: 2.1.25 dotenv: specifier: ^17.2.3 @@ -75,15 +75,12 @@ packages: peerDependencies: zod: ^3.24.1 - '@ark/fs@0.50.0': - resolution: {integrity: sha512-6OrxNt2T+/pL4RUMZK/aiVRLIS3acNs5uSpHDyZAP6+OXwXchFSQ1lJTH+uuGBlDWeQxPD7VmymYXLF5s1Eyhw==} + '@ark/fs@0.53.0': + resolution: {integrity: sha512-XL0EbBAZgyy+j9aPhftYaBsbKAW5PTNSKCN6oLRRdrHuHPSAZgR6765/z0YZGhPxHEUNmq0vBoSk8yOLk91dNQ==} '@ark/schema@0.53.0': resolution: {integrity: sha512-1PB7RThUiTlmIu8jbSurPrhHpVixPd4C+xNBUF/HrjIENCeDcAMg36n5mpMzED7OQGDVIzpfXXiMnaTiutjHJw==} - '@ark/util@0.50.0': - resolution: {integrity: sha512-tIkgIMVRpkfXRQIEf0G2CJryZVtHVrqcWHMDa5QKo0OEEBu0tHkRSIMm4Ln8cd8Bn9TPZtvc/kE2Gma8RESPSg==} - '@ark/util@0.53.0': resolution: {integrity: sha512-TGn4gLlA6dJcQiqrtCtd88JhGb2XBHo6qIejsDre+nxpGuUVW4G3YZGVrwjNBTO0EyR+ykzIo4joHJzOj+/cpA==} @@ -879,14 +876,12 @@ snapshots: '@img/sharp-linux-x64': 0.33.5 '@img/sharp-win32-x64': 0.33.5 - '@ark/fs@0.50.0': {} + '@ark/fs@0.53.0': {} '@ark/schema@0.53.0': dependencies: '@ark/util': 0.53.0 - '@ark/util@0.50.0': {} - '@ark/util@0.53.0': {} '@borewit/text-codec@0.1.1': {}