ShockVPN/shockbot
0
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Update review process

Browse Source
This commit is contained in:
Colin McDonnell
2025-12-21 22:23:18 -08:00
parent b33deb1b5a
commit d5bec7499b
6 changed files with 16273 additions and 9244 deletions
Download Patch File Download Diff File Expand all files Collapse all files
entry
+16230 -9208
View File
File diff suppressed because one or more lines are too long
mcp/checkout.ts
+10
View File
@@ -19,6 +19,7 @@ export type CheckoutPrResult = {
maintainerCanModify: boolean; maintainerCanModify: boolean;
url: string; url: string;
headRepo: string; headRepo: string;
diff: string;
}; };
interface CheckoutPrBranchParams { interface CheckoutPrBranchParams {
@@ -157,6 +158,14 @@ export function CheckoutPrTool(ctx: ToolContext) {
throw new Error(`PR #${pull_number} source repository was deleted`); throw new Error(`PR #${pull_number} source repository was deleted`);
} }
// fetch PR diff via API (authoritative source - not affected by main advancing)
const diffResponse = await ctx.octokit.rest.pulls.get({
owner: ctx.owner,
repo: ctx.name,
pull_number,
mediaType: { format: "diff" },
});
return { return {
success: true, success: true,
number: pr.data.number, number: pr.data.number,
@@ -167,6 +176,7 @@ export function CheckoutPrTool(ctx: ToolContext) {
maintainerCanModify: pr.data.maintainer_can_modify, maintainerCanModify: pr.data.maintainer_can_modify,
url: pr.data.html_url, url: pr.data.html_url,
headRepo: headRepo.full_name, headRepo: headRepo.full_name,
diff: diffResponse.data as unknown as string,
} satisfies CheckoutPrResult; } satisfies CheckoutPrResult;
}), }),
}); });
mcp/review.ts
+15 -16
View File
@@ -1,6 +1,3 @@
import { randomBytes } from "node:crypto";
import { writeFileSync } from "node:fs";
import { join } from "node:path";
import type { RestEndpointMethodTypes } from "@octokit/rest"; import type { RestEndpointMethodTypes } from "@octokit/rest";
import { type } from "arktype"; import { type } from "arktype";
import type { ToolContext } from "../main.ts"; import type { ToolContext } from "../main.ts";
@@ -65,7 +62,7 @@ export function StartReviewTool(ctx: ToolContext) {
return tool({ return tool({
name: "start_review", name: "start_review",
description: description:
"Start a new review session for a pull request. Creates a scratchpad file for gathering thoughts and a pending review on GitHub. Must be called before add_review_comment.", "Start a new review session for a pull request. Creates a pending review on GitHub. Must be called before add_review_comment.",
parameters: StartReview, parameters: StartReview,
execute: execute(async ({ pull_number }) => { execute: execute(async ({ pull_number }) => {
// check if review already started in this session // check if review already started in this session
@@ -119,12 +116,6 @@ export function StartReviewTool(ctx: ToolContext) {
} }
} }
// create scratchpad file
const scratchpadId = randomBytes(4).toString("hex");
const scratchpadPath = join(ctx.sharedTempDir, `pullfrog-review-${scratchpadId}.md`);
const scratchpadContent = `# Review ${scratchpadId}\n\n`;
writeFileSync(scratchpadPath, scratchpadContent);
// set PR context and review state // set PR context and review state
ctx.toolState.prNumber = pull_number; ctx.toolState.prNumber = pull_number;
ctx.toolState.review = { ctx.toolState.review = {
@@ -133,9 +124,18 @@ export function StartReviewTool(ctx: ToolContext) {
}; };
return { return {
reviewId: scratchpadId, message: `Review session started for PR #${pull_number}.`,
scratchpadPath, guidance: {
message: `Review session started. Use the scratchpad file to gather your thoughts, then call add_review_comment for each comment.`, analyze: [
"What does this PR change? Summarize in 1-2 sentences.",
"Is the approach sound? If not, stop and comment on approach first.",
"What bugs, edge cases, or security issues exist?",
],
beforeCommenting: [
"Skip nitpicks unless explicitly requested.",
"Would the codebase maintainer care about this feedback, based on what you can infer about the code quality standards in this repo?",
],
},
}; };
}), }),
}); });
@@ -281,9 +281,8 @@ export const Review = type({
}) })
.array() .array()
.describe( .describe(
// FORK PR NOTE: use HEAD not origin/<head> - for fork PRs, origin/<head> doesn't exist // FORK PR NOTE: checkout_pr returns the diff via GitHub API - use that for line numbers
// because the head branch is in a different repo (the fork). HEAD is the locally checked out PR branch. "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)."
"PRIMARY location for ALL feedback. 95%+ of review content should be here. Use 'git diff origin/<base>..HEAD' to find correct line numbers (RIGHT side for new code, LEFT for old). Works for both fork and same-repo PRs."
) )
.optional(), .optional(),
}); });
modes.ts
+11 -14
View File
@@ -100,29 +100,26 @@ ${
description: description:
"Review code, PRs, or implementations; provide feedback or suggestions; identify issues; or check code quality, style, and correctness", "Review code, PRs, or implementations; provide feedback or suggestions; identify issues; or check code quality, style, and correctness",
prompt: `Follow these steps: prompt: `Follow these steps:
1. Checkout the PR using ${ghPullfrogMcpName}/checkout_pr with the PR number. This fetches the PR branch and base branch, preparing the repo for review. 1. Checkout the PR using ${ghPullfrogMcpName}/checkout_pr with the PR number. This fetches the PR branch and returns the PR diff in the \`diff\` field of the response. Use this diff for your review - it shows exactly what's in the PR (fetched via GitHub API, so it's not affected by main advancing after the branch was created).
2. **IMPORTANT**: After calling ${ghPullfrogMcpName}/checkout_pr, the PR branch is checked out locally. View diff using: \`git diff origin/<base>..HEAD\` (replace <base> with 'base' from checkout_pr result, e.g., \`git diff origin/main..HEAD\`). Use two dots (..) not three dots (...) for reliable diffs. Do NOT use \`origin/<head>\` - the branch is checked out locally, not as a remote tracking branch. This works for both same-repo and fork PRs. 2. Start review session using ${ghPullfrogMcpName}/start_review. This creates a pending review on GitHub and returns analysis guidance. Follow the guidance before adding comments.
3. Start review session using ${ghPullfrogMcpName}/start_review. This creates a scratchpad file at a temp path (e.g., \`/tmp/pullfrog-review-abc123.md\`) and returns a session ID. The scratchpad file header contains the session ID for reference. Use this file as free-form space to gather your thoughts before adding comments. 3. **ANALYZE** - Before adding any comments, think through:
- What does this PR change? Summarize in 1-2 sentences.
- Is the approach sound? If not, **stop here** and comment on the approach first. Don't waste time on implementation details if the approach is wrong.
- What bugs, edge cases, or security issues exist?
4. **ANALYZE** - Use the scratchpad to gather your thoughts: 4. **BEFORE COMMENTING** - For each potential comment, ask yourself:
- Summarize what changes this PR makes - Is this a nitpick? Skip it unless explicitly requested.
- Evaluate the approach - is it sound? If not, **stop here** and leave feedback on the approach. Don't waste time on implementation details if the approach is wrong. - Would the codebase maintainer care about this feedback, based on what you can infer about the code quality standards in this repo?
- If approach is sound, analyze implementation - consider potential issues per file
- Identify bugs, security issues, edge cases
5. **SELF-CRITIQUE** - Before adding comments, review your scratchpad: 5. Add inline review comments one-by-one using ${ghPullfrogMcpName}/add_review_comment
- Remove nitpicks unless explicitly requested. Think documentation, JSDoc/docstrings, useless comments (compliments)
- Your level of nitpickiness should be proportional to the current state of the codebase. Try to guess how much the user will care about a specific critique.
6. Add inline review comments one-by-one using ${ghPullfrogMcpName}/add_review_comment
- Use **relative paths** from repo root (e.g., \`packages/core/src/utils.ts\`) - Use **relative paths** from repo root (e.g., \`packages/core/src/utils.ts\`)
- Use the NEW file line number from the diff (shown after \`+\` in hunk headers like \`@@ -10,5 +12,8 @@\` means new file starts at line 12) - Use the NEW file line number from the diff (shown after \`+\` in hunk headers like \`@@ -10,5 +12,8 @@\` means new file starts at line 12)
- Only comment on lines that appear in the diff. GitHub will reject comments on unchanged lines. - Only comment on lines that appear in the diff. GitHub will reject comments on unchanged lines.
- For issues appearing in multiple places, comment on the FIRST occurrence and reference others (e.g., "also at lines X, Y") - For issues appearing in multiple places, comment on the FIRST occurrence and reference others (e.g., "also at lines X, Y")
7. Submit the review using ${ghPullfrogMcpName}/submit_review 6. Submit the review using ${ghPullfrogMcpName}/submit_review
- The "body" field is ONLY for: (1) a 1-3 sentence high-level overview, (2) urgency level (e.g., "minor suggestions" vs "blocking issues"), (3) critical security callouts (e.g., API key exposure) - The "body" field is ONLY for: (1) a 1-3 sentence high-level overview, (2) urgency level (e.g., "minor suggestions" vs "blocking issues"), (3) critical security callouts (e.g., API key exposure)
**GENERAL GUIDANCE** **GENERAL GUIDANCE**
utils/cli.ts
+6 -5
View File
@@ -341,12 +341,13 @@ export const log = {
* Log tool call information to console with formatted output * Log tool call information to console with formatted output
*/ */
toolCall: ({ toolName, input }: { toolName: string; input: unknown }): void => { toolCall: ({ toolName, input }: { toolName: string; input: unknown }): void => {
let output = `→ ${toolName}\n`;
const inputFormatted = formatJsonValue(input); const inputFormatted = formatJsonValue(input);
if (inputFormatted !== "{}") { // if (inputFormatted !== "{}")
output += formatIndentedField("input", inputFormatted); const output = inputFormatted !== "{}" ? `${toolName}(${inputFormatted})` : `${toolName}()`;
}
// if (inputFormatted !== "{}") {
// output += formatIndentedField("input", inputFormatted);
// }
log.info(output.trimEnd()); log.info(output.trimEnd());
}, },
utils/setup.ts
+1 -1
View File
@@ -77,7 +77,7 @@ interface SetupGitAuthParams {
* FORK PR ARCHITECTURE: * FORK PR ARCHITECTURE:
* - origin: always points to BASE REPO (where PR targets) * - origin: always points to BASE REPO (where PR targets)
* - checkoutPrBranch sets per-branch pushRemote config for fork PRs * - checkoutPrBranch sets per-branch pushRemote config for fork PRs
* - diff operations use: git diff origin/<base>..HEAD * - checkout_pr returns the PR diff via GitHub API (authoritative source)
*/ */
export async function setupGitAuth(params: SetupGitAuthParams): Promise<void> { export async function setupGitAuth(params: SetupGitAuthParams): Promise<void> {
const repoDir = process.cwd(); const repoDir = process.cwd();