diff --git a/agents/instructions.ts b/agents/instructions.ts index deeaabc..69fd391 100644 --- a/agents/instructions.ts +++ b/agents/instructions.ts @@ -8,84 +8,84 @@ import { getModes } from "../modes.ts"; * Removes verbose GitHub API metadata (user objects, repository metadata, etc.) * and keeps only the fields agents actually need. */ -function extractEssentialEventData(event: Payload["event"]): Record { - const trigger = event.trigger; - const essential: Record = { trigger }; +// function extractEssentialEventData(event: Payload["event"]): Record { +// const trigger = event.trigger; +// const essential: Record = { trigger }; - // common fields - if ("issue_number" in event) { - essential.issue_number = event.issue_number; - } - if ("branch" in event && event.branch) { - essential.branch = event.branch; - } +// // common fields +// if ("issue_number" in event) { +// essential.issue_number = event.issue_number; +// } +// if ("branch" in event && event.branch) { +// essential.branch = event.branch; +// } - // trigger-specific fields - switch (trigger) { - case "issue_comment_created": - if ("comment_id" in event) essential.comment_id = event.comment_id; - if ("comment_body" in event) essential.comment_body = event.comment_body; - // include issue title/body if available in context (but not the entire context object) - if ("context" in event && event.context && typeof event.context === "object") { - const ctx = event.context as Record; - if (ctx.issue && typeof ctx.issue === "object") { - const issue = ctx.issue as Record; - if (issue.title) essential.issue_title = issue.title; - if (issue.body) essential.issue_body = issue.body; - } - } - break; +// // trigger-specific fields +// switch (trigger) { +// case "issue_comment_created": +// if ("comment_id" in event) essential.comment_id = event.comment_id; +// if ("comment_body" in event) essential.comment_body = event.comment_body; +// // include issue title/body if available in context (but not the entire context object) +// if ("context" in event && event.context && typeof event.context === "object") { +// const ctx = event.context as Record; +// if (ctx.issue && typeof ctx.issue === "object") { +// const issue = ctx.issue as Record; +// if (issue.title) essential.issue_title = issue.title; +// if (issue.body) essential.issue_body = issue.body; +// } +// } +// break; - case "issues_opened": - case "issues_assigned": - case "issues_labeled": - if ("issue_title" in event) essential.issue_title = event.issue_title; - if ("issue_body" in event) essential.issue_body = event.issue_body; - break; +// case "issues_opened": +// case "issues_assigned": +// case "issues_labeled": +// if ("issue_title" in event) essential.issue_title = event.issue_title; +// if ("issue_body" in event) essential.issue_body = event.issue_body; +// break; - case "pull_request_opened": - case "pull_request_review_requested": - if ("pr_title" in event) essential.pr_title = event.pr_title; - if ("pr_body" in event) essential.pr_body = event.pr_body; - if ("branch" in event) essential.branch = event.branch; - break; +// case "pull_request_opened": +// case "pull_request_review_requested": +// if ("pr_title" in event) essential.pr_title = event.pr_title; +// if ("pr_body" in event) essential.pr_body = event.pr_body; +// if ("branch" in event) essential.branch = event.branch; +// break; - case "pull_request_review_submitted": - if ("review_id" in event) essential.review_id = event.review_id; - if ("review_body" in event) essential.review_body = event.review_body; - if ("review_state" in event) essential.review_state = event.review_state; - if ("branch" in event) essential.branch = event.branch; - break; +// case "pull_request_review_submitted": +// if ("review_id" in event) essential.review_id = event.review_id; +// if ("review_body" in event) essential.review_body = event.review_body; +// if ("review_state" in event) essential.review_state = event.review_state; +// if ("branch" in event) essential.branch = event.branch; +// break; - case "pull_request_review_comment_created": - if ("comment_id" in event) essential.comment_id = event.comment_id; - if ("comment_body" in event) essential.comment_body = event.comment_body; - if ("pr_title" in event) essential.pr_title = event.pr_title; - if ("branch" in event) essential.branch = event.branch; - break; +// case "pull_request_review_comment_created": +// if ("comment_id" in event) essential.comment_id = event.comment_id; +// if ("comment_body" in event) essential.comment_body = event.comment_body; +// if ("pr_title" in event) essential.pr_title = event.pr_title; +// if ("branch" in event) essential.branch = event.branch; +// break; - case "check_suite_completed": - if ("pr_title" in event) essential.pr_title = event.pr_title; - if ("pr_body" in event) essential.pr_body = event.pr_body; - if ("branch" in event) essential.branch = event.branch; - if ("check_suite" in event) { - essential.check_suite = { - id: event.check_suite.id, - head_sha: event.check_suite.head_sha, - head_branch: event.check_suite.head_branch, - status: event.check_suite.status, - conclusion: event.check_suite.conclusion, - }; - } - break; +// case "check_suite_completed": +// if ("pr_title" in event) essential.pr_title = event.pr_title; +// if ("pr_body" in event) essential.pr_body = event.pr_body; +// if ("branch" in event) essential.branch = event.branch; +// if ("check_suite" in event) { +// essential.check_suite = { +// id: event.check_suite.id, +// head_sha: event.check_suite.head_sha, +// head_branch: event.check_suite.head_branch, +// status: event.check_suite.status, +// conclusion: event.check_suite.conclusion, +// }; +// } +// break; - case "workflow_dispatch": - if ("inputs" in event) essential.inputs = event.inputs; - break; - } +// case "workflow_dispatch": +// if ("inputs" in event) essential.inputs = event.inputs; +// break; +// } - return essential; -} +// return essential; +// } export const addInstructions = (payload: Payload) => { let encodedEvent = ""; @@ -95,8 +95,8 @@ export const addInstructions = (payload: Payload) => { // no meaningful event data to encode } else { // extract only essential fields to reduce token usage - const essentialEvent = extractEssentialEventData(payload.event); - encodedEvent = toonEncode(essentialEvent); + // const essentialEvent = payload.event; + encodedEvent = toonEncode(payload.event); } return ` @@ -168,7 +168,7 @@ MCP servers provide tools you can call. Inspect your available MCP servers at st Tool names may be formatted as \`(server name)/(tool name)\`, for example: \`${ghPullfrogMcpName}/create_issue_comment\` -**GitHub CLI prohibition**: Do not use the \`gh\` CLI under any circumstances. Use the corresponding tool from ${ghPullfrogMcpName} instead. +**GitHub CLI**: Prefer using MCP tools from ${ghPullfrogMcpName} for GitHub operations. The \`gh\` CLI is available as a fallback if needed, but MCP tools handle authentication and provide better integration. **Git operations**: All git operations must use ${ghPullfrogMcpName} MCP tools to ensure proper authentication and commit attribution. Do NOT use git commands directly (e.g., \`git commit\`, \`git push\`, \`git checkout\`, \`git branch\`) - these will use incorrect credentials and attribute commits to the wrong author. diff --git a/external.ts b/external.ts index ee34a66..42d7806 100644 --- a/external.ts +++ b/external.ts @@ -51,117 +51,171 @@ export const AgentName = type.enumerated(...Object.keys(agentsManifest)); export type AgentApiKeyName = (typeof agentsManifest)[AgentName]["apiKeyNames"][number]; +// base interface for common payload event fields +interface BasePayloadEvent { + issue_number?: number; + is_pr?: boolean; + branch?: string; + pr_title?: string; + pr_body?: string | null; + issue_title?: string; + issue_body?: string | null; + comment_id?: number; + comment_body?: string; + review_id?: number; + review_body?: string | null; + review_state?: string; + review_comments?: any[]; + context?: any; + thread?: any; + pull_request?: any; + check_suite?: { + id: number; + head_sha: string; + head_branch: string | null; + status: string | null; + conclusion: string | null; + url: string; + }; + comment_ids?: number[] | "all"; + [key: string]: any; +} + +interface PullRequestOpenedEvent extends BasePayloadEvent { + trigger: "pull_request_opened"; + issue_number: number; + is_pr: true; + pr_title: string; + pr_body: string | null; + branch: string; +} + +interface PullRequestReadyForReviewEvent extends BasePayloadEvent { + trigger: "pull_request_ready_for_review"; + issue_number: number; + is_pr: true; + pr_title: string; + pr_body: string | null; + branch: string; +} + +interface PullRequestReviewRequestedEvent extends BasePayloadEvent { + trigger: "pull_request_review_requested"; + issue_number: number; + is_pr: true; + pr_title: string; + pr_body: string | null; + branch: string; +} + +interface PullRequestReviewSubmittedEvent extends BasePayloadEvent { + trigger: "pull_request_review_submitted"; + issue_number: number; + is_pr: true; + review_id: number; + review_body: string | null; + review_state: string; + review_comments: any[]; + context: any; + branch: string; +} + +interface PullRequestReviewCommentCreatedEvent extends BasePayloadEvent { + trigger: "pull_request_review_comment_created"; + issue_number: number; + is_pr: true; + pr_title: string; + comment_id: number; + comment_body: string; + thread?: any; + branch: string; +} + +interface IssuesOpenedEvent extends BasePayloadEvent { + trigger: "issues_opened"; + issue_number: number; + issue_title: string; + issue_body: string | null; +} + +interface IssuesAssignedEvent extends BasePayloadEvent { + trigger: "issues_assigned"; + issue_number: number; + issue_title: string; + issue_body: string | null; +} + +interface IssuesLabeledEvent extends BasePayloadEvent { + trigger: "issues_labeled"; + issue_number: number; + issue_title: string; + issue_body: string | null; +} + +interface IssueCommentCreatedEvent extends BasePayloadEvent { + trigger: "issue_comment_created"; + comment_id: number; + comment_body: string; + issue_number: number; + // PR-specific fields (only present when is_pr is true) + is_pr?: true; + branch?: string; + pr_title?: string; + pr_body?: string | null; +} + +interface CheckSuiteCompletedEvent extends BasePayloadEvent { + trigger: "check_suite_completed"; + issue_number: number; + is_pr: true; + pr_title: string; + pr_body: string | null; + pull_request: any; + branch: string; + check_suite: { + id: number; + head_sha: string; + head_branch: string | null; + status: string | null; + conclusion: string | null; + url: string; + }; +} + +interface WorkflowDispatchEvent extends BasePayloadEvent { + trigger: "workflow_dispatch"; +} + +interface FixReviewEvent extends BasePayloadEvent { + trigger: "fix_review"; + issue_number: number; + is_pr: true; + review_id: number; + /** "all" to fix all comments, or specific comment IDs to fix */ + comment_ids: number[] | "all"; + branch: string; +} + +interface UnknownEvent extends BasePayloadEvent { + trigger: "unknown"; +} + // discriminated union for payload event based on trigger // note: all events use issue_number for consistency (PRs are issues in GitHub's API) export type PayloadEvent = - | { - trigger: "pull_request_opened"; - issue_number: number; - pr_title: string; - pr_body: string | null; - branch: string; - [key: string]: any; - } - | { - trigger: "pull_request_ready_for_review"; - issue_number: number; - pr_title: string; - pr_body: string | null; - branch: string; - [key: string]: any; - } - | { - trigger: "pull_request_review_requested"; - issue_number: number; - pr_title: string; - pr_body: string | null; - branch: string; - [key: string]: any; - } - | { - trigger: "pull_request_review_submitted"; - issue_number: number; - review_id: number; - review_body: string | null; - review_state: string; - review_comments: any[]; - context: any; - branch: string; - [key: string]: any; - } - | { - trigger: "pull_request_review_comment_created"; - issue_number: number; - pr_title: string; - comment_id: number; - comment_body: string; - thread?: any; - branch: string; - [key: string]: any; - } - | { - trigger: "issues_opened"; - issue_number: number; - issue_title: string; - issue_body: string | null; - [key: string]: any; - } - | { - trigger: "issues_assigned"; - issue_number: number; - issue_title: string; - issue_body: string | null; - [key: string]: any; - } - | { - trigger: "issues_labeled"; - issue_number: number; - issue_title: string; - issue_body: string | null; - [key: string]: any; - } - | { - trigger: "issue_comment_created"; - comment_id: number; - comment_body: string; - issue_number: number; - branch?: string; - [key: string]: any; - } - | { - trigger: "check_suite_completed"; - issue_number: number; - pr_title: string; - pr_body: string | null; - pull_request: any; - branch: string; - check_suite: { - id: number; - head_sha: string; - head_branch: string | null; - status: string | null; - conclusion: string | null; - url: string; - }; - [key: string]: any; - } - | { - trigger: "workflow_dispatch"; - [key: string]: any; - } - | { - trigger: "fix_review"; - issue_number: number; - review_id: number; - /** "all" to fix all comments, or specific comment IDs to fix */ - comment_ids: number[] | "all"; - branch: string; - [key: string]: any; - } - | { - trigger: "unknown"; - [key: string]: any; - }; + | PullRequestOpenedEvent + | PullRequestReadyForReviewEvent + | PullRequestReviewRequestedEvent + | PullRequestReviewSubmittedEvent + | PullRequestReviewCommentCreatedEvent + | IssuesOpenedEvent + | IssuesAssignedEvent + | IssuesLabeledEvent + | IssueCommentCreatedEvent + | CheckSuiteCompletedEvent + | WorkflowDispatchEvent + | FixReviewEvent + | UnknownEvent; export interface DispatchOptions { /** diff --git a/mcp/README.md b/mcp/README.md index af7ceb9..e6d361b 100644 --- a/mcp/README.md +++ b/mcp/README.md @@ -111,7 +111,7 @@ see individual files for documentation on other tools: ## usage in agents -agents should never use the `gh` cli. instead, they should use the mcp tools provided by this server. +agents should prefer using the mcp tools provided by this server. the `gh` cli is available as a fallback if needed, but mcp tools handle authentication and provide better integration. the agent instructions automatically include guidance on using these tools. diff --git a/mcp/prInfo.ts b/mcp/prInfo.ts index 554de93..ad8039c 100644 --- a/mcp/prInfo.ts +++ b/mcp/prInfo.ts @@ -9,8 +9,7 @@ export const PullRequestInfo = type({ export const PullRequestInfoTool = tool({ name: "get_pull_request", - description: - "Retrieve PR information and automatically prepare the repository for review by fetching and checking out the PR branch.", + description: "Retrieve PR information. Automatically fetches and checks out the PR branch.", parameters: PullRequestInfo, execute: contextualize(async ({ pull_number }, ctx) => { const pr = await ctx.octokit.rest.pulls.get({ @@ -28,18 +27,31 @@ export const PullRequestInfoTool = tool({ throw new Error(`Base branch not found for PR #${pull_number}`); } - // Automatically fetch and checkout branches for review + // detect fork PRs - head repo differs from base repo + const baseRepo = data.base.repo.full_name; + const headRepo = data.head.repo.full_name; + const isFork = headRepo !== baseRepo; + + // use gh pr checkout which handles fork PRs automatically + // it adds the fork as a remote if needed and checks out the PR branch + log.info(`Checking out PR #${pull_number} using gh pr checkout`); + $("gh", ["pr", "checkout", pull_number.toString()]); + + // fetch base branch for diff comparison log.info(`Fetching base branch: origin/${baseBranch}`); $("git", ["fetch", "origin", baseBranch, "--depth=20"]); - // use GitHub's PR ref which works for both fork and non-fork PRs - // refs/pull/{number}/head always points to the PR head commit - log.info(`Fetching PR #${pull_number} using refs/pull/${pull_number}/head`); - $("git", ["fetch", "origin", `refs/pull/${pull_number}/head`]); + // get current git status for summary + const currentBranch = $("git", ["rev-parse", "--abbrev-ref", "HEAD"], { log: false }).trim(); + const currentSha = $("git", ["rev-parse", "HEAD"], { log: false }).trim(); + const baseSha = $("git", ["rev-parse", `origin/${baseBranch}`], { log: false }).trim(); - log.info(`Checking out PR branch: ${headBranch}`); - // check out a local branch from FETCH_HEAD (the PR ref we just fetched) - $("git", ["checkout", "-B", headBranch, "FETCH_HEAD"]); + // build summary + const summary = `PR branch has been fetched and checked out: +- Base branch: \`origin/${baseBranch}\` (${baseSha.substring(0, 7)}) +- PR branch: \`${headBranch}\` (checked out locally, ${currentSha.substring(0, 7)}) +- Current branch: \`${currentBranch}\` +- View diff: \`git diff origin/${baseBranch}...HEAD\``; return { number: data.number, @@ -50,6 +62,8 @@ export const PullRequestInfoTool = tool({ merged: data.merged, base: baseBranch, head: headBranch, + isFork, + summary, }; }), }); diff --git a/modes.ts b/modes.ts index 7d5a881..9fb8ffc 100644 --- a/modes.ts +++ b/modes.ts @@ -85,7 +85,7 @@ ${ prompt: `Follow these steps: 1. Get PR info with ${ghPullfrogMcpName}/get_pull_request (this automatically prepares the repository by fetching and checking out the PR branch) -2. View diff: \`git diff origin/...origin/\` (replace and with 'base' and 'head' from PR info) +2. **IMPORTANT**: After calling ${ghPullfrogMcpName}/get_pull_request, the PR branch is already checked out locally. View diff using: \`git diff origin/...HEAD\` (replace with 'base' from PR info). Do NOT use \`origin/\` - the branch is checked out locally, not as a remote tracking branch. 3. Read files from the checked-out PR branch to understand the implementation. Always use **relative paths** from repo root (e.g., \`src/index.ts\`), never absolute paths. diff --git a/utils/setup.ts b/utils/setup.ts index 35852fe..eecce85 100644 --- a/utils/setup.ts +++ b/utils/setup.ts @@ -109,22 +109,13 @@ export function setupGitBranch(payload: Payload): void { log.info(`🌿 Setting up git branch: ${branch}`); - // if event has issue_number and branch, it's likely a PR - try PR ref first (works for forks) - const issueNumber = "issue_number" in payload.event ? payload.event.issue_number : undefined; - const isLikelyPR = issueNumber !== undefined && branch !== undefined; - - if (isLikelyPR) { + // if this is a PR-related event, use gh pr checkout which handles fork PRs automatically + if (payload.event.is_pr === true && payload.event.issue_number) { + const prNumber = payload.event.issue_number; try { - // use GitHub's PR ref which works for both fork and non-fork PRs - log.debug(`Fetching PR #${issueNumber} using refs/pull/${issueNumber}/head`); - execSync(`git fetch origin refs/pull/${issueNumber}/head`, { - cwd: repoDir, - stdio: "pipe", - }); - - // checkout from FETCH_HEAD (the PR ref we just fetched) - log.debug(`Checking out branch: ${branch}`); - execSync(`git checkout -B ${branch} FETCH_HEAD`, { + // gh pr checkout handles fork PRs by setting up remotes automatically + log.debug(`Checking out PR #${prNumber} using gh pr checkout`); + execSync(`gh pr checkout ${prNumber}`, { cwd: repoDir, stdio: "pipe", }); @@ -132,9 +123,9 @@ export function setupGitBranch(payload: Payload): void { log.info(`✓ Successfully checked out PR branch: ${branch}`); return; } catch (error) { - // if PR ref fetch fails, fall back to branch name fetch + // if gh pr checkout fails, fall back to branch name fetch log.debug( - `PR ref fetch failed, falling back to branch name fetch: ${error instanceof Error ? error.message : String(error)}` + `gh pr checkout failed, falling back to branch name fetch: ${error instanceof Error ? error.message : String(error)}` ); } }