Compare commits

...

4 Commits

Author SHA1 Message Date
Colin McDonnell 17ad3bd0e7 0.0.154 2025-12-22 14:55:32 -08:00
Colin McDonnell 25896559f0 Switch back to one-shot reviews 2025-12-22 14:55:19 -08:00
Colin McDonnell 5353d80388 Retries on oidc. 152 2025-12-22 14:33:18 -08:00
Colin McDonnell 2dea842981 Write diff to file 2025-12-22 14:20:42 -08:00
8 changed files with 411 additions and 402 deletions
+159 -224
View File
@@ -45360,14 +45360,14 @@ var require_retry_agent = __commonJS({
this.#options = options; this.#options = options;
} }
dispatch(opts, handler2) { dispatch(opts, handler2) {
const retry = new RetryHandler({ const retry2 = new RetryHandler({
...opts, ...opts,
retryOptions: this.#options retryOptions: this.#options
}, { }, {
dispatch: this.#agent.dispatch.bind(this.#agent), dispatch: this.#agent.dispatch.bind(this.#agent),
handler: handler2 handler: handler2
}); });
return this.#agent.dispatch(opts, retry); return this.#agent.dispatch(opts, retry2);
} }
close() { close() {
return this.#agent.close(); return this.#agent.close();
@@ -83328,7 +83328,7 @@ function query({
// package.json // package.json
var package_default = { var package_default = {
name: "@pullfrog/action", name: "@pullfrog/action",
version: "0.0.151", version: "0.0.153",
type: "module", type: "module",
files: [ files: [
"index.js", "index.js",
@@ -92628,36 +92628,38 @@ ${disableProgressComment ? "" : `
name: "Review", name: "Review",
description: "Review code, PRs, or implementations; provide feedback or suggestions; identify issues; or check code quality, style, and correctness", description: "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 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. Start review session using ${ghPullfrogMcpName}/start_review. This creates a pending review on GitHub and returns analysis guidance. Follow the guidance before adding comments. 1. **CHECKOUT** - Use ${ghPullfrogMcpName}/checkout_pr with the PR number. This fetches the PR branch and returns the diff. Use this diff for your review - it shows exactly what's in the PR.
3. **ANALYZE** - Before adding any comments, think through: 2. **UNDERSTAND CONTEXT** - Read the modified files to understand the changes in context. Don't just look at the diff - understand how the changes affect the overall codebase.
3. **ANALYZE** - Think through:
- What does this PR change? Summarize in 1-2 sentences. - 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. - Is the approach sound? If not, focus on the approach first. Don't waste time on implementation details if the approach is wrong.
- What bugs, edge cases, or security issues exist? - What bugs, edge cases, or security issues exist?
4. **BEFORE COMMENTING** - For each potential comment, ask yourself: 4. **DRAFT** - Mentally list all comments you would make. For each:
- Is this a nitpick? Skip it unless explicitly requested. - Note the file path (relative to repo root, e.g., \`packages/core/src/utils.ts\`)
- Would the codebase maintainer care about this feedback, based on what you can infer about the code quality standards in this repo? - Note the line number from the diff (use NEW file line number - shown after \`+\` in hunk headers like \`@@ -10,5 +12,8 @@\` means new file starts at line 12)
- Draft the comment text
5. Add inline review comments one-by-one using ${ghPullfrogMcpName}/add_review_comment 5. **SELF-CRITIQUE** - Before submitting, review your draft:
- Use **relative paths** from repo root (e.g., \`packages/core/src/utils.ts\`) - Remove nitpicks unless the user explicitly told you to be nitpicky. Nitpicks may include: requesting documentation/docstrings/JSDoc, commenting on minor code/whitespace formatting, commenting on small changes unrelated to the main changes.
- 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) - Ensure each comment is actionable - would the author know exactly what to do?
- Only comment on lines that appear in the diff. GitHub will reject comments on unchanged lines. - Would the codebase maintainer care about this feedback?
- For issues appearing in multiple places, comment on the FIRST occurrence and reference others (e.g., "also at lines X, Y") - If you have approach-level concerns, consider whether implementation-level comments are worth including
- For issues appearing in multiple places, keep only the FIRST occurrence and reference others (e.g., "also at lines X, Y")
6. Submit the review using ${ghPullfrogMcpName}/submit_review 6. **SUBMIT** - Use ${ghPullfrogMcpName}/create_pull_request_review with:
- 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) - \`comments\`: Array of all inline comments with file paths and line numbers
- \`body\`: 1-3 sentence summary with urgency level (e.g., "minor suggestions" vs "blocking issues") and any critical callouts (e.g., API key exposure)
**GENERAL GUIDANCE** **CRITICAL RULES**
- 95%+ of review content should be in inline \`comments\` array, not the \`body\`
- Do not leave any comments that are not potentially actionable. Do not leave complimentary comments just to be nice. - Only comment on lines that appear in the diff - GitHub will reject comments on unchanged lines
- Do not nitpick unless instructed explicitly to do so by the user's additional instructions. This includes: requesting documentation/docstrings/JSDoc. - Do not leave complimentary comments just to be nice
- **CRITICAL: Prioritize per-line feedback over summary text.** - Do not leave comments that are not actionable
- All specific feedback MUST go in inline review comments with file paths and line numbers from the diff `
- The vast majority of review content should be in inline review comments; the body should be brief and only summarize the urgency of the review and any cross-cutting concerns.
`
}, },
{ {
name: "Plan", name: "Plan",
@@ -92866,6 +92868,35 @@ import { pipeline } from "node:stream/promises";
// utils/github.ts // utils/github.ts
var core2 = __toESM(require_core(), 1); var core2 = __toESM(require_core(), 1);
import { createSign } from "node:crypto"; import { createSign } from "node:crypto";
// utils/retry.ts
var defaultShouldRetry = (error41) => {
if (!(error41 instanceof Error)) return false;
return error41.name === "AbortError" || error41.message.includes("fetch failed") || error41.message.includes("ECONNRESET") || error41.message.includes("ETIMEDOUT");
};
async function retry(fn2, options = {}) {
const maxAttempts = options.maxAttempts ?? 3;
const delayMs = options.delayMs ?? 1e3;
const shouldRetry = options.shouldRetry ?? defaultShouldRetry;
const label = options.label ?? "operation";
let lastError;
for (let attempt = 1; attempt <= maxAttempts; attempt++) {
try {
return await fn2();
} catch (error41) {
lastError = error41;
if (attempt === maxAttempts || !shouldRetry(error41)) {
throw error41;
}
const delay2 = delayMs * attempt;
log.warning(`\xBB ${label} failed (attempt ${attempt}/${maxAttempts}), retrying in ${delay2}ms...`);
await new Promise((resolve2) => setTimeout(resolve2, delay2));
}
}
throw lastError;
}
// utils/github.ts
function isGitHubActionsEnvironment() { function isGitHubActionsEnvironment() {
return Boolean(process.env.GITHUB_ACTIONS); return Boolean(process.env.GITHUB_ACTIONS);
} }
@@ -92998,7 +93029,7 @@ async function acquireTokenViaGitHubApp() {
} }
async function acquireNewToken() { async function acquireNewToken() {
if (isGitHubActionsEnvironment()) { if (isGitHubActionsEnvironment()) {
return await acquireTokenViaOIDC(); return await retry(() => acquireTokenViaOIDC(), { label: "token exchange" });
} else { } else {
return await acquireTokenViaGitHubApp(); return await acquireTokenViaGitHubApp();
} }
@@ -94721,7 +94752,7 @@ var agents = {
// main.ts // main.ts
import { mkdtemp as mkdtemp2 } from "node:fs/promises"; import { mkdtemp as mkdtemp2 } from "node:fs/promises";
import { tmpdir as tmpdir2 } from "node:os"; 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 // node_modules/.pnpm/universal-user-agent@7.0.3/node_modules/universal-user-agent/index.js
function getUserAgent() { function getUserAgent() {
@@ -122940,6 +122971,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 // utils/shell.ts
import { spawnSync as spawnSync4 } from "node:child_process"; import { spawnSync as spawnSync4 } from "node:child_process";
function $(cmd, args3, options) { function $(cmd, args3, options) {
@@ -123023,10 +123058,10 @@ async function checkoutPrBranch(params) {
const remoteName = `pr-${pullNumber}`; const remoteName = `pr-${pullNumber}`;
const forkUrl = `https://x-access-token:${token}@github.com/${headRepo.full_name}.git`; const forkUrl = `https://x-access-token:${token}@github.com/${headRepo.full_name}.git`;
try { try {
$("git", ["remote", "add", remoteName, forkUrl]); $("git", ["remote", "add", remoteName, forkUrl], { log: false });
log.debug(`\u{1F4CC} added remote '${remoteName}' for fork ${headRepo.full_name}`); log.debug(`\u{1F4CC} added remote '${remoteName}' for fork ${headRepo.full_name}`);
} catch { } catch {
$("git", ["remote", "set-url", remoteName, forkUrl]); $("git", ["remote", "set-url", remoteName, forkUrl], { log: false });
log.debug(`\u{1F4CC} updated remote '${remoteName}' for fork ${headRepo.full_name}`); log.debug(`\u{1F4CC} updated remote '${remoteName}' for fork ${headRepo.full_name}`);
} }
$("git", ["config", `branch.${localBranch}.pushRemote`, remoteName]); $("git", ["config", `branch.${localBranch}.pushRemote`, remoteName]);
@@ -123046,7 +123081,7 @@ async function checkoutPrBranch(params) {
function CheckoutPrTool(ctx) { function CheckoutPrTool(ctx) {
return tool({ return tool({
name: "checkout_pr", 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, parameters: CheckoutPr,
execute: execute(async ({ pull_number }) => { execute: execute(async ({ pull_number }) => {
const result = await checkoutPrBranch({ const result = await checkoutPrBranch({
@@ -123072,6 +123107,18 @@ function CheckoutPrTool(ctx) {
pull_number, pull_number,
mediaType: { format: "diff" } mediaType: { format: "diff" }
}); });
const diffContent = diffResponse.data;
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 = join8(tempDir, `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 { return {
success: true, success: true,
number: pr.data.number, number: pr.data.number,
@@ -123082,7 +123129,8 @@ function CheckoutPrTool(ctx) {
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 diff,
diffPath
}; };
}) })
}); });
@@ -123191,7 +123239,7 @@ function DebugShellCommandTool(_ctx) {
// prep/installNodeDependencies.ts // prep/installNodeDependencies.ts
import { existsSync as existsSync3, readFileSync as readFileSync2 } from "node:fs"; 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 // node_modules/.pnpm/package-manager-detector@1.6.0/node_modules/package-manager-detector/dist/commands.mjs
function dashDashArg(agent2, agentCommand) { function dashDashArg(agent2, agentCommand) {
@@ -123503,7 +123551,7 @@ async function isCommandAvailable(command) {
return result.exitCode === 0; return result.exitCode === 0;
} }
function getPackageManagerFromPackageJson() { function getPackageManagerFromPackageJson() {
const packageJsonPath = join8(process.cwd(), "package.json"); const packageJsonPath = join9(process.cwd(), "package.json");
try { try {
const content = readFileSync2(packageJsonPath, "utf-8"); const content = readFileSync2(packageJsonPath, "utf-8");
const pkg = JSON.parse(content); const pkg = JSON.parse(content);
@@ -123534,7 +123582,7 @@ async function installPackageManager(name, installSpec) {
return result.stderr || `failed to install ${name}`; return result.stderr || `failed to install ${name}`;
} }
if (name === "deno") { 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}`; process.env.PATH = `${denoPath}:${process.env.PATH}`;
} }
log.info(`\u2705 installed ${name}`); log.info(`\u2705 installed ${name}`);
@@ -123543,7 +123591,7 @@ async function installPackageManager(name, installSpec) {
var installNodeDependencies = { var installNodeDependencies = {
name: "installNodeDependencies", name: "installNodeDependencies",
shouldRun: () => { shouldRun: () => {
const packageJsonPath = join8(process.cwd(), "package.json"); const packageJsonPath = join9(process.cwd(), "package.json");
return existsSync3(packageJsonPath); return existsSync3(packageJsonPath);
}, },
run: async () => { run: async () => {
@@ -123606,7 +123654,7 @@ var installNodeDependencies = {
// prep/installPythonDependencies.ts // prep/installPythonDependencies.ts
import { existsSync as existsSync4 } from "node:fs"; import { existsSync as existsSync4 } from "node:fs";
import { join as join9 } from "node:path"; import { join as join10 } from "node:path";
var PYTHON_CONFIGS = [ var PYTHON_CONFIGS = [
{ {
file: "requirements.txt", file: "requirements.txt",
@@ -123678,11 +123726,11 @@ var installPythonDependencies = {
return false; return false;
} }
const cwd2 = process.cwd(); 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 () => { run: async () => {
const cwd2 = process.cwd(); 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) { if (!config2) {
return { return {
language: "python", language: "python",
@@ -124426,187 +124474,7 @@ function PullRequestInfoTool(ctx) {
} }
// mcp/review.ts // mcp/review.ts
var ADD_PULL_REQUEST_REVIEW_THREAD = ` var CreatePullRequestReview = type({
mutation AddPullRequestReviewThread($pullRequestReviewId: ID!, $path: String!, $line: Int!, $body: String!, $side: DiffSide) {
addPullRequestReviewThread(input: {
pullRequestReviewId: $pullRequestReviewId,
path: $path,
line: $line,
body: $body,
side: $side
}) {
thread {
id
}
}
}
`;
async function findPendingReview(ctx, pull_number) {
const reviews = await ctx.octokit.rest.pulls.listReviews({
owner: ctx.owner,
repo: ctx.name,
pull_number,
per_page: 100
});
const pendingReview = reviews.data.find((r) => r.state === "PENDING");
if (pendingReview) {
return { id: pendingReview.id, node_id: pendingReview.node_id };
}
return null;
}
var StartReview = type({
pull_number: type.number.describe("The pull request number to review")
});
function StartReviewTool(ctx) {
return tool({
name: "start_review",
description: "Start a new review session for a pull request. Creates a pending review on GitHub. Must be called before add_review_comment.",
parameters: StartReview,
execute: execute(async ({ pull_number }) => {
if (ctx.toolState.review) {
throw new Error(
`Review session already in progress. Call submit_review first to finish it.`
);
}
const pr = await ctx.octokit.rest.pulls.get({
owner: ctx.owner,
repo: ctx.name,
pull_number
});
let reviewId;
let reviewNodeId;
log.debug(`creating pending review for PR #${pull_number}...`);
try {
const result = await ctx.octokit.rest.pulls.createReview({
owner: ctx.owner,
repo: ctx.name,
pull_number,
commit_id: pr.data.head.sha
// no 'event' = PENDING review
});
reviewId = result.data.id;
reviewNodeId = result.data.node_id;
log.debug(`created new pending review: id=${reviewId}`);
} catch (error41) {
const errorMessage = error41 instanceof Error ? error41.message : String(error41);
log.debug(`createReview failed: ${errorMessage}`);
if (errorMessage.includes("pending review")) {
log.debug(`pending review already exists, fetching existing review...`);
const existing = await findPendingReview(ctx, pull_number);
if (!existing) {
throw new Error(
"GitHub says a pending review exists but we couldn't find it. Try again or check the PR reviews."
);
}
reviewId = existing.id;
reviewNodeId = existing.node_id;
log.debug(`reusing existing pending review: id=${reviewId}`);
} else {
throw error41;
}
}
ctx.toolState.prNumber = pull_number;
ctx.toolState.review = {
nodeId: reviewNodeId,
id: reviewId
};
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."
};
})
});
}
var AddReviewComment = 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 - the NEW file line number)"
),
body: type.string.describe("The comment text for this specific line"),
side: type.enumerated("LEFT", "RIGHT").describe("Side of the diff: LEFT (old code) or RIGHT (new code). Defaults to RIGHT.").optional()
});
function AddReviewCommentTool(ctx) {
return tool({
name: "add_review_comment",
description: "Add a comment to the current review session. Must call start_review first. Comments are stored in draft state until submit_review is called.",
parameters: AddReviewComment,
execute: execute(async ({ path: path4, line, body, side }) => {
if (!ctx.toolState.review) {
throw new Error("No review session started. Call start_review first.");
}
log.debug(
`adding review comment: reviewNodeId=${ctx.toolState.review.nodeId}, 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}`);
return {
success: true,
message: `Comment added to ${path4}:${line}`,
threadId: result.addPullRequestReviewThread.thread.id
};
})
});
}
var SubmitReview = type({
body: type.string.describe(
"Review body text. Typically 1-3 sentences with high-level overview and urgency level. Action links are auto-appended."
).optional()
});
function SubmitReviewTool(ctx) {
return tool({
name: "submit_review",
description: "Submit the current review session. All comments added via add_review_comment will be published. Must call start_review first.",
parameters: SubmitReview,
execute: execute(async ({ body }) => {
if (!ctx.toolState.review) {
throw new Error("No review session started. Call start_review first.");
}
if (ctx.toolState.prNumber === void 0) {
throw new Error("No PR context. Call checkout_pr or start_review first.");
}
const reviewId = ctx.toolState.review.id;
log.debug(
`submitting review: id=${reviewId}, nodeId=${ctx.toolState.review.nodeId}, prNumber=${ctx.toolState.prNumber}`
);
const apiUrl = process.env.API_URL || "https://pullfrog.com";
const fixAllUrl = `${apiUrl}/trigger/${ctx.owner}/${ctx.name}/${ctx.toolState.prNumber}?action=fix&review_id=${reviewId}`;
const fixApprovedUrl = `${apiUrl}/trigger/${ctx.owner}/${ctx.name}/${ctx.toolState.prNumber}?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 \u2794](${fixAllUrl})`, `[Fix \u{1F44D}s \u2794](${fixApprovedUrl})`]
});
const bodyWithFooter = (body || "") + footer;
const result = await ctx.octokit.rest.pulls.submitReview({
owner: ctx.owner,
repo: ctx.name,
pull_number: ctx.toolState.prNumber,
review_id: reviewId,
event: "COMMENT",
body: bodyWithFooter
});
log.debug(`review submitted: reviewId=${result.data.id}, state=${result.data.state}`);
delete ctx.toolState.review;
await deleteProgressComment(ctx);
return {
success: true,
reviewId: result.data.id,
html_url: result.data.html_url,
state: result.data.state
};
})
});
}
var Review = type({
pull_number: type.number.describe("The pull request number to review"), pull_number: type.number.describe("The pull request number to review"),
body: type.string.describe( 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." "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."
@@ -124629,6 +124497,76 @@ var Review = type({
"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 the diff returned from checkout_pr to find correct line numbers (RIGHT side for new code, LEFT for old)."
).optional() ).optional()
}); });
function CreatePullRequestReviewTool(ctx) {
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 = [] }) => {
ctx.toolState.prNumber = pull_number;
const pr = await ctx.octokit.rest.pulls.get({
owner: ctx.owner,
repo: ctx.name,
pull_number
});
const params = {
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) {
params.comments = comments.map((comment) => {
const 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;
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 \u2794](${fixAllUrl})`, `[Fix \u{1F44D}s \u2794](${fixApprovedUrl})`]
});
const updatedBody = (body || "") + 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
};
})
});
}
// mcp/reviewComments.ts // mcp/reviewComments.ts
var REVIEW_THREADS_QUERY = ` var REVIEW_THREADS_QUERY = `
@@ -124840,10 +124778,7 @@ async function startMcpHttpServer(ctx) {
GetIssueCommentsTool(ctx), GetIssueCommentsTool(ctx),
GetIssueEventsTool(ctx), GetIssueEventsTool(ctx),
PullRequestTool(ctx), PullRequestTool(ctx),
// ReviewTool(ctx), CreatePullRequestReviewTool(ctx),
StartReviewTool(ctx),
AddReviewCommentTool(ctx),
SubmitReviewTool(ctx),
PullRequestInfoTool(ctx), PullRequestInfoTool(ctx),
CheckoutPrTool(ctx), CheckoutPrTool(ctx),
GetReviewCommentsTool(ctx), GetReviewCommentsTool(ctx),
@@ -125248,7 +125183,7 @@ function resolveAgent({
return agent2; return agent2;
} }
async function createTempDirectory() { async function createTempDirectory() {
const sharedTempDir = await mkdtemp2(join10(tmpdir2(), "pullfrog-")); const sharedTempDir = await mkdtemp2(join11(tmpdir2(), "pullfrog-"));
process.env.PULLFROG_TEMP_DIR = sharedTempDir; process.env.PULLFROG_TEMP_DIR = sharedTempDir;
log.info(`\u{1F4C2} PULLFROG_TEMP_DIR has been created at ${sharedTempDir}`); log.info(`\u{1F4C2} PULLFROG_TEMP_DIR has been created at ${sharedTempDir}`);
return sharedTempDir; return sharedTempDir;
+29 -5
View File
@@ -1,3 +1,5 @@
import { writeFileSync } from "node:fs";
import { join } from "node:path";
import type { Octokit } from "@octokit/rest"; import type { Octokit } from "@octokit/rest";
import { type } from "arktype"; import { type } from "arktype";
import type { ToolContext } from "../main.ts"; import type { ToolContext } from "../main.ts";
@@ -20,6 +22,7 @@ export type CheckoutPrResult = {
url: string; url: string;
headRepo: string; headRepo: string;
diff: string; diff: string;
diffPath: string;
}; };
interface CheckoutPrBranchParams { interface CheckoutPrBranchParams {
@@ -104,13 +107,13 @@ export async function checkoutPrBranch(
const remoteName = `pr-${pullNumber}`; const remoteName = `pr-${pullNumber}`;
const forkUrl = `https://x-access-token:${token}@github.com/${headRepo.full_name}.git`; const forkUrl = `https://x-access-token:${token}@github.com/${headRepo.full_name}.git`;
// add fork as a named remote (ignore error if already exists) // add fork as a named remote (suppress logging to avoid "error: remote already exists" spam)
try { try {
$("git", ["remote", "add", remoteName, forkUrl]); $("git", ["remote", "add", remoteName, forkUrl], { log: false });
log.debug(`📌 added remote '${remoteName}' for fork ${headRepo.full_name}`); log.debug(`📌 added remote '${remoteName}' for fork ${headRepo.full_name}`);
} catch { } catch {
// remote already exists, update its URL // remote already exists, update its URL
$("git", ["remote", "set-url", remoteName, forkUrl]); $("git", ["remote", "set-url", remoteName, forkUrl], { log: false });
log.debug(`📌 updated remote '${remoteName}' for fork ${headRepo.full_name}`); log.debug(`📌 updated remote '${remoteName}' for fork ${headRepo.full_name}`);
} }
@@ -140,7 +143,8 @@ export function CheckoutPrTool(ctx: ToolContext) {
return tool({ return tool({
name: "checkout_pr", name: "checkout_pr",
description: 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, parameters: CheckoutPr,
execute: execute(async ({ pull_number }) => { execute: execute(async ({ pull_number }) => {
const result = await checkoutPrBranch({ const result = await checkoutPrBranch({
@@ -174,6 +178,25 @@ export function CheckoutPrTool(ctx: ToolContext) {
mediaType: { format: "diff" }, mediaType: { format: "diff" },
}); });
// write diff to file for grep access
const diffContent = diffResponse.data as unknown as string;
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)`);
// 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 { return {
success: true, success: true,
number: pr.data.number, number: pr.data.number,
@@ -184,7 +207,8 @@ 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, diff,
diffPath,
} satisfies CheckoutPrResult; } satisfies CheckoutPrResult;
}), }),
}); });
+193 -142
View File
@@ -6,16 +6,150 @@ import { log } from "../utils/cli.ts";
import { deleteProgressComment } from "./comment.ts"; import { deleteProgressComment } from "./comment.ts";
import { execute, tool } from "./shared.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 // graphql mutation to add a comment thread to a pending review
// note: REST API doesn't support adding comments to an existing pending review // note: REST API doesn't support adding comments to an existing pending review
const ADD_PULL_REQUEST_REVIEW_THREAD = ` 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: { addPullRequestReviewThread(input: {
pullRequestReviewId: $pullRequestReviewId, pullRequestReviewId: $pullRequestReviewId,
path: $path, path: $path,
line: $line, line: $line,
body: $body, body: $body,
side: $side side: $side,
subjectType: $subjectType
}) { }) {
thread { thread {
id id
@@ -92,6 +226,13 @@ export function StartReviewTool(ctx: ToolContext) {
commit_id: pr.data.head.sha, commit_id: pr.data.head.sha,
// no 'event' = PENDING review // 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; reviewId = result.data.id;
reviewNodeId = result.data.node_id; reviewNodeId = result.data.node_id;
log.debug(`created new pending review: id=${reviewId}`); log.debug(`created new pending review: id=${reviewId}`);
@@ -126,10 +267,7 @@ export function StartReviewTool(ctx: ToolContext) {
log.debug(`review session started: id=${reviewId}, nodeId=${reviewNodeId}`); log.debug(`review session started: id=${reviewId}, nodeId=${reviewNodeId}`);
return { return {
message: `Review session started for PR #${pull_number}.`, message: `Review session started for PR #${pull_number}. Add comments with add_review_comment, then submit with submit_review.`,
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.",
}; };
}), }),
}); });
@@ -160,28 +298,59 @@ export function AddReviewCommentTool(ctx: ToolContext) {
throw new Error("No review session started. Call start_review first."); throw new Error("No review session started. Call start_review first.");
} }
const reviewNodeId = ctx.toolState.review.nodeId;
log.debug( 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) // add comment thread via GraphQL (REST doesn't support adding to existing pending review)
const result = await ctx.octokit.graphql<AddPullRequestReviewThreadResponse>( let result: AddPullRequestReviewThreadResponse;
ADD_PULL_REQUEST_REVIEW_THREAD, try {
{ result = await ctx.octokit.graphql<AddPullRequestReviewThreadResponse>(
pullRequestReviewId: ctx.toolState.review.nodeId, ADD_PULL_REQUEST_REVIEW_THREAD,
path, {
line, pullRequestReviewId: reviewNodeId,
body, path,
side: side || "RIGHT", line,
} body,
); side: side || "RIGHT",
subjectType: "LINE",
}
);
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 { return {
success: true, success: true,
message: `Comment added to ${path}:${line}`, message: `Comment added to ${path}:${line}`,
threadId: result.addPullRequestReviewThread.thread.id, threadId,
}; };
}), }),
}); });
@@ -238,6 +407,10 @@ export function SubmitReviewTool(ctx: ToolContext) {
body: bodyWithFooter, 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}`); log.debug(`review submitted: reviewId=${result.data.id}, state=${result.data.state}`);
// clear review state // clear review state
@@ -255,126 +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);
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 { AddLabelsTool } from "./labels.ts";
import { PullRequestTool } from "./pr.ts"; import { PullRequestTool } from "./pr.ts";
import { PullRequestInfoTool } from "./prInfo.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 { GetReviewCommentsTool, ListPullRequestReviewsTool } from "./reviewComments.ts";
import { SelectModeTool } from "./selectMode.ts"; import { SelectModeTool } from "./selectMode.ts";
import { addTools } from "./shared.ts"; import { addTools } from "./shared.ts";
@@ -84,10 +84,7 @@ export async function startMcpHttpServer(
GetIssueCommentsTool(ctx), GetIssueCommentsTool(ctx),
GetIssueEventsTool(ctx), GetIssueEventsTool(ctx),
PullRequestTool(ctx), PullRequestTool(ctx),
// ReviewTool(ctx), CreatePullRequestReviewTool(ctx),
StartReviewTool(ctx),
AddReviewCommentTool(ctx),
SubmitReviewTool(ctx),
PullRequestInfoTool(ctx), PullRequestInfoTool(ctx),
CheckoutPrTool(ctx), CheckoutPrTool(ctx),
GetReviewCommentsTool(ctx), GetReviewCommentsTool(ctx),
+24 -22
View File
@@ -100,36 +100,38 @@ ${
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 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. Start review session using ${ghPullfrogMcpName}/start_review. This creates a pending review on GitHub and returns analysis guidance. Follow the guidance before adding comments. 1. **CHECKOUT** - Use ${ghPullfrogMcpName}/checkout_pr with the PR number. This fetches the PR branch and returns the diff. Use this diff for your review - it shows exactly what's in the PR.
3. **ANALYZE** - Before adding any comments, think through: 2. **UNDERSTAND CONTEXT** - Read the modified files to understand the changes in context. Don't just look at the diff - understand how the changes affect the overall codebase.
3. **ANALYZE** - Think through:
- What does this PR change? Summarize in 1-2 sentences. - 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. - Is the approach sound? If not, focus on the approach first. Don't waste time on implementation details if the approach is wrong.
- What bugs, edge cases, or security issues exist? - What bugs, edge cases, or security issues exist?
4. **BEFORE COMMENTING** - For each potential comment, ask yourself: 4. **DRAFT** - Mentally list all comments you would make. For each:
- Is this a nitpick? Skip it unless explicitly requested. - Note the file path (relative to repo root, e.g., \`packages/core/src/utils.ts\`)
- Would the codebase maintainer care about this feedback, based on what you can infer about the code quality standards in this repo? - Note the line number from the diff (use NEW file line number - shown after \`+\` in hunk headers like \`@@ -10,5 +12,8 @@\` means new file starts at line 12)
- Draft the comment text
5. Add inline review comments one-by-one using ${ghPullfrogMcpName}/add_review_comment 5. **SELF-CRITIQUE** - Before submitting, review your draft:
- Use **relative paths** from repo root (e.g., \`packages/core/src/utils.ts\`) - Remove nitpicks unless the user explicitly told you to be nitpicky. Nitpicks may include: requesting documentation/docstrings/JSDoc, commenting on minor code/whitespace formatting, commenting on small changes unrelated to the main changes.
- 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) - Ensure each comment is actionable - would the author know exactly what to do?
- Only comment on lines that appear in the diff. GitHub will reject comments on unchanged lines. - Would the codebase maintainer care about this feedback?
- For issues appearing in multiple places, comment on the FIRST occurrence and reference others (e.g., "also at lines X, Y") - If you have approach-level concerns, consider whether implementation-level comments are worth including
- For issues appearing in multiple places, keep only the FIRST occurrence and reference others (e.g., "also at lines X, Y")
6. Submit the review using ${ghPullfrogMcpName}/submit_review 6. **SUBMIT** - Use ${ghPullfrogMcpName}/create_pull_request_review with:
- 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) - \`comments\`: Array of all inline comments with file paths and line numbers
- \`body\`: 1-3 sentence summary with urgency level (e.g., "minor suggestions" vs "blocking issues") and any critical callouts (e.g., API key exposure)
**GENERAL GUIDANCE** **CRITICAL RULES**
- 95%+ of review content should be in inline \`comments\` array, not the \`body\`
- Do not leave any comments that are not potentially actionable. Do not leave complimentary comments just to be nice. - Only comment on lines that appear in the diff - GitHub will reject comments on unchanged lines
- Do not nitpick unless instructed explicitly to do so by the user's additional instructions. This includes: requesting documentation/docstrings/JSDoc. - Do not leave complimentary comments just to be nice
- **CRITICAL: Prioritize per-line feedback over summary text.** - Do not leave comments that are not actionable
- All specific feedback MUST go in inline review comments with file paths and line numbers from the diff `,
- The vast majority of review content should be in inline review comments; the body should be brief and only summarize the urgency of the review and any cross-cutting concerns.
`,
}, },
{ {
name: "Plan", name: "Plan",
+1 -1
View File
@@ -1,6 +1,6 @@
{ {
"name": "@pullfrog/action", "name": "@pullfrog/action",
"version": "0.0.151", "version": "0.0.154",
"type": "module", "type": "module",
"files": [ "files": [
"index.js", "index.js",
+1 -1
View File
@@ -278,7 +278,7 @@ export const log = {
/** /**
* Print debug message (only if LOG_LEVEL=debug) * Print debug message (only if LOG_LEVEL=debug)
*/ */
debug: (message: string): void => { debug: (message: string | unknown): void => {
if (isDebugEnabled()) { if (isDebugEnabled()) {
if (isGitHubActions) { if (isGitHubActions) {
// using this instead of core.debug // using this instead of core.debug
+2 -2
View File
@@ -1,6 +1,7 @@
import { createSign } from "node:crypto"; import { createSign } from "node:crypto";
import * as core from "@actions/core"; import * as core from "@actions/core";
import { log } from "./cli.ts"; import { log } from "./cli.ts";
import { retry } from "./retry.ts";
export interface InstallationToken { export interface InstallationToken {
token: string; token: string;
@@ -56,7 +57,6 @@ async function acquireTokenViaOIDC(): Promise<string> {
log.info("» exchanging OIDC token for installation token..."); log.info("» exchanging OIDC token for installation token...");
// Add timeout to prevent long waits (30 seconds)
const timeoutMs = 30000; const timeoutMs = 30000;
const controller = new AbortController(); const controller = new AbortController();
const timeoutId = setTimeout(() => controller.abort(), timeoutMs); const timeoutId = setTimeout(() => controller.abort(), timeoutMs);
@@ -234,7 +234,7 @@ async function acquireTokenViaGitHubApp(): Promise<string> {
async function acquireNewToken(): Promise<string> { async function acquireNewToken(): Promise<string> {
if (isGitHubActionsEnvironment()) { if (isGitHubActionsEnvironment()) {
return await acquireTokenViaOIDC(); return await retry(() => acquireTokenViaOIDC(), { label: "token exchange" });
} else { } else {
return await acquireTokenViaGitHubApp(); return await acquireTokenViaGitHubApp();
} }