diff --git a/mcp/comment.ts b/mcp/comment.ts index 14a6abf..12c118a 100644 --- a/mcp/comment.ts +++ b/mcp/comment.ts @@ -398,15 +398,75 @@ export const ReplyToReviewComment = type({ ), }); +/** + * decision returned by `duplicateReplyDecision` when a session has already + * posted an identical reply to the same parent review comment. + */ +export interface DuplicateReplyDecision { + kind: "already-replied"; + commentId: number; + url: string | undefined; + reason: string; +} + +/** + * decide whether a second reply_to_review_comment call in the same session + * is a duplicate of an earlier reply to the same parent comment. + * + * the agent is instructed to call reply_to_review_comment exactly once per + * parent comment per AddressReviews session, but in practice it sometimes + * emits the same call twice. PR #610 reproduced this with Kimi K2: + * identical body posted 3 seconds apart, only one tool_use event in the + * agent log. the second post is always redundant and clutters the PR thread. + * + * we key on (comment_id, bodyWithFooter) so a legitimate follow-up reply + * with different content still goes through. within a single run the + * footer is constant (workflow run + model + jobId), so byte-equal bodies + * catch the stutter without blocking real follow-ups. + * + * mirrors the shape of `duplicateReviewDecision` in mcp/review.ts. + */ +export function duplicateReplyDecision(params: { + existing: { commentId: number; url: string | undefined; bodyWithFooter: string } | undefined; + bodyWithFooter: string; +}): DuplicateReplyDecision | null { + const existing = params.existing; + if (!existing) return null; + if (existing.bodyWithFooter !== params.bodyWithFooter) return null; + return { + kind: "already-replied", + commentId: existing.commentId, + url: existing.url, + reason: `reply ${existing.commentId} with identical body was already posted in this session; ignoring duplicate call`, + }; +} + export function ReplyToReviewCommentTool(ctx: ToolContext) { return tool({ name: "reply_to_review_comment", description: - "Reply to a PR review comment thread (NOT issue comments — this only works for inline review comments on PR diffs). Call this for EACH comment you address in AddressReviews mode. Keep replies extremely brief (1 sentence max).", + "Reply to a PR review comment thread (NOT issue comments — this only works for inline review comments on PR diffs). Call exactly ONCE per parent comment you address in AddressReviews mode — duplicate calls with the same body are a no-op. Keep replies extremely brief (1 sentence max).", parameters: ReplyToReviewComment, execute: execute(async ({ pull_number, comment_id, body }) => { const bodyWithFooter = addFooter(ctx, body); + // guard against duplicate reply submissions in the same session. + // see duplicateReplyDecision for the rationale. + const dup = duplicateReplyDecision({ + existing: ctx.toolState.reviewReplies?.get(comment_id), + bodyWithFooter, + }); + if (dup) { + log.info(`skipping duplicate review reply: ${dup.reason}`); + return { + success: true, + skipped: true, + reason: dup.reason, + commentId: dup.commentId, + url: dup.url, + }; + } + const result = await ctx.octokit.rest.pulls.createReplyForReviewComment({ owner: ctx.repo.owner, repo: ctx.repo.name, @@ -420,6 +480,14 @@ export function ReplyToReviewCommentTool(ctx: ToolContext) { // a substantive write happened (used by reportErrorToComment / handleAgentResult) ctx.toolState.wasUpdated = true; + // record this reply for in-session dedupe of subsequent identical calls. + ctx.toolState.reviewReplies ??= new Map(); + ctx.toolState.reviewReplies.set(comment_id, { + commentId: result.data.id, + url: result.data.html_url, + bodyWithFooter, + }); + return { success: true, commentId: result.data.id, diff --git a/mcp/server.ts b/mcp/server.ts index 32dce5f..bc75cf0 100644 --- a/mcp/server.ts +++ b/mcp/server.ts @@ -131,6 +131,15 @@ export interface ToolState { nodeId: string; reviewedSha: string | undefined; }; + // dedupe key: parent review comment_id → most-recent reply written this + // session by reply_to_review_comment. used by duplicateReplyDecision to + // skip identical-body re-emissions of the same call (PR #610 root cause). + // body-keyed (not just id-keyed) so legitimate follow-up replies with + // different content still go through. + reviewReplies?: Map< + number, + { commentId: number; url: string | undefined; bodyWithFooter: string } + >; dependencyInstallation?: { status: "not_started" | "in_progress" | "completed" | "failed"; promise: Promise | undefined; diff --git a/modes.ts b/modes.ts index e74d860..e0f7775 100644 --- a/modes.ts +++ b/modes.ts @@ -147,7 +147,7 @@ For simple, well-defined tasks, skip the plan phase and go straight to build.`, 5. Finalize: - confirm a clean working tree, then push via \`${t("push_branch")}\` (same push/prepush guidance as Build mode in *SYSTEM*) - - reply to each comment using \`${t("reply_to_review_comment")}\` + - reply to each comment **exactly once** using \`${t("reply_to_review_comment")}\` — do not re-emit the same call (the runtime dedupes identical bodies and the second call is wasted) - resolve addressed threads via \`${t("resolve_review_thread")}\` - call \`${t("report_progress")}\` with a brief summary (or the exact push error if push failed)`, },