From 53970308ee1a00d772e8df8d4d2dccdaa5ba2891 Mon Sep 17 00:00:00 2001 From: David Blass Date: Sat, 28 Feb 2026 18:30:49 +0000 Subject: [PATCH] Add incremental re-review on new PR commits (#388) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * add incremental re-review on new PR commits When new commits are pushed to a PR that Pullfrog has previously reviewed, automatically perform a focused re-review on only the new changes. Includes a supersede mechanism to abort stale in-flight reviews on rapid pushes, a new IncrementalReview mode with incremental diff + prior-feedback awareness, and a PRReview tracking model so re-review fires for both auto-reviewed and manually-triggered PRs. Reviews now always submit (APPROVE when clean). Co-authored-by: Cursor * simplify re-review eligibility and add summary to incremental reviews Remove the path1/path2 distinction for re-review eligibility — now simply requires prReReview=enabled and a prior Pullfrog review on the PR. Show the re-review toggle regardless of prCreated setting. Add a top-level summary body to incremental reviews for consistency with full reviews. Co-authored-by: Cursor * replace superseded polling with server-side in-flight dedup and add prApproveEnabled setting Made-with: Cursor * add armstrong cursor command Made-with: Cursor * update armstrong * feat: Pullfrogger game v1 (#378) * Frogger game basis code (CC0 1.0 Unversal license). * Initial React port. * fix props for Sprite. * fixed context issue in FroggerGame. * Fixed format. * Fixed lint issue in FroggerGame. * Restoring LeapingLoader, using FroggerGame as a new fallback in Suspense. * feat: Display toast when URL ready. * Add props constraints on Sprite. * feat: frog sprite. * fix: zoom and alignment. * fix: extract const. * fix: mv types. * fix: mv game into index.tsx file. * fix: replacing deprecated event prop which with key. * feat: Log sprite. * feat: turtle sprite. * Adjusting game colors. * feat: sprites for the cars. * rm primitive sprites. * fix: bulldozer sprite position. * Adjusting colors. * Shape constraints. * rm original. * minor: naming, cleanup. * fix: renderers dict. * fix: adjusting and renaming racer. * fix renderer binding for scored frogs. * feat: responsive layout with gap below and maintained aspect ratio. * grammar fix. * feat: road lane dividers. * feat: using AbortController to cleanup events. * fix: cleanup and shortening. * feat: extracting drawGameBackground. * feat: initObstacleRows and updateAndDrawObstacles helpers. * feat: initFroggers helper. * feat: drawFroggers helper. * feat: checkForCollision helper. * feat: makeKeydownHandler helper. * feat: listenToKeyboardEvents helper. * mv cleanup into drawGameBackground. * fix: cleanup. * feat: better adjustBrightness helper. * Polling on the page.tsx side, restoring timeout and fallbacks, dynamic link msg. * FEAT: wait for workflow to complete and notify additionally with a big link (incl.db migration). * Fix: larger title, shorter link. * fix(hook): Writing completedAt from hook.workflow_run.updated_at according to suggestion. * fix(loader): rm unused props from WorkflowRunClientProps as suggested. * fix(loader): mv id=dev case into the page. * fix(DNRY): mv PollStartedResult and PollCompletedResult types. * fix(DNRY): reusing drawEllipse() helper in Sprite. * fix(style): reordering methods by priority in Sprite. * fix(frogger): rm empty rows from canvas, square game. * feat: game canvas rounded corners. * fix(DNRY): extracting SpriteShape type for faster reference. * fix: rm target from links, use same window. * add incremental re-review on new PR commits When new commits are pushed to a PR that Pullfrog has previously reviewed, automatically perform a focused re-review on only the new changes. Includes a supersede mechanism to abort stale in-flight reviews on rapid pushes, a new IncrementalReview mode with incremental diff + prior-feedback awareness, and a PRReview tracking model so re-review fires for both auto-reviewed and manually-triggered PRs. Reviews now always submit (APPROVE when clean). Co-authored-by: Cursor * replace superseded polling with server-side in-flight dedup and add prApproveEnabled setting Made-with: Cursor * consolidate workflow_run completed handling and track completedAt Removes the duplicate exported handleWorkflowRunCompleted in favor of the private one, merges status + orphan resolution logic into a single path, and sets completedAt on both normal completion and orphan cancel. Made-with: Cursor * fix rebase conflict resolution: restore eligibility logic, incremental review summaries, and exhaustiveness check - replace deleted hasPullfrogReviewedPR call with WorkflowRun.findFirst (the utility file was removed by the dedup improvements commit) - restore IncrementalReview summary body in modes.ts and selectMode.ts (lost during ca0168b conflict resolution; origin had re-added them via f98f902) - use switch + satisfies never for workflow_run event dispatch - lowercase comments per project conventions Made-with: Cursor * fix workflow-run polling architecture and improve incremental review prompts move polling loops from server actions to client to avoid serverless timeouts (pollForCompleted ran up to 600s in a single invocation). each server action is now a single DB check; client drives retries. also fix misleading prompt text about incremental diff scope and remove dead code in handleWebhook. Made-with: Cursor * await reportReviewNodeId to eliminate race condition and webhook sleep - refactor reportReviewNodeId from fire-and-forget to async/awaited, guaranteeing the dedup signal lands before the tool returns - remove the 5-second grace period sleep in the synchronize webhook handler (no longer needed with the awaited PATCH) - update IncrementalReview guidance to use get_review_comments for detailed prior line-level feedback instead of just review summaries - remove dead fallbackUrl field from CheckStartedResult type Made-with: Cursor * fix rebase artifacts: broken triggeringIssue reference, review formatting, and prompt wording Made-with: Cursor --------- Co-authored-by: Cursor Co-authored-by: Anna Bocharova --- entry | 120 ++++++++++++++++++++++++++++++++++++++++---- external.ts | 12 +++++ main.ts | 1 + mcp/review.ts | 47 ++++++++++++++++- mcp/selectMode.ts | 35 +++++++++++-- mcp/server.ts | 1 + modes.ts | 44 ++++++++++++++-- utils/body.ts | 1 + utils/payload.ts | 1 + utils/runContext.ts | 2 + 10 files changed, 247 insertions(+), 17 deletions(-) diff --git a/entry b/entry index 5b27472..ef086f9 100755 --- a/entry +++ b/entry @@ -143289,11 +143289,16 @@ function CreatePullRequestReviewTool(ctx) { parameters: CreatePullRequestReview, execute: execute(async ({ pull_number, body, approved, commit_id, comments = [] }) => { ctx.toolState.issueNumber = pull_number; + let event = approved ? "APPROVE" : "COMMENT"; + if (event === "APPROVE" && !ctx.prApproveEnabled) { + log.info("prApproveEnabled is disabled \u2014 downgrading APPROVE to COMMENT"); + event = "COMMENT"; + } const params = { owner: ctx.repo.owner, repo: ctx.repo.name, pull_number, - event: approved ? "APPROVE" : "COMMENT" + event }; if (body) params.body = body; if (commit_id) { @@ -143331,6 +143336,8 @@ function CreatePullRequestReviewTool(ctx) { throw new Error(`createReview returned invalid data: ${JSON.stringify(result.data)}`); } const reviewId = result.data.id; + const reviewNodeId = result.data.node_id; + await reportReviewNodeId(ctx, reviewNodeId); const customParts = []; if (!approved) { if (comments.length > 0) { @@ -143373,6 +143380,34 @@ function CreatePullRequestReviewTool(ctx) { }) }); } +async function reportReviewNodeId(ctx, reviewNodeId) { + for (let remaining = 2; remaining >= 0; remaining--) { + try { + const response = await apiFetch({ + path: `/api/workflow-run/${ctx.runId}`, + method: "PATCH", + headers: { + authorization: `Bearer ${ctx.apiToken}`, + "content-type": "application/json" + }, + body: JSON.stringify({ reviewNodeId }), + signal: AbortSignal.timeout(1e4) + }); + if (response.ok) return; + if (remaining > 0) { + log.debug(`reportReviewNodeId got ${response.status}, retrying (${remaining} left)`); + await new Promise((r) => setTimeout(r, 2e3)); + } + } catch (error49) { + if (remaining > 0) { + log.debug(`reportReviewNodeId failed, retrying (${remaining} left): ${error49}`); + await new Promise((r) => setTimeout(r, 2e3)); + } else { + log.debug(`reportReviewNodeId exhausted retries: ${error49}`); + } + } + } +} // mcp/reviewComments.ts import { writeFileSync as writeFileSync6 } from "node:fs"; @@ -143840,7 +143875,7 @@ function ResolveReviewThreadTool(ctx) { // mcp/selectMode.ts var SelectModeParams = type({ mode: type.string.describe( - "the name of the mode to select (e.g., 'Build', 'Plan', 'Review', 'Fix', 'AddressReviews', 'Task')" + "the name of the mode to select (e.g., 'Build', 'Plan', 'Review', 'IncrementalReview', 'Fix', 'AddressReviews', 'Task')" ) }); function resolveMode(modes2, modeName) { @@ -143919,9 +143954,37 @@ Each task in the \`tasks\` array should include: After all tasks complete, consolidate into a **single** review: - merge the \`comments\` arrays from all subagent outputs -- submit one \`${ghPullfrogMcpName}/create_pull_request_review\` with the merged comments and a unified summary body +- if subagents found actionable issues: submit one \`${ghPullfrogMcpName}/create_pull_request_review\` with \`approved: false\`, the merged comments, and a unified summary body +- if no subagent found actionable issues: submit with \`approved: true\` and a brief positive summary (no inline comments) +- call \`${ghPullfrogMcpName}/report_progress\` with the summary + +Use max effort for thorough reviews.`, + IncrementalReview: `### Checklist + +1. Checkout the PR via \`${ghPullfrogMcpName}/checkout_pr\` \u2014 this returns PR metadata and a \`diffPath\`. Read the diff to identify the major areas of change. +2. Generate the incremental diff using the \`before_sha\` from EVENT DATA: \`git diff ...HEAD\`. This isolates only the new commits. If the command fails (e.g., force-push rewrote history), fall back to reviewing the full PR diff. +3. Fetch previous reviews via \`${ghPullfrogMcpName}/list_pull_request_reviews\`. For the most recent Pullfrog review, call \`${ghPullfrogMcpName}/get_review_comments\` with the review ID to retrieve specific prior line-level feedback. Include the prior review summary and comment details when crafting subagent tasks. +4. Delegate multiple subagents in a single \`${ghPullfrogMcpName}/delegate\` call, each focused on a specific area of the new changes. Provide both the full diff path and the incremental diff. +5. After all subagents return, consolidate their findings into a single review. + +### Crafting each task + +Each task in the \`tasks\` array should include: +- the full diff file path AND the incremental diff (so the subagent can see both new changes and full context) +- what specific area/aspect to focus on +- instruct it to prioritize reviewing code in the incremental diff while using the full diff for context and to catch any changes not covered by the incremental diff +- include the prior review comments (from step 3) so the subagent knows what feedback was already given \u2014 instruct it to avoid repeating prior issues and to note whether prior feedback was addressed by the new commits +- instruct it to actively hunt for problems: trace data flow, check boundaries, explore failure modes, verify assumptions, consider lifecycle, spot performance issues +- draft inline comments with NEW line numbers from the full PR diff \u2014 every comment must be actionable (2-3 sentences max) +- call \`${ghPullfrogMcpName}/set_output\` with a JSON object: \`{ "summary": "...", "comments": [{ "path": "file.ts", "line": 42, "body": "..." }, ...] }\` + +### Post-delegation + +After all tasks complete, consolidate into a **single** review: +- merge the \`comments\` arrays from all subagent outputs +- if subagents found actionable issues: submit one \`${ghPullfrogMcpName}/create_pull_request_review\` with \`approved: false\`, the merged comments, and a unified summary body +- if no subagent found actionable issues: submit with \`approved: true\` and a brief positive summary (no inline comments) - call \`${ghPullfrogMcpName}/report_progress\` with the summary -- if no subagent found actionable issues, skip the review \u2014 just call \`report_progress\` noting the PR was reviewed Use max effort for thorough reviews.`, Plan: `### Checklist @@ -144592,7 +144655,7 @@ Keep the progress comment extremely brief. The summary should be 1-2 sentences m { name: "Review", description: "Review code, PRs, or implementations; provide feedback or suggestions; identify issues; or check code quality, style, and correctness", - prompt: `Follow these steps to review the PR. Your job is to find problems\u2014assume they exist until you've proven otherwise. Do not submit a clean review without thorough investigation. **If you have nothing interesting to say, do NOT submit a review at all\u2014use \`report_progress\` instead.** + prompt: `Follow these steps to review the PR. Your job is to find problems\u2014assume they exist until you've proven otherwise. Do not submit a clean review without thorough investigation. 1. **CHECKOUT** - Call ${ghPullfrogMcpName}/checkout_pr with the PR number. This should give you all PR metadata you need, including a \`diffPath\`: a path to a temp file containing the PR diff. @@ -144610,15 +144673,50 @@ Keep the progress comment extremely brief. The summary should be 1-2 sentences m - **Check PR consistency**: Does the PR title/description match the actual code changes? Flag significant discrepancies. - Do NOT stop at "this looks reasonable." Dig until you either find a problem or have concrete evidence there isn't one. -4. **DRAFT LINE-BY-LINE COMMENTS** - Every comment must be actionable: the author should need to change something in response. 2-3 sentences max. Use the NEW line number from the diff (second column: \`| OLD | NEW | TYPE | CODE\`). If no issues found, skip to step 5. NO COMPLIMENTS. NO NITPICKING ABOUT CHANGES UNRELATED TO THE MAIN CHANGE. Non-actionable comments (praise, style preferences, minor optimizatfixons, documentation nits) must not be drafted. If no comments survive and you have no significant concerns, **do not submit a review**. Use \`${ghPullfrogMcpName}/report_progress\` to note the PR was reviewed and no issues were found. +4. **DRAFT LINE-BY-LINE COMMENTS** - Every comment must be actionable: the author should need to change something in response. 2-3 sentences max. Use the NEW line number from the diff (second column: \`| OLD | NEW | TYPE | CODE\`). If no issues found, skip to step 5. NO COMPLIMENTS. NO NITPICKING ABOUT CHANGES UNRELATED TO THE MAIN CHANGE. Non-actionable comments (praise, style preferences, minor optimizatfixons, documentation nits) must not be drafted. -5. **WRITE SUMMARY** - Draft a 1-3 sentence summary for the review body. Include urgency level and any concerns about code outside the diff. +5. **WRITE SUMMARY** - Draft a 1-3 sentence summary for the review body. If issues were found, include urgency level and any concerns about code outside the diff. If no issues were found, write a brief approval summary (e.g., "Changes look good. No issues found."). -6. **SUBMIT** \u2014 Use ${ghPullfrogMcpName}/create_pull_request_review: +6. **SUBMIT** \u2014 Always submit a review via ${ghPullfrogMcpName}/create_pull_request_review: - \`body\`: The summary from step 5 - \`comments\`: The inline comments from step 4 - \`approved\`: Set to \`true\` ONLY if the review contains no actionable feedback \u2014 neither inline comments nor actionable content in the body. An approval signals "no changes needed." +${permalinkTip} +` + }, + { + name: "IncrementalReview", + description: "Re-review a PR after new commits are pushed; focus on new changes since the last review", + prompt: `Follow these steps to incrementally re-review the PR after new commits were pushed. Focus on what changed since the last review. + +1. **CHECKOUT** - Call ${ghPullfrogMcpName}/checkout_pr with the PR number. This gives you the full PR diff via \`diffPath\`. + +2. **INCREMENTAL DIFF** - EVENT DATA contains \`before_sha\` (the HEAD before this push). Generate the incremental diff: + \`git diff ...HEAD\` + This shows the changes introduced by this push. Cross-reference with previous reviews (step 3) to confirm coverage of all unreviewed changes \u2014 the full PR diff fills any gaps. + **If the diff command fails** (e.g., force-push rewrote history), fall back to reviewing the full PR diff from step 1. + +3. **FETCH PREVIOUS REVIEWS** - Use ${ghPullfrogMcpName}/list_pull_request_reviews to find previous Pullfrog reviews. For the most recent one, call ${ghPullfrogMcpName}/get_review_comments with the review ID to see specific line-level feedback. This lets you avoid repeating issues and assess whether prior feedback was addressed by the new commits. + +4. **ANALYZE** - Read the incremental diff to understand the new changes. Use the full PR diff for surrounding context and to catch any changes not covered by the incremental diff. + - **Understand the change**: What is new or modified since the last review? + - **Evaluate the approach**: Are the new changes sound? Do they address prior feedback? + +5. **INVESTIGATE** - Hunt for problems in the new code using the same techniques as a full review: + - Trace data flow, check boundaries, explore failure modes, verify assumptions, consider lifecycle, spot performance issues. + - Focus investigation on code that changed in the incremental diff, but trace its effects through the broader codebase. + - Do NOT repeat feedback already given in previous reviews unless it was not addressed. + +6. **DRAFT LINE-BY-LINE COMMENTS** - Every comment must be actionable. 2-3 sentences max. Use the NEW line number from the full PR diff. NO COMPLIMENTS. NO NITPICKING. + +7. **WRITE SUMMARY** - Draft a 1-3 sentence summary for the review body. Focus on what changed since the last review and whether the new changes are sound. If issues were found, include urgency level. If no issues were found, write a brief approval summary. + +8. **SUBMIT** \u2014 Use ${ghPullfrogMcpName}/create_pull_request_review: + - \`body\`: The summary from step 7 + - \`comments\`: The inline comments from step 6 + - \`approved\`: Set to \`true\` ONLY if the review contains no actionable feedback \u2014 neither inline comments nor actionable content in the body. An approval signals "no changes needed." + ${permalinkTip} ` }, @@ -146828,6 +146926,7 @@ async function fetchBodyHtml(ctx) { })).data.body_html; case "pull_request_opened": case "pull_request_ready_for_review": + case "pull_request_synchronize": case "pull_request_review_requested": if (!event.issue_number) return; return (await ctx.octokit.rest.issues.get({ @@ -147422,6 +147521,7 @@ function resolvePayload(resolvedPromptInput, repoSettings) { effort: inputs.effort ?? jsonPayload?.effort ?? "auto", timeout: inputs.timeout ?? jsonPayload?.timeout, cwd: resolveCwd(inputs.cwd), + progressCommentId: jsonPayload?.progressCommentId, debug: jsonPayload?.debug, // permissions: inputs > repoSettings > fallbacks web: inputs.web ?? repoSettings.web ?? "enabled", @@ -147473,7 +147573,8 @@ var defaultSettings = { web: "enabled", search: "enabled", push: "restricted", - shell: "restricted" + shell: "restricted", + prApproveEnabled: false }; var defaultRunContext = { settings: defaultSettings, @@ -147741,6 +147842,7 @@ async function main() { agent: agent2, modes: modes2, postCheckoutScript: runContext.repoSettings.postCheckoutScript, + prApproveEnabled: runContext.repoSettings.prApproveEnabled, toolState, runId: runInfo.runId, jobId: runInfo.jobId, diff --git a/external.ts b/external.ts index 5acfcbc..3a25734 100644 --- a/external.ts +++ b/external.ts @@ -241,6 +241,17 @@ interface ImplementPlanEvent extends BasePayloadEvent { body: string | null; } +interface PullRequestSynchronizeEvent extends BasePayloadEvent { + trigger: "pull_request_synchronize"; + issue_number: number; + is_pr: true; + title: string; + body: string | null; + branch: string; + /** SHA before the push -- used to compute incremental diff via `git diff before_sha...HEAD` */ + before_sha: string; +} + interface UnknownEvent extends BasePayloadEvent { trigger: "unknown"; } @@ -250,6 +261,7 @@ interface UnknownEvent extends BasePayloadEvent { export type PayloadEvent = | PullRequestOpenedEvent | PullRequestReadyForReviewEvent + | PullRequestSynchronizeEvent | PullRequestReviewRequestedEvent | PullRequestReviewSubmittedEvent | PullRequestReviewCommentCreatedEvent diff --git a/main.ts b/main.ts index fc475ee..a2b3fbe 100644 --- a/main.ts +++ b/main.ts @@ -155,6 +155,7 @@ export async function main(): Promise { agent, modes, postCheckoutScript: runContext.repoSettings.postCheckoutScript, + prApproveEnabled: runContext.repoSettings.prApproveEnabled, toolState, runId: runInfo.runId, jobId: runInfo.jobId, diff --git a/mcp/review.ts b/mcp/review.ts index 4b0aa13..a31a701 100644 --- a/mcp/review.ts +++ b/mcp/review.ts @@ -1,5 +1,6 @@ import type { RestEndpointMethodTypes } from "@octokit/rest"; import { type } from "arktype"; +import { apiFetch } from "../utils/apiFetch.ts"; import { getApiUrl } from "../utils/apiUrl.ts"; import { buildPullfrogFooter } from "../utils/buildPullfrogFooter.ts"; import { log } from "../utils/cli.ts"; @@ -73,12 +74,19 @@ export function CreatePullRequestReviewTool(ctx: ToolContext) { // set issue context (PRs are issues) ctx.toolState.issueNumber = pull_number; + // enforce prApproveEnabled: downgrade APPROVE to COMMENT if disabled + let event: "APPROVE" | "COMMENT" = approved ? "APPROVE" : "COMMENT"; + if (event === "APPROVE" && !ctx.prApproveEnabled) { + log.info("prApproveEnabled is disabled — downgrading APPROVE to COMMENT"); + event = "COMMENT"; + } + // compose the request const params: RestEndpointMethodTypes["pulls"]["createReview"]["parameters"] = { owner: ctx.repo.owner, repo: ctx.repo.name, pull_number, - event: approved ? "APPROVE" : "COMMENT", + event, }; if (body) params.body = body; if (commit_id) { @@ -115,12 +123,20 @@ export function CreatePullRequestReviewTool(ctx: ToolContext) { 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; + const reviewNodeId = result.data.node_id; + + // report review node ID to server so the in-flight dedup check + // in the synchronize webhook handler sees this run as "review submitted." + // awaited (not fire-and-forget) to guarantee the signal lands before + // any subsequent push's webhook checks for in-flight runs. + await reportReviewNodeId(ctx, reviewNodeId); // build quick links footer and update the review body // only include "Fix all" and "Fix 👍s" links if there are actual review comments @@ -175,6 +191,35 @@ export function CreatePullRequestReviewTool(ctx: ToolContext) { }); } +async function reportReviewNodeId(ctx: ToolContext, reviewNodeId: string): Promise { + for (let remaining = 2; remaining >= 0; remaining--) { + try { + const response = await apiFetch({ + path: `/api/workflow-run/${ctx.runId}`, + method: "PATCH", + headers: { + authorization: `Bearer ${ctx.apiToken}`, + "content-type": "application/json", + }, + body: JSON.stringify({ reviewNodeId }), + signal: AbortSignal.timeout(10_000), + }); + if (response.ok) return; + if (remaining > 0) { + log.debug(`reportReviewNodeId got ${response.status}, retrying (${remaining} left)`); + await new Promise((r) => setTimeout(r, 2000)); + } + } catch (error) { + if (remaining > 0) { + log.debug(`reportReviewNodeId failed, retrying (${remaining} left): ${error}`); + await new Promise((r) => setTimeout(r, 2000)); + } else { + log.debug(`reportReviewNodeId exhausted retries: ${error}`); + } + } + } +} + // ============================================================================= // 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, diff --git a/mcp/selectMode.ts b/mcp/selectMode.ts index b735ebd..39b447d 100644 --- a/mcp/selectMode.ts +++ b/mcp/selectMode.ts @@ -6,7 +6,7 @@ import { execute, tool } from "./shared.ts"; export const SelectModeParams = type({ mode: type.string.describe( - "the name of the mode to select (e.g., 'Build', 'Plan', 'Review', 'Fix', 'AddressReviews', 'Task')" + "the name of the mode to select (e.g., 'Build', 'Plan', 'Review', 'IncrementalReview', 'Fix', 'AddressReviews', 'Task')" ), }); @@ -89,9 +89,38 @@ Each task in the \`tasks\` array should include: After all tasks complete, consolidate into a **single** review: - merge the \`comments\` arrays from all subagent outputs -- submit one \`${ghPullfrogMcpName}/create_pull_request_review\` with the merged comments and a unified summary body +- if subagents found actionable issues: submit one \`${ghPullfrogMcpName}/create_pull_request_review\` with \`approved: false\`, the merged comments, and a unified summary body +- if no subagent found actionable issues: submit with \`approved: true\` and a brief positive summary (no inline comments) +- call \`${ghPullfrogMcpName}/report_progress\` with the summary + +Use max effort for thorough reviews.`, + + IncrementalReview: `### Checklist + +1. Checkout the PR via \`${ghPullfrogMcpName}/checkout_pr\` — this returns PR metadata and a \`diffPath\`. Read the diff to identify the major areas of change. +2. Generate the incremental diff using the \`before_sha\` from EVENT DATA: \`git diff ...HEAD\`. This isolates only the new commits. If the command fails (e.g., force-push rewrote history), fall back to reviewing the full PR diff. +3. Fetch previous reviews via \`${ghPullfrogMcpName}/list_pull_request_reviews\`. For the most recent Pullfrog review, call \`${ghPullfrogMcpName}/get_review_comments\` with the review ID to retrieve specific prior line-level feedback. Include the prior review summary and comment details when crafting subagent tasks. +4. Delegate multiple subagents in a single \`${ghPullfrogMcpName}/delegate\` call, each focused on a specific area of the new changes. Provide both the full diff path and the incremental diff. +5. After all subagents return, consolidate their findings into a single review. + +### Crafting each task + +Each task in the \`tasks\` array should include: +- the full diff file path AND the incremental diff (so the subagent can see both new changes and full context) +- what specific area/aspect to focus on +- instruct it to prioritize reviewing code in the incremental diff while using the full diff for context and to catch any changes not covered by the incremental diff +- include the prior review comments (from step 3) so the subagent knows what feedback was already given — instruct it to avoid repeating prior issues and to note whether prior feedback was addressed by the new commits +- instruct it to actively hunt for problems: trace data flow, check boundaries, explore failure modes, verify assumptions, consider lifecycle, spot performance issues +- draft inline comments with NEW line numbers from the full PR diff — every comment must be actionable (2-3 sentences max) +- call \`${ghPullfrogMcpName}/set_output\` with a JSON object: \`{ "summary": "...", "comments": [{ "path": "file.ts", "line": 42, "body": "..." }, ...] }\` + +### Post-delegation + +After all tasks complete, consolidate into a **single** review: +- merge the \`comments\` arrays from all subagent outputs +- if subagents found actionable issues: submit one \`${ghPullfrogMcpName}/create_pull_request_review\` with \`approved: false\`, the merged comments, and a unified summary body +- if no subagent found actionable issues: submit with \`approved: true\` and a brief positive summary (no inline comments) - call \`${ghPullfrogMcpName}/report_progress\` with the summary -- if no subagent found actionable issues, skip the review — just call \`report_progress\` noting the PR was reviewed Use max effort for thorough reviews.`, diff --git a/mcp/server.ts b/mcp/server.ts index 532c8a5..f5cda13 100644 --- a/mcp/server.ts +++ b/mcp/server.ts @@ -99,6 +99,7 @@ export interface ToolContext { agent: Agent; modes: Mode[]; postCheckoutScript: string | null; + prApproveEnabled: boolean; toolState: ToolState; runId: number | undefined; jobId: string | undefined; diff --git a/modes.ts b/modes.ts index 1e104b7..b3c86d0 100644 --- a/modes.ts +++ b/modes.ts @@ -101,7 +101,7 @@ Keep the progress comment extremely brief. The summary should be 1-2 sentences m name: "Review", description: "Review code, PRs, or implementations; provide feedback or suggestions; identify issues; or check code quality, style, and correctness", - prompt: `Follow these steps to review the PR. Your job is to find problems—assume they exist until you've proven otherwise. Do not submit a clean review without thorough investigation. **If you have nothing interesting to say, do NOT submit a review at all—use \`report_progress\` instead.** + prompt: `Follow these steps to review the PR. Your job is to find problems—assume they exist until you've proven otherwise. Do not submit a clean review without thorough investigation. 1. **CHECKOUT** - Call ${ghPullfrogMcpName}/checkout_pr with the PR number. This should give you all PR metadata you need, including a \`diffPath\`: a path to a temp file containing the PR diff. @@ -119,15 +119,51 @@ Keep the progress comment extremely brief. The summary should be 1-2 sentences m - **Check PR consistency**: Does the PR title/description match the actual code changes? Flag significant discrepancies. - Do NOT stop at "this looks reasonable." Dig until you either find a problem or have concrete evidence there isn't one. -4. **DRAFT LINE-BY-LINE COMMENTS** - Every comment must be actionable: the author should need to change something in response. 2-3 sentences max. Use the NEW line number from the diff (second column: \`| OLD | NEW | TYPE | CODE\`). If no issues found, skip to step 5. NO COMPLIMENTS. NO NITPICKING ABOUT CHANGES UNRELATED TO THE MAIN CHANGE. Non-actionable comments (praise, style preferences, minor optimizatfixons, documentation nits) must not be drafted. If no comments survive and you have no significant concerns, **do not submit a review**. Use \`${ghPullfrogMcpName}/report_progress\` to note the PR was reviewed and no issues were found. +4. **DRAFT LINE-BY-LINE COMMENTS** - Every comment must be actionable: the author should need to change something in response. 2-3 sentences max. Use the NEW line number from the diff (second column: \`| OLD | NEW | TYPE | CODE\`). If no issues found, skip to step 5. NO COMPLIMENTS. NO NITPICKING ABOUT CHANGES UNRELATED TO THE MAIN CHANGE. Non-actionable comments (praise, style preferences, minor optimizatfixons, documentation nits) must not be drafted. -5. **WRITE SUMMARY** - Draft a 1-3 sentence summary for the review body. Include urgency level and any concerns about code outside the diff. +5. **WRITE SUMMARY** - Draft a 1-3 sentence summary for the review body. If issues were found, include urgency level and any concerns about code outside the diff. If no issues were found, write a brief approval summary (e.g., "Changes look good. No issues found."). -6. **SUBMIT** — Use ${ghPullfrogMcpName}/create_pull_request_review: +6. **SUBMIT** — Always submit a review via ${ghPullfrogMcpName}/create_pull_request_review: - \`body\`: The summary from step 5 - \`comments\`: The inline comments from step 4 - \`approved\`: Set to \`true\` ONLY if the review contains no actionable feedback — neither inline comments nor actionable content in the body. An approval signals "no changes needed." +${permalinkTip} +`, + }, + { + name: "IncrementalReview", + description: + "Re-review a PR after new commits are pushed; focus on new changes since the last review", + prompt: `Follow these steps to incrementally re-review the PR after new commits were pushed. Focus on what changed since the last review. + +1. **CHECKOUT** - Call ${ghPullfrogMcpName}/checkout_pr with the PR number. This gives you the full PR diff via \`diffPath\`. + +2. **INCREMENTAL DIFF** - EVENT DATA contains \`before_sha\` (the HEAD before this push). Generate the incremental diff: + \`git diff ...HEAD\` + This shows the changes introduced by this push. Cross-reference with previous reviews (step 3) to confirm coverage of all unreviewed changes — the full PR diff fills any gaps. + **If the diff command fails** (e.g., force-push rewrote history), fall back to reviewing the full PR diff from step 1. + +3. **FETCH PREVIOUS REVIEWS** - Use ${ghPullfrogMcpName}/list_pull_request_reviews to find previous Pullfrog reviews. For the most recent one, call ${ghPullfrogMcpName}/get_review_comments with the review ID to see specific line-level feedback. This lets you avoid repeating issues and assess whether prior feedback was addressed by the new commits. + +4. **ANALYZE** - Read the incremental diff to understand the new changes. Use the full PR diff for surrounding context and to catch any changes not covered by the incremental diff. + - **Understand the change**: What is new or modified since the last review? + - **Evaluate the approach**: Are the new changes sound? Do they address prior feedback? + +5. **INVESTIGATE** - Hunt for problems in the new code using the same techniques as a full review: + - Trace data flow, check boundaries, explore failure modes, verify assumptions, consider lifecycle, spot performance issues. + - Focus investigation on code that changed in the incremental diff, but trace its effects through the broader codebase. + - Do NOT repeat feedback already given in previous reviews unless it was not addressed. + +6. **DRAFT LINE-BY-LINE COMMENTS** - Every comment must be actionable. 2-3 sentences max. Use the NEW line number from the full PR diff. NO COMPLIMENTS. NO NITPICKING. + +7. **WRITE SUMMARY** - Draft a 1-3 sentence summary for the review body. Focus on what changed since the last review and whether the new changes are sound. If issues were found, include urgency level. If no issues were found, write a brief approval summary. + +8. **SUBMIT** — Use ${ghPullfrogMcpName}/create_pull_request_review: + - \`body\`: The summary from step 7 + - \`comments\`: The inline comments from step 6 + - \`approved\`: Set to \`true\` ONLY if the review contains no actionable feedback — neither inline comments nor actionable content in the body. An approval signals "no changes needed." + ${permalinkTip} `, }, diff --git a/utils/body.ts b/utils/body.ts index 9e357f5..1c87dd4 100644 --- a/utils/body.ts +++ b/utils/body.ts @@ -72,6 +72,7 @@ async function fetchBodyHtml(ctx: ResolveBodyContext): Promise repoSettings > fallbacks diff --git a/utils/runContext.ts b/utils/runContext.ts index e5c39ab..3f123f4 100644 --- a/utils/runContext.ts +++ b/utils/runContext.ts @@ -19,6 +19,7 @@ export interface RepoSettings { search: ToolPermission; push: PushPermission; shell: ShellPermission; + prApproveEnabled: boolean; } export interface RunContext { @@ -36,6 +37,7 @@ const defaultSettings: RepoSettings = { search: "enabled", push: "restricted", shell: "restricted", + prApproveEnabled: false, }; const defaultRunContext: RunContext = {