Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
247 changes: 247 additions & 0 deletions base-action/test/inline-comment-utils.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,247 @@
#!/usr/bin/env bun

import { describe, test, expect } from "bun:test";
import {
findExistingRootComment,
resolveInReplyTo,
type ReviewComment,
} from "../../src/mcp/inline-comment-utils";

describe("inline-comment-utils", () => {
describe("findExistingRootComment", () => {
const mockComments: ReviewComment[] = [
{
id: 100,
path: "src/index.ts",
line: 42,
original_line: 42,
in_reply_to_id: null,
},
{
id: 101,
path: "src/index.ts",
line: 42,
original_line: 42,
in_reply_to_id: 100, // This is a reply, not a root comment
},
{
id: 102,
path: "src/utils.ts",
line: 10,
original_line: 10,
in_reply_to_id: null,
},
{
id: 103,
path: "src/index.ts",
line: 50,
original_line: 55, // original_line differs from current line
in_reply_to_id: null,
},
];

test("should find existing root comment on same path and line", () => {
const result = findExistingRootComment(mockComments, "src/index.ts", 42);
expect(result).toBeDefined();
expect(result?.id).toBe(100);
});

test("should not find reply comments (only root comments)", () => {
// Comment 101 is on the same line but it's a reply
const replyComment = mockComments[1]!;
const comments: ReviewComment[] = [replyComment];
const result = findExistingRootComment(comments, "src/index.ts", 42);
expect(result).toBeUndefined();
});

test("should find comment by original_line when line doesn't match", () => {
const result = findExistingRootComment(mockComments, "src/index.ts", 55);
expect(result).toBeDefined();
expect(result?.id).toBe(103);
});

test("should return undefined when no matching comment exists", () => {
const result = findExistingRootComment(mockComments, "src/index.ts", 999);
expect(result).toBeUndefined();
});

test("should return undefined when path doesn't match", () => {
const result = findExistingRootComment(
mockComments,
"src/nonexistent.ts",
42,
);
expect(result).toBeUndefined();
});

test("should return undefined when targetLine is undefined", () => {
const result = findExistingRootComment(
mockComments,
"src/index.ts",
undefined,
);
expect(result).toBeUndefined();
});

test("should return undefined when comments array is empty", () => {
const result = findExistingRootComment([], "src/index.ts", 42);
expect(result).toBeUndefined();
});
});

describe("resolveInReplyTo", () => {
const mockComments: ReviewComment[] = [
{
id: 100,
path: "src/index.ts",
line: 42,
original_line: 42,
in_reply_to_id: null,
},
{
id: 102,
path: "src/utils.ts",
line: 10,
original_line: 10,
in_reply_to_id: null,
},
];

test("should return explicit in_reply_to when provided", () => {
const result = resolveInReplyTo(999, mockComments, "src/index.ts", 42);
expect(result).toBe(999);
});

test("should auto-detect existing comment when in_reply_to is undefined", () => {
const result = resolveInReplyTo(
undefined,
mockComments,
"src/index.ts",
42,
);
expect(result).toBe(100);
});

test("should return undefined when no explicit in_reply_to and no existing comment", () => {
const result = resolveInReplyTo(
undefined,
mockComments,
"src/index.ts",
999,
);
expect(result).toBeUndefined();
});

test("should prioritize explicit in_reply_to over auto-detection", () => {
// Even though there's an existing comment on line 42, explicit value takes precedence
const result = resolveInReplyTo(555, mockComments, "src/index.ts", 42);
expect(result).toBe(555);
});

test("should handle empty comments array with undefined in_reply_to", () => {
const result = resolveInReplyTo(undefined, [], "src/index.ts", 42);
expect(result).toBeUndefined();
});
});

describe("auto-deduplication scenarios", () => {
test("Scenario: First review creates new comment (no existing comments)", () => {
const existingComments: ReviewComment[] = [];
const result = resolveInReplyTo(
undefined,
existingComments,
"src/index.ts",
42,
);
// No existing comment, should create new thread
expect(result).toBeUndefined();
});

test("Scenario: Second review should reply to first comment (deduplication)", () => {
const existingComments: ReviewComment[] = [
{
id: 100,
path: "src/index.ts",
line: 42,
original_line: 42,
in_reply_to_id: null,
},
];
const result = resolveInReplyTo(
undefined,
existingComments,
"src/index.ts",
42,
);
// Should reply to existing comment instead of creating duplicate
expect(result).toBe(100);
});

test("Scenario: Third review should still reply to root comment (not to second reply)", () => {
const existingComments: ReviewComment[] = [
{
id: 100,
path: "src/index.ts",
line: 42,
original_line: 42,
in_reply_to_id: null, // Root comment
},
{
id: 101,
path: "src/index.ts",
line: 42,
original_line: 42,
in_reply_to_id: 100, // Reply to root
},
];
const result = resolveInReplyTo(
undefined,
existingComments,
"src/index.ts",
42,
);
// Should reply to root comment (100), not to the reply (101)
expect(result).toBe(100);
});

test("Scenario: Different file should not trigger deduplication", () => {
const existingComments: ReviewComment[] = [
{
id: 100,
path: "src/index.ts",
line: 42,
original_line: 42,
in_reply_to_id: null,
},
];
const result = resolveInReplyTo(
undefined,
existingComments,
"src/other.ts", // Different file
42,
);
// Different file, should create new thread
expect(result).toBeUndefined();
});

test("Scenario: Different line should not trigger deduplication", () => {
const existingComments: ReviewComment[] = [
{
id: 100,
path: "src/index.ts",
line: 42,
original_line: 42,
in_reply_to_id: null,
},
];
const result = resolveInReplyTo(
undefined,
existingComments,
"src/index.ts",
50, // Different line
);
// Different line, should create new thread
expect(result).toBeUndefined();
});
});
});
18 changes: 16 additions & 2 deletions src/create-prompt/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -555,7 +555,13 @@ ${sanitizeContent(eventData.commentBody)}
Your request is in <trigger_comment> above${eventData.eventName === "issues" ? ` (or the ${entityType} body for assigned/labeled events)` : ""}.

