refactor: extract update_claude_comment logic to standalone testable function

- Create new updateClaudeComment function in operations/comments
- Add comprehensive unit tests following image-downloader pattern
- Update MCP server to use extracted function
- Refactor update-comment-link.ts and update-with-branch.ts to eliminate duplication
- All tests passing (10 new tests for update-claude-comment)

Co-authored-by: ashwin-ant <ashwin-ant@users.noreply.github.com>

src/entrypoints/update-comment-link.ts

@@ -12,6 +12,7 @@ import {
 } from "../github/context";
 import { GITHUB_SERVER_URL } from "../github/api/config";
 import { checkAndDeleteEmptyBranch } from "../github/operations/branch-cleanup";
+import { updateClaudeComment } from "../github/operations/comments/update-claude-comment";
 
 async function run() {
   try {
@@ -204,23 +205,15 @@ async function run() {
 
     const updatedBody = updateCommentBody(commentInput);
 
-    // Update the comment using the appropriate API
+    // Update the comment using the extracted updateClaudeComment function
     try {
-      if (isPRReviewComment) {
-        await octokit.rest.pulls.updateReviewComment({
-          owner,
-          repo,
-          comment_id: commentId,
-          body: updatedBody,
-        });
-      } else {
-        await octokit.rest.issues.updateComment({
-          owner,
-          repo,
-          comment_id: commentId,
-          body: updatedBody,
-        });
-      }
+      await updateClaudeComment(octokit, {
+        owner,
+        repo,
+        commentId,
+        body: updatedBody,
+        isPullRequestReviewComment: isPRReviewComment,
+      });
       console.log(
         `✅ Updated ${isPRReviewComment ? "PR review" : "issue"} comment ${commentId} with job link`,
       );

src/github/operations/comments/update-claude-comment.ts

@@ -0,0 +1,70 @@
+import { Octokit } from "@octokit/rest";
+
+export type UpdateClaudeCommentParams = {
+  owner: string;
+  repo: string;
+  commentId: number;
+  body: string;
+  isPullRequestReviewComment: boolean;
+};
+
+export type UpdateClaudeCommentResult = {
+  id: number;
+  html_url: string;
+  updated_at: string;
+};
+
+/**
+ * Updates a Claude comment on GitHub (either an issue/PR comment or a PR review comment)
+ * 
+ * @param octokit - Authenticated Octokit instance
+ * @param params - Parameters for updating the comment
+ * @returns The updated comment details
+ * @throws Error if the update fails
+ */
+export async function updateClaudeComment(
+  octokit: Octokit,
+  params: UpdateClaudeCommentParams,
+): Promise<UpdateClaudeCommentResult> {
+  const { owner, repo, commentId, body, isPullRequestReviewComment } = params;
+
+  let response;
+
+  try {
+    if (isPullRequestReviewComment) {
+      // Try PR review comment API first
+      response = await octokit.rest.pulls.updateReviewComment({
+        owner,
+        repo,
+        comment_id: commentId,
+        body,
+      });
+    } else {
+      // 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;
+    }
+  }
+
+  return {
+    id: response.data.id,
+    html_url: response.data.html_url,
+    updated_at: response.data.updated_at,
+  };
+}

src/github/operations/comments/update-with-branch.ts

@@ -15,6 +15,7 @@ import {
   isPullRequestReviewCommentEvent,
   type ParsedGitHubContext,
 } from "../../context";
+import { updateClaudeComment } from "./update-claude-comment";
 
 export async function updateTrackingComment(
   octokit: Octokits,
@@ -36,25 +37,19 @@ export async function updateTrackingComment(
 
   // Update the existing comment with the branch link
   try {
-    if (isPullRequestReviewCommentEvent(context)) {
-      // For PR review comments (inline comments), use the pulls API
-      await octokit.rest.pulls.updateReviewComment({
-        owner,
-        repo,
-        comment_id: commentId,
-        body: updatedBody,
-      });
-      console.log(`✅ Updated PR review comment ${commentId} with branch link`);
-    } else {
-      // For all other comments, use the issues API
-      await octokit.rest.issues.updateComment({
-        owner,
-        repo,
-        comment_id: commentId,
-        body: updatedBody,
-      });
-      console.log(`✅ Updated issue comment ${commentId} with branch link`);
-    }
+    const isPRReviewComment = isPullRequestReviewCommentEvent(context);
+    
+    await updateClaudeComment(octokit, {
+      owner,
+      repo,
+      commentId,
+      body: updatedBody,
+      isPullRequestReviewComment: isPRReviewComment,
+    });
+    
+    console.log(
+      `✅ Updated ${isPRReviewComment ? "PR review" : "issue"} comment ${commentId} with branch link`,
+    );
   } catch (error) {
     console.error("Error updating comment with branch link:", error);
     throw error;

src/mcp/github-file-ops-server.ts

@@ -8,6 +8,7 @@ import { join } from "path";
 import fetch from "node-fetch";
 import { GITHUB_API_URL } from "../github/api/config";
 import { Octokit } from "@octokit/rest";
+import { updateClaudeComment } from "../github/operations/comments/update-claude-comment";
 
 type GitHubRef = {
   object: {
@@ -473,53 +474,19 @@ server.tool(
       const isPullRequestReviewComment =
         eventName === "pull_request_review_comment";
 
-      let response;
-
-      try {
-        if (isPullRequestReviewComment) {
-          // Try PR review comment API first
-          response = await octokit.rest.pulls.updateReviewComment({
-            owner,
-            repo,
-            comment_id: commentId,
-            body,
-          });
-        } else {
-          // 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 result = await updateClaudeComment(octokit, {
+        owner,
+        repo,
+        commentId,
+        body,
+        isPullRequestReviewComment,
+      });
 
       return {
         content: [
           {
             type: "text",
-            text: JSON.stringify(
-              {
-                id: response.data.id,
-                html_url: response.data.html_url,
-                updated_at: response.data.updated_at,
-              },
-              null,
-              2,
-            ),
+            text: JSON.stringify(result, null, 2),
           },
         ],
       };

test/update-claude-comment.test.ts

@@ -0,0 +1,388 @@
+import { describe, test, expect, jest, beforeEach } from "bun:test";
+import { Octokit } from "@octokit/rest";
+import {
+  updateClaudeComment,
+  type UpdateClaudeCommentParams,
+  type UpdateClaudeCommentResult,
+} from "../src/github/operations/comments/update-claude-comment";
+
+describe("updateClaudeComment", () => {
+  let mockOctokit: Octokit;
+
+  beforeEach(() => {
+    mockOctokit = {
+      rest: {
+        issues: {
+          updateComment: jest.fn(),
+        },
+        pulls: {
+          updateReviewComment: jest.fn(),
+        },
+      },
+    } as any as Octokit;
+  });
+
+  test("should update issue comment successfully", async () => {
+    const mockResponse = {
+      data: {
+        id: 123456,
+        html_url: "https://github.com/owner/repo/issues/1#issuecomment-123456",
+        updated_at: "2024-01-01T00:00:00Z",
+        body: "Updated comment",
+      },
+    };
+
+    // @ts-expect-error Mock implementation doesn't match full type signature
+    mockOctokit.rest.issues.updateComment = jest.fn().mockResolvedValue(mockResponse);
+
+    const params: UpdateClaudeCommentParams = {
+      owner: "testowner",
+      repo: "testrepo",
+      commentId: 123456,
+      body: "Updated comment",
+      isPullRequestReviewComment: false,
+    };
+
+    const result = await updateClaudeComment(mockOctokit, params);
+
+    expect(mockOctokit.rest.issues.updateComment).toHaveBeenCalledWith({
+      owner: "testowner",
+      repo: "testrepo",
+      comment_id: 123456,
+      body: "Updated comment",
+    });
+
+    expect(result).toEqual({
+      id: 123456,
+      html_url: "https://github.com/owner/repo/issues/1#issuecomment-123456",
+      updated_at: "2024-01-01T00:00:00Z",
+    });
+  });
+
+  test("should update PR comment successfully", async () => {
+    const mockResponse = {
+      data: {
+        id: 789012,
+        html_url: "https://github.com/owner/repo/pull/2#issuecomment-789012",
+        updated_at: "2024-01-02T00:00:00Z",
+        body: "Updated PR comment",
+      },
+    };
+
+    // @ts-expect-error Mock implementation doesn't match full type signature
+    mockOctokit.rest.issues.updateComment = jest.fn().mockResolvedValue(mockResponse);
+
+    const params: UpdateClaudeCommentParams = {
+      owner: "testowner",
+      repo: "testrepo",
+      commentId: 789012,
+      body: "Updated PR comment",
+      isPullRequestReviewComment: false,
+    };
+
+    const result = await updateClaudeComment(mockOctokit, params);
+
+    expect(mockOctokit.rest.issues.updateComment).toHaveBeenCalledWith({
+      owner: "testowner",
+      repo: "testrepo",
+      comment_id: 789012,
+      body: "Updated PR comment",
+    });
+
+    expect(result).toEqual({
+      id: 789012,
+      html_url: "https://github.com/owner/repo/pull/2#issuecomment-789012",
+      updated_at: "2024-01-02T00:00:00Z",
+    });
+  });
+
+  test("should update PR review comment successfully", async () => {
+    const mockResponse = {
+      data: {
+        id: 345678,
+        html_url: "https://github.com/owner/repo/pull/3#discussion_r345678",
+        updated_at: "2024-01-03T00:00:00Z",
+        body: "Updated review comment",
+      },
+    };
+
+    // @ts-expect-error Mock implementation doesn't match full type signature
+    mockOctokit.rest.pulls.updateReviewComment = jest.fn().mockResolvedValue(mockResponse);
+
+    const params: UpdateClaudeCommentParams = {
+      owner: "testowner",
+      repo: "testrepo",
+      commentId: 345678,
+      body: "Updated review comment",
+      isPullRequestReviewComment: true,
+    };
+
+    const result = await updateClaudeComment(mockOctokit, params);
+
+    expect(mockOctokit.rest.pulls.updateReviewComment).toHaveBeenCalledWith({
+      owner: "testowner",
+      repo: "testrepo",
+      comment_id: 345678,
+      body: "Updated review comment",
+    });
+
+    expect(result).toEqual({
+      id: 345678,
+      html_url: "https://github.com/owner/repo/pull/3#discussion_r345678",
+      updated_at: "2024-01-03T00:00:00Z",
+    });
+  });
+
+  test("should fallback to issue comment API when PR review comment update fails with 404", async () => {
+    const mockError = new Error("Not Found") as any;
+    mockError.status = 404;
+
+    const mockResponse = {
+      data: {
+        id: 456789,
+        html_url: "https://github.com/owner/repo/pull/4#issuecomment-456789",
+        updated_at: "2024-01-04T00:00:00Z",
+        body: "Updated via fallback",
+      },
+    };
+
+    // @ts-expect-error Mock implementation doesn't match full type signature
+    mockOctokit.rest.pulls.updateReviewComment = jest.fn().mockRejectedValue(mockError);
+    // @ts-expect-error Mock implementation doesn't match full type signature
+    mockOctokit.rest.issues.updateComment = jest.fn().mockResolvedValue(mockResponse);
+
+    const params: UpdateClaudeCommentParams = {
+      owner: "testowner",
+      repo: "testrepo",
+      commentId: 456789,
+      body: "Updated via fallback",
+      isPullRequestReviewComment: true,
+    };
+
+    const result = await updateClaudeComment(mockOctokit, params);
+
+    expect(mockOctokit.rest.pulls.updateReviewComment).toHaveBeenCalledWith({
+      owner: "testowner",
+      repo: "testrepo",
+      comment_id: 456789,
+      body: "Updated via fallback",
+    });
+
+    expect(mockOctokit.rest.issues.updateComment).toHaveBeenCalledWith({
+      owner: "testowner",
+      repo: "testrepo",
+      comment_id: 456789,
+      body: "Updated via fallback",
+    });
+
+    expect(result).toEqual({
+      id: 456789,
+      html_url: "https://github.com/owner/repo/pull/4#issuecomment-456789",
+      updated_at: "2024-01-04T00:00:00Z",
+    });
+  });
+
+  test("should propagate error when PR review comment update fails with non-404 error", async () => {
+    const mockError = new Error("Internal Server Error") as any;
+    mockError.status = 500;
+
+    // @ts-expect-error Mock implementation doesn't match full type signature
+    mockOctokit.rest.pulls.updateReviewComment = jest.fn().mockRejectedValue(mockError);
+
+    const params: UpdateClaudeCommentParams = {
+      owner: "testowner",
+      repo: "testrepo",
+      commentId: 567890,
+      body: "This will fail",
+      isPullRequestReviewComment: true,
+    };
+
+    await expect(updateClaudeComment(mockOctokit, params)).rejects.toEqual(mockError);
+
+    expect(mockOctokit.rest.pulls.updateReviewComment).toHaveBeenCalledWith({
+      owner: "testowner",
+      repo: "testrepo",
+      comment_id: 567890,
+      body: "This will fail",
+    });
+
+    // Ensure fallback wasn't attempted
+    expect(mockOctokit.rest.issues.updateComment).not.toHaveBeenCalled();
+  });
+
+  test("should propagate error when issue comment update fails", async () => {
+    const mockError = new Error("Forbidden");
+
+    // @ts-expect-error Mock implementation doesn't match full type signature
+    mockOctokit.rest.issues.updateComment = jest.fn().mockRejectedValue(mockError);
+
+    const params: UpdateClaudeCommentParams = {
+      owner: "testowner",
+      repo: "testrepo",
+      commentId: 678901,
+      body: "This will also fail",
+      isPullRequestReviewComment: false,
+    };
+
+    await expect(updateClaudeComment(mockOctokit, params)).rejects.toEqual(mockError);
+
+    expect(mockOctokit.rest.issues.updateComment).toHaveBeenCalledWith({
+      owner: "testowner",
+      repo: "testrepo",
+      comment_id: 678901,
+      body: "This will also fail",
+    });
+  });
+
+  test("should handle empty body", async () => {
+    const mockResponse = {
+      data: {
+        id: 111222,
+        html_url: "https://github.com/owner/repo/issues/5#issuecomment-111222",
+        updated_at: "2024-01-05T00:00:00Z",
+        body: "",
+      },
+    };
+
+    // @ts-expect-error Mock implementation doesn't match full type signature
+    mockOctokit.rest.issues.updateComment = jest.fn().mockResolvedValue(mockResponse);
+
+    const params: UpdateClaudeCommentParams = {
+      owner: "testowner",
+      repo: "testrepo",
+      commentId: 111222,
+      body: "",
+      isPullRequestReviewComment: false,
+    };
+
+    const result = await updateClaudeComment(mockOctokit, params);
+
+    expect(result).toEqual({
+      id: 111222,
+      html_url: "https://github.com/owner/repo/issues/5#issuecomment-111222",
+      updated_at: "2024-01-05T00:00:00Z",
+    });
+  });
+
+  test("should handle very long body", async () => {
+    const longBody = "x".repeat(10000);
+    const mockResponse = {
+      data: {
+        id: 333444,
+        html_url: "https://github.com/owner/repo/issues/6#issuecomment-333444",
+        updated_at: "2024-01-06T00:00:00Z",
+        body: longBody,
+      },
+    };
+
+    // @ts-expect-error Mock implementation doesn't match full type signature
+    mockOctokit.rest.issues.updateComment = jest.fn().mockResolvedValue(mockResponse);
+
+    const params: UpdateClaudeCommentParams = {
+      owner: "testowner",
+      repo: "testrepo",
+      commentId: 333444,
+      body: longBody,
+      isPullRequestReviewComment: false,
+    };
+
+    const result = await updateClaudeComment(mockOctokit, params);
+
+    expect(mockOctokit.rest.issues.updateComment).toHaveBeenCalledWith({
+      owner: "testowner",
+      repo: "testrepo",
+      comment_id: 333444,
+      body: longBody,
+    });
+
+    expect(result).toEqual({
+      id: 333444,
+      html_url: "https://github.com/owner/repo/issues/6#issuecomment-333444",
+      updated_at: "2024-01-06T00:00:00Z",
+    });
+  });
+
+  test("should handle markdown formatting in body", async () => {
+    const markdownBody = `
+# Header
+- List item 1
+- List item 2
+
+\`\`\`typescript
+const code = "example";
+\`\`\`
+
+[Link](https://example.com)
+    `.trim();
+
+    const mockResponse = {
+      data: {
+        id: 555666,
+        html_url: "https://github.com/owner/repo/issues/7#issuecomment-555666",
+        updated_at: "2024-01-07T00:00:00Z",
+        body: markdownBody,
+      },
+    };
+
+    // @ts-expect-error Mock implementation doesn't match full type signature
+    mockOctokit.rest.issues.updateComment = jest.fn().mockResolvedValue(mockResponse);
+
+    const params: UpdateClaudeCommentParams = {
+      owner: "testowner",
+      repo: "testrepo",
+      commentId: 555666,
+      body: markdownBody,
+      isPullRequestReviewComment: false,
+    };
+
+    const result = await updateClaudeComment(mockOctokit, params);
+
+    expect(mockOctokit.rest.issues.updateComment).toHaveBeenCalledWith({
+      owner: "testowner",
+      repo: "testrepo",
+      comment_id: 555666,
+      body: markdownBody,
+    });
+
+    expect(result).toEqual({
+      id: 555666,
+      html_url: "https://github.com/owner/repo/issues/7#issuecomment-555666",
+      updated_at: "2024-01-07T00:00:00Z",
+    });
+  });
+
+  test("should handle different response data fields", async () => {
+    const mockResponse = {
+      data: {
+        id: 777888,
+        html_url: "https://github.com/owner/repo/pull/8#discussion_r777888",
+        updated_at: "2024-01-08T12:30:45Z",
+        body: "Updated",
+        // Additional fields that might be in the response
+        created_at: "2024-01-01T00:00:00Z",
+        user: { login: "bot" },
+        node_id: "MDI0OlB1bGxSZXF1ZXN0UmV2aWV3Q29tbWVudDc3Nzg4OA==",
+      },
+    };
+
+    // @ts-expect-error Mock implementation doesn't match full type signature
+    mockOctokit.rest.pulls.updateReviewComment = jest.fn().mockResolvedValue(mockResponse);
+
+    const params: UpdateClaudeCommentParams = {
+      owner: "testowner",
+      repo: "testrepo",
+      commentId: 777888,
+      body: "Updated",
+      isPullRequestReviewComment: true,
+    };
+
+    const result = await updateClaudeComment(mockOctokit, params);
+
+    // Should only return the specific fields we care about
+    expect(result).toEqual({
+      id: 777888,
+      html_url: "https://github.com/owner/repo/pull/8#discussion_r777888",
+      updated_at: "2024-01-08T12:30:45Z",
+    });
+  });
+});