Compare commits
6 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
 Colin McDonnell
|
e9a585ce47 | ||
|
Colin McDonnell
|
7407b6cbc5 | ||
|
Colin McDonnell
|
507efb0c25 | ||
|
Colin McDonnell
|
6d572f3ce8 | ||
|
Colin McDonnell
|
73139a169c | ||
|
Colin McDonnell
|
d5bec7499b |
@@ -83328,7 +83328,7 @@ function query({
|
||||
// package.json
|
||||
var package_default = {
|
||||
name: "@pullfrog/action",
|
||||
version: "0.0.148",
|
||||
version: "0.0.150",
|
||||
type: "module",
|
||||
files: [
|
||||
"index.js",
|
||||
@@ -83653,12 +83653,9 @@ var log = {
|
||||
* Log tool call information to console with formatted output
|
||||
*/
|
||||
toolCall: ({ toolName, input }) => {
|
||||
let output = `\u2192 ${toolName}
|
||||
`;
|
||||
const inputFormatted = formatJsonValue(input);
|
||||
if (inputFormatted !== "{}") {
|
||||
output += formatIndentedField("input", inputFormatted);
|
||||
}
|
||||
const timestamp = isDebugEnabled() ? ` [${(/* @__PURE__ */ new Date()).toISOString()}]` : "";
|
||||
const output = inputFormatted !== "{}" ? `\u2192 ${toolName}(${inputFormatted})${timestamp}` : `\u2192 ${toolName}()${timestamp}`;
|
||||
log.info(output.trimEnd());
|
||||
}
|
||||
};
|
||||
@@ -83666,20 +83663,6 @@ function formatJsonValue(value2) {
|
||||
const compact = JSON.stringify(value2);
|
||||
return compact.length > 80 || compact.includes("\n") ? JSON.stringify(value2, null, 2) : compact;
|
||||
}
|
||||
function formatIndentedField(label, content) {
|
||||
if (!content.includes("\n")) {
|
||||
return ` ${label}: ${content}
|
||||
`;
|
||||
}
|
||||
const lines = content.split("\n");
|
||||
let formatted = ` ${label}: ${lines[0]}
|
||||
`;
|
||||
for (let i = 1; i < lines.length; i++) {
|
||||
formatted += ` ${lines[i]}
|
||||
`;
|
||||
}
|
||||
return formatted;
|
||||
}
|
||||
|
||||
// agents/instructions.ts
|
||||
import { execSync } from "node:child_process";
|
||||
@@ -92645,29 +92628,26 @@ ${disableProgressComment ? "" : `
|
||||
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:
|
||||
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:
|
||||
- Summarize what changes this PR makes
|
||||
- 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.
|
||||
- If approach is sound, analyze implementation - consider potential issues per file
|
||||
- Identify bugs, security issues, edge cases
|
||||
4. **BEFORE COMMENTING** - For each potential comment, ask yourself:
|
||||
- Is this a nitpick? Skip it 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?
|
||||
|
||||
5. **SELF-CRITIQUE** - Before adding comments, review your scratchpad:
|
||||
- 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
|
||||
5. 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 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.
|
||||
- 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)
|
||||
|
||||
**GENERAL GUIDANCE**
|
||||
@@ -92890,12 +92870,11 @@ function isGitHubActionsEnvironment() {
|
||||
return Boolean(process.env.GITHUB_ACTIONS);
|
||||
}
|
||||
async function acquireTokenViaOIDC() {
|
||||
log.debug("\xBB generating OIDC token...");
|
||||
log.info("\xBB generating OIDC token...");
|
||||
const oidcToken = await core2.getIDToken("pullfrog-api");
|
||||
log.debug("\xBB OIDC token generated successfully");
|
||||
const apiUrl = process.env.API_URL || "https://pullfrog.com";
|
||||
log.debug("\xBB exchanging OIDC token for installation token...");
|
||||
const timeoutMs = 5e3;
|
||||
log.info("\xBB exchanging OIDC token for installation token...");
|
||||
const timeoutMs = 3e4;
|
||||
const controller = new AbortController();
|
||||
const timeoutId = setTimeout(() => controller.abort(), timeoutMs);
|
||||
try {
|
||||
@@ -92912,7 +92891,7 @@ async function acquireTokenViaOIDC() {
|
||||
throw new Error(`Token exchange failed: ${tokenResponse.status} ${tokenResponse.statusText}`);
|
||||
}
|
||||
const tokenData = await tokenResponse.json();
|
||||
log.debug(`\xBB installation token obtained for ${tokenData.repository || "all repositories"}`);
|
||||
log.info(`\xBB installation token obtained for ${tokenData.repository || "all repositories"}`);
|
||||
return tokenData.token;
|
||||
} catch (error41) {
|
||||
clearTimeout(timeoutId);
|
||||
@@ -94742,7 +94721,7 @@ var agents = {
|
||||
// main.ts
|
||||
import { mkdtemp as mkdtemp2 } from "node:fs/promises";
|
||||
import { tmpdir as tmpdir2 } from "node:os";
|
||||
import { join as join11 } from "node:path";
|
||||
import { join as join10 } from "node:path";
|
||||
|
||||
// node_modules/.pnpm/universal-user-agent@7.0.3/node_modules/universal-user-agent/index.js
|
||||
function getUserAgent() {
|
||||
@@ -98177,7 +98156,7 @@ var DEFAULT_REPO_SETTINGS = {
|
||||
};
|
||||
async function fetchWorkflowRunInfo(runId) {
|
||||
const apiUrl = process.env.API_URL || "https://pullfrog.com";
|
||||
const timeoutMs = 5e3;
|
||||
const timeoutMs = 3e4;
|
||||
const controller = new AbortController();
|
||||
const timeoutId = setTimeout(() => controller.abort(), timeoutMs);
|
||||
try {
|
||||
@@ -98208,7 +98187,7 @@ async function fetchRepoSettings({
|
||||
}
|
||||
async function getRepoSettings(token, repoContext) {
|
||||
const apiUrl = process.env.API_URL || "https://pullfrog.com";
|
||||
const timeoutMs = 5e3;
|
||||
const timeoutMs = 3e4;
|
||||
const controller = new AbortController();
|
||||
const timeoutId = setTimeout(() => controller.abort(), timeoutMs);
|
||||
try {
|
||||
@@ -123015,17 +122994,18 @@ async function checkoutPrBranch(params) {
|
||||
const isFork = headRepo.full_name !== pr.data.base.repo.full_name;
|
||||
const baseBranch = pr.data.base.ref;
|
||||
const headBranch = pr.data.head.ref;
|
||||
const currentBranch = $("git", ["rev-parse", "--abbrev-ref", "HEAD"], { log: false }).trim();
|
||||
const alreadyOnBranch = currentBranch === headBranch;
|
||||
const localBranch = `pr-${pullNumber}`;
|
||||
const currentSha = $("git", ["rev-parse", "HEAD"], { log: false }).trim();
|
||||
const alreadyOnBranch = currentSha === pr.data.head.sha;
|
||||
if (alreadyOnBranch) {
|
||||
log.debug(`already on PR branch ${headBranch}, skipping checkout`);
|
||||
log.debug(`already on PR branch ${localBranch}, skipping checkout`);
|
||||
} else {
|
||||
log.debug(`\u{1F4E5} fetching base branch (${baseBranch})...`);
|
||||
$("git", ["fetch", "--no-tags", "origin", baseBranch]);
|
||||
$("git", ["checkout", "-B", baseBranch, `origin/${baseBranch}`]);
|
||||
log.debug(`\u{1F33F} fetching PR #${pullNumber} (${headBranch})...`);
|
||||
$("git", ["fetch", "--no-tags", "origin", `pull/${pullNumber}/head:${headBranch}`]);
|
||||
$("git", ["checkout", headBranch]);
|
||||
log.debug(`\u{1F33F} fetching PR #${pullNumber} (${localBranch})...`);
|
||||
$("git", ["fetch", "--no-tags", "origin", `pull/${pullNumber}/head:${localBranch}`]);
|
||||
$("git", ["checkout", localBranch]);
|
||||
log.debug(`\u2713 checked out PR #${pullNumber}`);
|
||||
}
|
||||
if (alreadyOnBranch) {
|
||||
@@ -123042,15 +123022,17 @@ async function checkoutPrBranch(params) {
|
||||
$("git", ["remote", "set-url", remoteName, forkUrl]);
|
||||
log.debug(`\u{1F4CC} updated remote '${remoteName}' for fork ${headRepo.full_name}`);
|
||||
}
|
||||
$("git", ["config", `branch.${headBranch}.pushRemote`, remoteName]);
|
||||
log.debug(`\u{1F4CC} configured branch '${headBranch}' to push to '${remoteName}'`);
|
||||
$("git", ["config", `branch.${localBranch}.pushRemote`, remoteName]);
|
||||
$("git", ["config", `branch.${localBranch}.merge`, `refs/heads/${headBranch}`]);
|
||||
log.debug(`\u{1F4CC} configured branch '${localBranch}' to push to '${remoteName}/${headBranch}'`);
|
||||
if (!pr.data.maintainer_can_modify) {
|
||||
log.warning(
|
||||
`\u26A0\uFE0F fork PR has maintainer_can_modify=false - push operations will fail. ask the PR author to enable "Allow edits from maintainers" or the fork may be owned by an organization.`
|
||||
);
|
||||
}
|
||||
} else {
|
||||
$("git", ["config", `branch.${headBranch}.pushRemote`, "origin"]);
|
||||
$("git", ["config", `branch.${localBranch}.pushRemote`, "origin"]);
|
||||
$("git", ["config", `branch.${localBranch}.merge`, `refs/heads/${headBranch}`]);
|
||||
}
|
||||
return { prNumber: pullNumber };
|
||||
}
|
||||
@@ -123077,6 +123059,12 @@ function CheckoutPrTool(ctx) {
|
||||
if (!headRepo) {
|
||||
throw new Error(`PR #${pull_number} source repository was deleted`);
|
||||
}
|
||||
const diffResponse = await ctx.octokit.rest.pulls.get({
|
||||
owner: ctx.owner,
|
||||
repo: ctx.name,
|
||||
pull_number,
|
||||
mediaType: { format: "diff" }
|
||||
});
|
||||
return {
|
||||
success: true,
|
||||
number: pr.data.number,
|
||||
@@ -123086,7 +123074,8 @@ function CheckoutPrTool(ctx) {
|
||||
isFork: headRepo.full_name !== pr.data.base.repo.full_name,
|
||||
maintainerCanModify: pr.data.maintainer_can_modify,
|
||||
url: pr.data.html_url,
|
||||
headRepo: headRepo.full_name
|
||||
headRepo: headRepo.full_name,
|
||||
diff: diffResponse.data
|
||||
};
|
||||
})
|
||||
});
|
||||
@@ -124095,8 +124084,15 @@ function PushBranchTool(_ctx) {
|
||||
remote = $("git", ["config", `branch.${branch}.pushRemote`], { log: false }).trim();
|
||||
} catch {
|
||||
}
|
||||
const args3 = force ? ["push", "--force", "-u", remote, branch] : ["push", "-u", remote, branch];
|
||||
log.debug(`pushing branch ${branch} to ${remote}`);
|
||||
let remoteBranch = branch;
|
||||
try {
|
||||
const mergeRef = $("git", ["config", `branch.${branch}.merge`], { log: false }).trim();
|
||||
remoteBranch = mergeRef.replace("refs/heads/", "");
|
||||
} catch {
|
||||
}
|
||||
const refspec = branch === remoteBranch ? branch : `${branch}:${remoteBranch}`;
|
||||
const args3 = force ? ["push", "--force", "-u", remote, refspec] : ["push", "-u", remote, refspec];
|
||||
log.debug(`pushing ${branch} to ${remote}/${remoteBranch}`);
|
||||
if (force) {
|
||||
log.warning(`force pushing - this will overwrite remote history`);
|
||||
}
|
||||
@@ -124104,9 +124100,10 @@ function PushBranchTool(_ctx) {
|
||||
return {
|
||||
success: true,
|
||||
branch,
|
||||
remoteBranch,
|
||||
remote,
|
||||
force,
|
||||
message: `successfully pushed branch ${branch}`
|
||||
message: `successfully pushed ${branch} to ${remote}/${remoteBranch}`
|
||||
};
|
||||
})
|
||||
});
|
||||
@@ -124422,9 +124419,6 @@ function PullRequestInfoTool(ctx) {
|
||||
}
|
||||
|
||||
// mcp/review.ts
|
||||
import { randomBytes } from "node:crypto";
|
||||
import { writeFileSync as writeFileSync4 } from "node:fs";
|
||||
import { join as join10 } from "node:path";
|
||||
var ADD_PULL_REQUEST_REVIEW_THREAD = `
|
||||
mutation AddPullRequestReviewThread($pullRequestReviewId: ID!, $path: String!, $line: Int!, $body: String!, $side: DiffSide) {
|
||||
addPullRequestReviewThread(input: {
|
||||
@@ -124459,7 +124453,7 @@ var StartReview = type({
|
||||
function StartReviewTool(ctx) {
|
||||
return tool({
|
||||
name: "start_review",
|
||||
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.",
|
||||
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) {
|
||||
@@ -124504,21 +124498,15 @@ function StartReviewTool(ctx) {
|
||||
throw error41;
|
||||
}
|
||||
}
|
||||
const scratchpadId = randomBytes(4).toString("hex");
|
||||
const scratchpadPath = join10(ctx.sharedTempDir, `pullfrog-review-${scratchpadId}.md`);
|
||||
const scratchpadContent = `# Review ${scratchpadId}
|
||||
|
||||
`;
|
||||
writeFileSync4(scratchpadPath, scratchpadContent);
|
||||
ctx.toolState.prNumber = pull_number;
|
||||
ctx.toolState.review = {
|
||||
nodeId: reviewNodeId,
|
||||
id: reviewId
|
||||
};
|
||||
log.debug(`review session started: id=${reviewId}, nodeId=${reviewNodeId}`);
|
||||
return {
|
||||
reviewId: scratchpadId,
|
||||
scratchpadPath,
|
||||
message: `Review session started. Use the scratchpad file to gather your thoughts, then call add_review_comment for each comment.`
|
||||
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."
|
||||
};
|
||||
})
|
||||
});
|
||||
@@ -124540,7 +124528,10 @@ function AddReviewCommentTool(ctx) {
|
||||
if (!ctx.toolState.review) {
|
||||
throw new Error("No review session started. Call start_review first.");
|
||||
}
|
||||
await ctx.octokit.graphql(
|
||||
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,
|
||||
@@ -124550,9 +124541,11 @@ function AddReviewCommentTool(ctx) {
|
||||
side: side || "RIGHT"
|
||||
}
|
||||
);
|
||||
log.debug(`review comment added: threadId=${result.addPullRequestReviewThread.thread.id}`);
|
||||
return {
|
||||
success: true,
|
||||
message: `Comment added to ${path4}:${line}`
|
||||
message: `Comment added to ${path4}:${line}`,
|
||||
threadId: result.addPullRequestReviewThread.thread.id
|
||||
};
|
||||
})
|
||||
});
|
||||
@@ -124575,6 +124568,9 @@ function SubmitReviewTool(ctx) {
|
||||
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}`;
|
||||
@@ -124591,6 +124587,7 @@ function SubmitReviewTool(ctx) {
|
||||
event: "COMMENT",
|
||||
body: bodyWithFooter
|
||||
});
|
||||
log.debug(`review submitted: reviewId=${result.data.id}, state=${result.data.state}`);
|
||||
delete ctx.toolState.review;
|
||||
await deleteProgressComment(ctx);
|
||||
return {
|
||||
@@ -124621,9 +124618,8 @@ var Review = type({
|
||||
),
|
||||
start_line: type.number.describe("Start line for multi-line comments (optional, for commenting on ranges)").optional()
|
||||
}).array().describe(
|
||||
// FORK PR NOTE: use HEAD not origin/<head> - for fork PRs, origin/<head> doesn't exist
|
||||
// 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 '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."
|
||||
// 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()
|
||||
});
|
||||
|
||||
@@ -125245,7 +125241,7 @@ function resolveAgent({
|
||||
return agent2;
|
||||
}
|
||||
async function createTempDirectory() {
|
||||
const sharedTempDir = await mkdtemp2(join11(tmpdir2(), "pullfrog-"));
|
||||
const sharedTempDir = await mkdtemp2(join10(tmpdir2(), "pullfrog-"));
|
||||
process.env.PULLFROG_TEMP_DIR = sharedTempDir;
|
||||
log.info(`\u{1F4C2} PULLFROG_TEMP_DIR has been created at ${sharedTempDir}`);
|
||||
return sharedTempDir;
|
||||
|
||||
+28
-10
@@ -19,6 +19,7 @@ export type CheckoutPrResult = {
|
||||
maintainerCanModify: boolean;
|
||||
url: string;
|
||||
headRepo: string;
|
||||
diff: string;
|
||||
};
|
||||
|
||||
interface CheckoutPrBranchParams {
|
||||
@@ -60,12 +61,17 @@ export async function checkoutPrBranch(
|
||||
const baseBranch = pr.data.base.ref;
|
||||
const headBranch = pr.data.head.ref;
|
||||
|
||||
// check if we're already on the correct branch
|
||||
const currentBranch = $("git", ["rev-parse", "--abbrev-ref", "HEAD"], { log: false }).trim();
|
||||
const alreadyOnBranch = currentBranch === headBranch;
|
||||
// always use pr-{number} as local branch name for consistency
|
||||
// this avoids naming conflicts and makes push config simpler
|
||||
const localBranch = `pr-${pullNumber}`;
|
||||
|
||||
// check if we're already on the correct commit (not just branch name)
|
||||
// this handles fork PRs where head branch name might match base branch name
|
||||
const currentSha = $("git", ["rev-parse", "HEAD"], { log: false }).trim();
|
||||
const alreadyOnBranch = currentSha === pr.data.head.sha;
|
||||
|
||||
if (alreadyOnBranch) {
|
||||
log.debug(`already on PR branch ${headBranch}, skipping checkout`);
|
||||
log.debug(`already on PR branch ${localBranch}, skipping checkout`);
|
||||
} else {
|
||||
// fetch base branch so origin/<base> exists for diff operations
|
||||
log.debug(`📥 fetching base branch (${baseBranch})...`);
|
||||
@@ -76,11 +82,11 @@ export async function checkoutPrBranch(
|
||||
$("git", ["checkout", "-B", baseBranch, `origin/${baseBranch}`]);
|
||||
|
||||
// fetch PR branch using pull/{n}/head refspec (works for both fork and same-repo PRs)
|
||||
log.debug(`🌿 fetching PR #${pullNumber} (${headBranch})...`);
|
||||
$("git", ["fetch", "--no-tags", "origin", `pull/${pullNumber}/head:${headBranch}`]);
|
||||
log.debug(`🌿 fetching PR #${pullNumber} (${localBranch})...`);
|
||||
$("git", ["fetch", "--no-tags", "origin", `pull/${pullNumber}/head:${localBranch}`]);
|
||||
|
||||
// checkout the branch
|
||||
$("git", ["checkout", headBranch]);
|
||||
$("git", ["checkout", localBranch]);
|
||||
log.debug(`✓ checked out PR #${pullNumber}`);
|
||||
}
|
||||
|
||||
@@ -109,8 +115,10 @@ export async function checkoutPrBranch(
|
||||
}
|
||||
|
||||
// set branch push config so `git push` knows where to push
|
||||
$("git", ["config", `branch.${headBranch}.pushRemote`, remoteName]);
|
||||
log.debug(`📌 configured branch '${headBranch}' to push to '${remoteName}'`);
|
||||
$("git", ["config", `branch.${localBranch}.pushRemote`, remoteName]);
|
||||
// set merge ref so git knows the remote branch name (may differ from local)
|
||||
$("git", ["config", `branch.${localBranch}.merge`, `refs/heads/${headBranch}`]);
|
||||
log.debug(`📌 configured branch '${localBranch}' to push to '${remoteName}/${headBranch}'`);
|
||||
|
||||
// warn if maintainer can't modify (push will likely fail)
|
||||
if (!pr.data.maintainer_can_modify) {
|
||||
@@ -121,7 +129,8 @@ export async function checkoutPrBranch(
|
||||
}
|
||||
} else {
|
||||
// for same-repo PRs, push to origin
|
||||
$("git", ["config", `branch.${headBranch}.pushRemote`, "origin"]);
|
||||
$("git", ["config", `branch.${localBranch}.pushRemote`, "origin"]);
|
||||
$("git", ["config", `branch.${localBranch}.merge`, `refs/heads/${headBranch}`]);
|
||||
}
|
||||
|
||||
return { prNumber: pullNumber };
|
||||
@@ -157,6 +166,14 @@ export function CheckoutPrTool(ctx: ToolContext) {
|
||||
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 {
|
||||
success: true,
|
||||
number: pr.data.number,
|
||||
@@ -167,6 +184,7 @@ export function CheckoutPrTool(ctx: ToolContext) {
|
||||
maintainerCanModify: pr.data.maintainer_can_modify,
|
||||
url: pr.data.html_url,
|
||||
headRepo: headRepo.full_name,
|
||||
diff: diffResponse.data as unknown as string,
|
||||
} satisfies CheckoutPrResult;
|
||||
}),
|
||||
});
|
||||
|
||||
+18
-5
@@ -158,11 +158,23 @@ export function PushBranchTool(_ctx: ToolContext) {
|
||||
// no configured pushRemote, default to origin
|
||||
}
|
||||
|
||||
const args = force
|
||||
? ["push", "--force", "-u", remote, branch]
|
||||
: ["push", "-u", remote, branch];
|
||||
// check if branch has a configured merge ref (remote branch name may differ from local)
|
||||
let remoteBranch = branch;
|
||||
try {
|
||||
const mergeRef = $("git", ["config", `branch.${branch}.merge`], { log: false }).trim();
|
||||
// merge ref is like "refs/heads/main", extract the branch name
|
||||
remoteBranch = mergeRef.replace("refs/heads/", "");
|
||||
} catch {
|
||||
// no configured merge ref, use local branch name
|
||||
}
|
||||
|
||||
log.debug(`pushing branch ${branch} to ${remote}`);
|
||||
// use refspec when local and remote branch names differ
|
||||
const refspec = branch === remoteBranch ? branch : `${branch}:${remoteBranch}`;
|
||||
const args = force
|
||||
? ["push", "--force", "-u", remote, refspec]
|
||||
: ["push", "-u", remote, refspec];
|
||||
|
||||
log.debug(`pushing ${branch} to ${remote}/${remoteBranch}`);
|
||||
if (force) {
|
||||
log.warning(`force pushing - this will overwrite remote history`);
|
||||
}
|
||||
@@ -171,9 +183,10 @@ export function PushBranchTool(_ctx: ToolContext) {
|
||||
return {
|
||||
success: true,
|
||||
branch,
|
||||
remoteBranch,
|
||||
remote,
|
||||
force,
|
||||
message: `successfully pushed branch ${branch}`,
|
||||
message: `successfully pushed ${branch} to ${remote}/${remoteBranch}`,
|
||||
};
|
||||
}),
|
||||
});
|
||||
|
||||
+22
-17
@@ -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 } from "arktype";
|
||||
import type { ToolContext } from "../main.ts";
|
||||
@@ -65,7 +62,7 @@ export function StartReviewTool(ctx: ToolContext) {
|
||||
return tool({
|
||||
name: "start_review",
|
||||
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,
|
||||
execute: execute(async ({ pull_number }) => {
|
||||
// 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
|
||||
ctx.toolState.prNumber = pull_number;
|
||||
ctx.toolState.review = {
|
||||
@@ -132,10 +123,13 @@ export function StartReviewTool(ctx: ToolContext) {
|
||||
id: reviewId,
|
||||
};
|
||||
|
||||
log.debug(`review session started: id=${reviewId}, nodeId=${reviewNodeId}`);
|
||||
|
||||
return {
|
||||
reviewId: scratchpadId,
|
||||
scratchpadPath,
|
||||
message: `Review session started. Use the scratchpad file to gather your thoughts, then call add_review_comment for each comment.`,
|
||||
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.",
|
||||
};
|
||||
}),
|
||||
});
|
||||
@@ -166,8 +160,12 @@ export function AddReviewCommentTool(ctx: ToolContext) {
|
||||
throw new Error("No review session started. Call start_review first.");
|
||||
}
|
||||
|
||||
log.debug(
|
||||
`adding review comment: reviewNodeId=${ctx.toolState.review.nodeId}, path=${path}, line=${line}, side=${side || "RIGHT"}`
|
||||
);
|
||||
|
||||
// add comment thread via GraphQL (REST doesn't support adding to existing pending review)
|
||||
await ctx.octokit.graphql<AddPullRequestReviewThreadResponse>(
|
||||
const result = await ctx.octokit.graphql<AddPullRequestReviewThreadResponse>(
|
||||
ADD_PULL_REQUEST_REVIEW_THREAD,
|
||||
{
|
||||
pullRequestReviewId: ctx.toolState.review.nodeId,
|
||||
@@ -178,9 +176,12 @@ export function AddReviewCommentTool(ctx: ToolContext) {
|
||||
}
|
||||
);
|
||||
|
||||
log.debug(`review comment added: threadId=${result.addPullRequestReviewThread.thread.id}`);
|
||||
|
||||
return {
|
||||
success: true,
|
||||
message: `Comment added to ${path}:${line}`,
|
||||
threadId: result.addPullRequestReviewThread.thread.id,
|
||||
};
|
||||
}),
|
||||
});
|
||||
@@ -211,6 +212,9 @@ export function SubmitReviewTool(ctx: ToolContext) {
|
||||
}
|
||||
|
||||
const reviewId = ctx.toolState.review.id;
|
||||
log.debug(
|
||||
`submitting review: id=${reviewId}, nodeId=${ctx.toolState.review.nodeId}, prNumber=${ctx.toolState.prNumber}`
|
||||
);
|
||||
|
||||
// build quick links footer
|
||||
const apiUrl = process.env.API_URL || "https://pullfrog.com";
|
||||
@@ -234,6 +238,8 @@ export function SubmitReviewTool(ctx: ToolContext) {
|
||||
body: bodyWithFooter,
|
||||
});
|
||||
|
||||
log.debug(`review submitted: reviewId=${result.data.id}, state=${result.data.state}`);
|
||||
|
||||
// clear review state
|
||||
delete ctx.toolState.review;
|
||||
|
||||
@@ -281,9 +287,8 @@ export const Review = type({
|
||||
})
|
||||
.array()
|
||||
.describe(
|
||||
// FORK PR NOTE: use HEAD not origin/<head> - for fork PRs, origin/<head> doesn't exist
|
||||
// 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 '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."
|
||||
// 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(),
|
||||
});
|
||||
|
||||
@@ -100,29 +100,26 @@ ${
|
||||
description:
|
||||
"Review code, PRs, or implementations; provide feedback or suggestions; identify issues; or check code quality, style, and correctness",
|
||||
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:
|
||||
- Summarize what changes this PR makes
|
||||
- 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.
|
||||
- If approach is sound, analyze implementation - consider potential issues per file
|
||||
- Identify bugs, security issues, edge cases
|
||||
4. **BEFORE COMMENTING** - For each potential comment, ask yourself:
|
||||
- Is this a nitpick? Skip it 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?
|
||||
|
||||
5. **SELF-CRITIQUE** - Before adding comments, review your scratchpad:
|
||||
- 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
|
||||
5. 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 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.
|
||||
- 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)
|
||||
|
||||
**GENERAL GUIDANCE**
|
||||
|
||||
+1
-1
@@ -1,6 +1,6 @@
|
||||
{
|
||||
"name": "@pullfrog/action",
|
||||
"version": "0.0.148",
|
||||
"version": "0.0.150",
|
||||
"type": "module",
|
||||
"files": [
|
||||
"index.js",
|
||||
|
||||
+4
-4
@@ -36,8 +36,8 @@ export interface WorkflowRunInfo {
|
||||
export async function fetchWorkflowRunInfo(runId: string): Promise<WorkflowRunInfo> {
|
||||
const apiUrl = process.env.API_URL || "https://pullfrog.com";
|
||||
|
||||
// add timeout to prevent hanging (5 seconds)
|
||||
const timeoutMs = 5000;
|
||||
// add timeout to prevent hanging (30 seconds)
|
||||
const timeoutMs = 30000;
|
||||
const controller = new AbortController();
|
||||
const timeoutId = setTimeout(() => controller.abort(), timeoutMs);
|
||||
|
||||
@@ -90,8 +90,8 @@ export async function getRepoSettings(
|
||||
): Promise<RepoSettings> {
|
||||
const apiUrl = process.env.API_URL || "https://pullfrog.com";
|
||||
|
||||
// Add timeout to prevent hanging (5 seconds)
|
||||
const timeoutMs = 5000;
|
||||
// Add timeout to prevent hanging (30 seconds)
|
||||
const timeoutMs = 30000;
|
||||
const controller = new AbortController();
|
||||
const timeoutId = setTimeout(() => controller.abort(), timeoutMs);
|
||||
|
||||
|
||||
+5
-5
@@ -341,12 +341,12 @@ export const log = {
|
||||
* Log tool call information to console with formatted output
|
||||
*/
|
||||
toolCall: ({ toolName, input }: { toolName: string; input: unknown }): void => {
|
||||
let output = `→ ${toolName}\n`;
|
||||
|
||||
const inputFormatted = formatJsonValue(input);
|
||||
if (inputFormatted !== "{}") {
|
||||
output += formatIndentedField("input", inputFormatted);
|
||||
}
|
||||
const timestamp = isDebugEnabled() ? ` [${new Date().toISOString()}]` : "";
|
||||
const output =
|
||||
inputFormatted !== "{}"
|
||||
? `→ ${toolName}(${inputFormatted})${timestamp}`
|
||||
: `→ ${toolName}()${timestamp}`;
|
||||
|
||||
log.info(output.trimEnd());
|
||||
},
|
||||
|
||||
+5
-6
@@ -48,17 +48,16 @@ function isGitHubActionsEnvironment(): boolean {
|
||||
}
|
||||
|
||||
async function acquireTokenViaOIDC(): Promise<string> {
|
||||
log.debug("» generating OIDC token...");
|
||||
log.info("» generating OIDC token...");
|
||||
|
||||
const oidcToken = await core.getIDToken("pullfrog-api");
|
||||
log.debug("» OIDC token generated successfully");
|
||||
|
||||
const apiUrl = process.env.API_URL || "https://pullfrog.com";
|
||||
|
||||
log.debug("» exchanging OIDC token for installation token...");
|
||||
log.info("» exchanging OIDC token for installation token...");
|
||||
|
||||
// Add timeout to prevent long waits (5 seconds)
|
||||
const timeoutMs = 5000;
|
||||
// Add timeout to prevent long waits (30 seconds)
|
||||
const timeoutMs = 30000;
|
||||
const controller = new AbortController();
|
||||
const timeoutId = setTimeout(() => controller.abort(), timeoutMs);
|
||||
|
||||
@@ -79,7 +78,7 @@ async function acquireTokenViaOIDC(): Promise<string> {
|
||||
}
|
||||
|
||||
const tokenData = (await tokenResponse.json()) as InstallationToken;
|
||||
log.debug(`» installation token obtained for ${tokenData.repository || "all repositories"}`);
|
||||
log.info(`» installation token obtained for ${tokenData.repository || "all repositories"}`);
|
||||
|
||||
return tokenData.token;
|
||||
} catch (error) {
|
||||
|
||||
+1
-1
@@ -77,7 +77,7 @@ interface SetupGitAuthParams {
|
||||
* FORK PR ARCHITECTURE:
|
||||
* - origin: always points to BASE REPO (where PR targets)
|
||||
* - 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> {
|
||||
const repoDir = process.cwd();
|
||||
|
||||
Reference in New Issue
Block a user