From b9f09384050c9d6bd8d6cfeb0eb2633aa4123931 Mon Sep 17 00:00:00 2001 From: Colin McDonnell Date: Wed, 13 May 2026 22:49:06 +0000 Subject: [PATCH] mcp: restore operational guidance dropped in #723 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #723's revision pass cut four substantive strings along with the negative anchors. those strings address real, audit-observed failure modes and the positive examples don't carry them. restored: - push_branch: "if the response reports a timeout, the underlying push may have actually succeeded — verify with git log origin/ before retrying" (was on the tool description) - create_pull_request_review commit_id .describe(): "must be the FULL 40-character SHA — abbreviated SHAs are rejected by GitHub with 422" - create_pull_request_review comments[].line .describe(): "must sit inside a `@@` hunk... dropped entries are reported under droppedComments in the response" - create_pull_request_review comments[].start_line .describe(): "both start_line and line must sit inside the same @@ hunk" also: get_commit_info example used a 31-character SHA (non-standard truncation). swapped to a 7-char short form, which is what git log --oneline emits and what agents see in practice. note that this tool accepts either full or abbreviated, unlike create_pull_request_review which requires full. --- mcp/commitInfo.ts | 2 +- mcp/git.ts | 3 ++- mcp/review.ts | 8 +++++--- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/mcp/commitInfo.ts b/mcp/commitInfo.ts index 44c86b6..a39b678 100644 --- a/mcp/commitInfo.ts +++ b/mcp/commitInfo.ts @@ -16,7 +16,7 @@ export function CommitInfoTool(ctx: ToolContext) { description: "Retrieve commit metadata and diff via GitHub API. Use this instead of git show for reviewing commits - " + "it works with shallow clones and shows the actual changes in the commit. Returns diffPath pointing to formatted diff file. " + - 'Example: `get_commit_info({ sha: "2a6ab5d06ba64dd987bd88c4403287" })`.', + 'Example: `get_commit_info({ sha: "2a6ab5d" })`.', parameters: CommitInfo, execute: execute(async ({ sha }) => { const response = await ctx.octokit.rest.repos.getCommit({ diff --git a/mcp/git.ts b/mcp/git.ts index d882787..da5cdd4 100644 --- a/mcp/git.ts +++ b/mcp/git.ts @@ -222,7 +222,8 @@ export function PushBranchTool(ctx: ToolContext) { "If specifying branchName, use the LOCAL branch name (e.g., 'pr-1'), not the remote branch name. " + "The correct remote and remote branch are determined automatically from branch config set by checkout_pr. " + "Requires a clean working tree. Runs the repository prepush hook (if configured) before the network push — hook failure means tests/lint or similar in that script failed, not necessarily a Pullfrog timeout. " + - "Never force push unless explicitly requested. Pushes to the default branch are blocked in restricted mode.", + "Never force push unless explicitly requested. Pushes to the default branch are blocked in restricted mode. " + + "If the response reports a timeout, the underlying push may have actually succeeded — verify with `git log origin/` (or this tool with command 'log') before retrying, otherwise you'll push a duplicate.", parameters: PushBranch, execute: execute(async ({ branchName, force }) => { // permission check diff --git a/mcp/review.ts b/mcp/review.ts index d03c01c..fb6ce42 100644 --- a/mcp/review.ts +++ b/mcp/review.ts @@ -320,14 +320,16 @@ export const CreatePullRequestReview = type({ ) .optional(), commit_id: type.string - .describe("Optional SHA of the commit being reviewed. Defaults to latest.") + .describe( + "Optional SHA of the commit being reviewed. Defaults to latest. Must be the FULL 40-character SHA — abbreviated SHAs are rejected by GitHub with `422 Unprocessable Entity`. The PR-synchronize event payload's `head_sha` is already full-length." + ) .optional(), comments: type({ path: type.string.describe( "The file path to comment on (relative to repo root). Must be a file that appears in the PR diff." ), line: type.number.describe( - "Line number to comment on. For multi-line ranges, this is the end line. Use NEW column from diff format." + "Line number to comment on. For multi-line ranges, this is the end line. Use NEW column from diff format. Must sit inside a `@@` hunk in the PR diff — anchors on context-only or untouched lines are dropped silently (the rest of the review still posts; dropped entries are reported under `droppedComments` in the response)." ), side: type .enumerated("LEFT", "RIGHT") @@ -345,7 +347,7 @@ export const CreatePullRequestReview = type({ .optional(), start_line: type.number .describe( - "Start line for multi-line comment ranges. Omit for single-line comments. The range [start_line, line] defines which lines a suggestion replaces." + "Start line for multi-line comment ranges. Omit for single-line comments. The range [start_line, line] defines which lines a suggestion replaces. Both `start_line` and `line` must sit inside the same `@@` hunk — a `start_line` outside the hunk causes the whole comment to be dropped even when `line` is valid. If you need to comment on context just above/below a hunk, shrink the range to a single line that is provably modified." ) .optional(), })