Decide what's being asked:
1. **Question or code review** - Answer directly or provide feedback
1. **Question or code review** - Answer directly or provide feedback${
eventData.isPR
? `
- For inline comments: Use mcp__github_inline_comment__create_inline_comment to add specific feedback on lines
- Duplicate comments are automatically prevented - if a comment exists on a line, yours will be added as a reply`
: ""
}
2. **Code change** - Implement the change, commit, and push

Communication:
Expand Down Expand Up @@ -735,7 +741,15 @@ ${eventData.eventName === "issue_comment" || eventData.eventName === "pull_reque
- Look for bugs, security issues, performance problems, and other issues
- Suggest improvements for readability and maintainability
- Check for best practices and coding standards
- Reference specific code sections with file paths and line numbers${eventData.isPR ? `\n - AFTER reading files and analyzing code, you MUST call mcp__github_comment__update_claude_comment to post your review` : ""}
- Reference specific code sections with file paths and line numbers${
eventData.isPR
? `\n - AFTER reading files and analyzing code, you MUST call mcp__github_comment__update_claude_comment to post your review
- INLINE COMMENTS (PR reviews):
- You have access to mcp__github_inline_comment__create_inline_comment to add inline comments on specific lines
- The tool automatically prevents duplicate comments - if a comment already exists on a line, your comment will be added as a reply to the existing thread
- If you see existing comments in the <review_comments> section with comment IDs, you can reference them in your feedback`
: ""
}
- Formulate a concise, technical, and helpful response based on the context.
- Reference specific code with inline formatting or code blocks.
- Include relevant file paths and line numbers when applicable.${
Expand Down
35 changes: 34 additions & 1 deletion src/mcp/github-inline-comment-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { StdioServerTransport } from "@modelcontextprotocol/sdk/server/stdio.js"
import { z } from "zod";
import { createOctokit } from "../github/api/client";
import { sanitizeContent } from "../github/utils/sanitizer";
import { resolveInReplyTo } from "./inline-comment-utils";

// Get repository and PR information from environment variables
const REPO_OWNER = process.env.REPO_OWNER;
Expand Down Expand Up @@ -67,8 +68,14 @@ server.tool(
.describe(
"Specific commit SHA to comment on (defaults to latest commit)",
),
in_reply_to: z
.number()
.optional()
.describe(
"Comment ID to reply to (creates a reply in an existing thread instead of a new thread)",
),
},
async ({ path, body, line, startLine, side, commit_id }) => {
async ({ path, body, line, startLine, side, commit_id, in_reply_to }) => {
try {
const githubToken = process.env.GITHUB_TOKEN;

Expand Down Expand Up @@ -124,6 +131,32 @@ server.tool(
params.line = line;
}

// Resolve in_reply_to: use explicit value or auto-detect from existing comments
const { data: existingComments } =
await octokit.rest.pulls.listReviewComments({
owner,
repo,
pull_number,
per_page: 100,
});

const resolvedInReplyTo = resolveInReplyTo(
in_reply_to,
existingComments,
path,
line,
);

if (resolvedInReplyTo !== undefined) {
params.in_reply_to = resolvedInReplyTo;
if (in_reply_to === undefined) {
// Auto-deduplication case
console.log(
`Auto-deduplication: Found existing comment ${resolvedInReplyTo} on ${path}:${line}. Replying to existing thread instead of creating duplicate.`,
);
}
}

const result = await octokit.rest.pulls.createReviewComment(params);

return {
Expand Down
Loading