From b61c881f0ef0bcdfa728d034d9bfbc7e667bf76b Mon Sep 17 00:00:00 2001 From: Ashwin Bhat Date: Fri, 30 May 2025 07:56:32 -0700 Subject: [PATCH] feat: add unified update_claude_comment tool - Add new update_claude_comment tool that automatically handles both issue and PR comments - Remove individual update_issue_comment and update_pull_request_comment tools - Pass CLAUDE_COMMENT_ID, GITHUB_EVENT_NAME, and IS_PR to MCP server environment - Use Octokit instead of raw fetch for better type safety and error handling - Simplify Claude's comment update workflow by removing need for owner/repo/commentId params - Update prompts and tests to use the new unified tool --- src/create-prompt/index.ts | 6 +-- src/mcp/github-file-ops-server.ts | 87 +++++++++++++------------------ test/create-prompt.test.ts | 31 ++--------- 3 files changed, 40 insertions(+), 84 deletions(-) diff --git a/src/create-prompt/index.ts b/src/create-prompt/index.ts index 79ad7e3..e292f34 100644 --- a/src/create-prompt/index.ts +++ b/src/create-prompt/index.ts @@ -35,10 +35,7 @@ const BASE_ALLOWED_TOOLS = [ ]; const DISALLOWED_TOOLS = ["WebSearch", "WebFetch"]; -export function buildAllowedToolsString( - eventData: EventData, - customAllowedTools?: string, -): string { +export function buildAllowedToolsString(customAllowedTools?: string): string { let baseTools = [...BASE_ALLOWED_TOOLS]; let allAllowedTools = baseTools.join(","); @@ -639,7 +636,6 @@ export async function createPrompt( // Set allowed tools const allAllowedTools = buildAllowedToolsString( - preparedContext.eventData, preparedContext.allowedTools, ); const allDisallowedTools = buildDisallowedToolsString( diff --git a/src/mcp/github-file-ops-server.ts b/src/mcp/github-file-ops-server.ts index 1a3e4ba..ad2f038 100644 --- a/src/mcp/github-file-ops-server.ts +++ b/src/mcp/github-file-ops-server.ts @@ -7,6 +7,7 @@ import { readFile } from "fs/promises"; import { join } from "path"; import fetch from "node-fetch"; import { GITHUB_API_URL } from "../github/api/config"; +import { Octokit } from "@octokit/rest"; type GitHubRef = { object: { @@ -451,7 +452,6 @@ server.tool( const githubToken = process.env.GITHUB_TOKEN; const claudeCommentId = process.env.CLAUDE_COMMENT_ID; const eventName = process.env.GITHUB_EVENT_NAME; - const isPR = process.env.IS_PR === "true"; if (!githubToken) { throw new Error("GITHUB_TOKEN environment variable is required"); @@ -464,73 +464,58 @@ server.tool( const repo = REPO_NAME; const commentId = parseInt(claudeCommentId, 10); + // Create Octokit instance + const octokit = new Octokit({ + auth: githubToken, + }); + // Determine if this is a PR review comment based on event type const isPullRequestReviewComment = eventName === "pull_request_review_comment"; let response; - let updateUrl: string; - if (isPullRequestReviewComment) { - // Use the PR review comment API - updateUrl = `${GITHUB_API_URL}/repos/${owner}/${repo}/pulls/comments/${commentId}`; - } else { - // Use the issue comment API (works for both issues and PR general comments) - updateUrl = `${GITHUB_API_URL}/repos/${owner}/${repo}/issues/comments/${commentId}`; - } - - response = await fetch(updateUrl, { - method: "PATCH", - headers: { - Accept: "application/vnd.github+json", - Authorization: `Bearer ${githubToken}`, - "X-GitHub-Api-Version": "2022-11-28", - "Content-Type": "application/json", - }, - body: JSON.stringify({ body }), - }); - - if (!response.ok) { - const errorText = await response.text(); - - // If PR review comment update fails, fall back to issue comment API - if (isPullRequestReviewComment && response.status === 404) { - const fallbackUrl = `${GITHUB_API_URL}/repos/${owner}/${repo}/issues/comments/${commentId}`; - response = await fetch(fallbackUrl, { - method: "PATCH", - headers: { - Accept: "application/vnd.github+json", - Authorization: `Bearer ${githubToken}`, - "X-GitHub-Api-Version": "2022-11-28", - "Content-Type": "application/json", - }, - body: JSON.stringify({ body }), + try { + if (isPullRequestReviewComment) { + // Try PR review comment API first + response = await octokit.rest.pulls.updateReviewComment({ + owner, + repo, + comment_id: commentId, + body, }); - - if (!response.ok) { - const fallbackErrorText = await response.text(); - throw new Error( - `Failed to update comment: ${response.status} - ${fallbackErrorText}`, - ); - } } else { - throw new Error( - `Failed to update comment: ${response.status} - ${errorText}`, - ); + // Use issue comment API (works for both issues and PR general comments) + response = await octokit.rest.issues.updateComment({ + owner, + repo, + comment_id: commentId, + body, + }); + } + } catch (error: any) { + // If PR review comment update fails with 404, fall back to issue comment API + if (isPullRequestReviewComment && error.status === 404) { + response = await octokit.rest.issues.updateComment({ + owner, + repo, + comment_id: commentId, + body, + }); + } else { + throw error; } } - const updatedComment = await response.json(); - return { content: [ { type: "text", text: JSON.stringify( { - id: updatedComment.id, - html_url: updatedComment.html_url, - updated_at: updatedComment.updated_at, + id: response.data.id, + html_url: response.data.html_url, + updated_at: response.data.updated_at, }, null, 2, diff --git a/test/create-prompt.test.ts b/test/create-prompt.test.ts index fa98ebd..617f0ac 100644 --- a/test/create-prompt.test.ts +++ b/test/create-prompt.test.ts @@ -8,7 +8,6 @@ import { buildDisallowedToolsString, } from "../src/create-prompt"; import type { PreparedContext } from "../src/create-prompt"; -import type { EventData } from "../src/create-prompt/types"; describe("generatePrompt", () => { const mockGitHubData = { @@ -619,15 +618,7 @@ describe("getEventTypeAndContext", () => { describe("buildAllowedToolsString", () => { test("should return issue comment tool for regular events", () => { - const mockEventData: EventData = { - eventName: "issue_comment", - commentId: "123", - isPR: true, - prNumber: "456", - commentBody: "Test comment", - }; - - const result = buildAllowedToolsString(mockEventData); + const result = buildAllowedToolsString(); // The base tools should be in the result expect(result).toContain("Edit"); @@ -644,15 +635,7 @@ describe("buildAllowedToolsString", () => { }); test("should return PR comment tool for inline review comments", () => { - const mockEventData: EventData = { - eventName: "pull_request_review_comment", - isPR: true, - prNumber: "456", - commentBody: "Test review comment", - commentId: "789", - }; - - const result = buildAllowedToolsString(mockEventData); + const result = buildAllowedToolsString(); // The base tools should be in the result expect(result).toContain("Edit"); @@ -669,16 +652,8 @@ describe("buildAllowedToolsString", () => { }); test("should append custom tools when provided", () => { - const mockEventData: EventData = { - eventName: "issue_comment", - commentId: "123", - isPR: true, - prNumber: "456", - commentBody: "Test comment", - }; - const customTools = "Tool1,Tool2,Tool3"; - const result = buildAllowedToolsString(mockEventData, customTools); + const result = buildAllowedToolsString(customTools); // Base tools should be present expect(result).toContain("Edit");