From 418a1fa208c42e0304682954ca37ff24ff3455f8 Mon Sep 17 00:00:00 2001 From: d-morrison Date: Mon, 3 Aug 2026 15:25:55 -0700 Subject: [PATCH 1/2] start: deny reviewer gh pr comment to stop duplicate review comments (closes #381) From b2a29fc959b529e3cbb64223532a220db07f42da Mon Sep 17 00:00:00 2001 From: d-morrison Date: Mon, 3 Aug 2026 15:40:43 -0700 Subject: [PATCH 2/2] fix(review): deny reviewer gh pr comment so it stops duplicate-posting The reviewer agent self-posted its `## Code review` summary via `gh pr comment` (as claude[bot]) while the workflow's "Post review comment" step also posted it (as github-actions[bot]), so every review round left two top-level comments. Three places claimed `gh pr comment` was "not granted / denied", but it was only absent from the allowlist -- the code-review plugin's command frontmatter re-granted it, and nothing in --disallowedTools took it away. - Deny `Bash(gh pr comment:*)` in run-claude-review-attempt's --disallowedTools, and reword the reviewer prompt to OUTPUT its review (findings + Verdict) as its final message instead of posting a top-level comment. The workflow posts it, with run-link header, collapse, and cost linkage the self-post lacked. - Switch the prior-review-context fetch and the collapse step to match the github-actions[bot] author the workflow posts under (both admit claude[bot] too, for tag-mode tracking comments). The collapse step previously matched claude[bot] alone, so it silently folded nothing in agent mode. - Correct the stale "not granted / necessarily denied" comments in the composite header and check-review-execution.sh. Retires the raw-gh-pr-comment-republishing class (#312, #318, #381): with the tool denied the agent never issues the command, so there is nothing to republish. Closes #381. Co-Authored-By: Claude Opus 4.8 --- .../run-claude-review-attempt/action.yml | 54 +++++++++++++------ .github/workflows/claude-code-review.yml | 38 ++++++++----- .../scripts/check-review-execution.sh | 22 ++++---- .../fix-duplicate-review-comments.fixed.md | 12 +++++ 4 files changed, 89 insertions(+), 37 deletions(-) create mode 100644 changelog.d/fix-duplicate-review-comments.fixed.md diff --git a/.github/actions/run-claude-review-attempt/action.yml b/.github/actions/run-claude-review-attempt/action.yml index 8707d16e..0b211c4c 100644 --- a/.github/actions/run-claude-review-attempt/action.yml +++ b/.github/actions/run-claude-review-attempt/action.yml @@ -175,11 +175,20 @@ runs: # is_error:false but never emits a concluding verdict block: the # same stub-review signature check-review-execution.sh detects # (gha#173, Lacaedemon/sparta#590), from this separate cause. - # `gh pr comment` — also in the plugin's declared tool list — is - # deliberately NOT granted: claude-code-review.yml posts the review - # itself via its "Post review comment" step (with dedup/collapse - # handling), so letting the plugin self-post would risk duplicate - # or uncontrolled comments. + # `gh pr comment` -- also in the plugin's declared tool list, and + # re-granted by its command frontmatter (`allowed-tools`), so a bare + # allowlist omission does NOT block it -- is explicitly DENIED in + # `--disallowedTools` below (deny beats a command's own allow-list). + # claude-code-review.yml posts the review itself via its "Post review + # comment" step (with run-link header, dedup/collapse, and cost + # linkage), so a plugin self-post produces a duplicate top-level + # comment (the agent's as `claude[bot]`, the workflow's as + # `github-actions[bot]`) that the collapse step -- which matches on + # the workflow's run URL -- can never fold. Denying the tool, plus + # the prompt below telling the agent to OUTPUT its review rather than + # post it, closes that channel; it also retires the raw-`gh pr + # comment`-republishing class (gha#312/#318/#381), since the agent + # never issues the command there is nothing to republish. # # `WebFetch` and `Bash(curl:*)` let the reviewer fetch the rendered # output a repo's `prompt-addendum` may tell it to check the diff @@ -252,7 +261,7 @@ runs: --allowedTools "mcp__github_inline_comment__create_inline_comment,Bash(python3:*),Bash(maxima:*),WebFetch,WebSearch,Bash(curl:*),Bash(gh pr view:*),Bash(gh pr diff:*),Bash(gh pr list:*),Bash(gh issue view:*),Bash(gh issue list:*),Bash(gh search:*)" --disallowedTools - "Bash(git add:*),Bash(git commit:*),Bash(git rm:*),Bash(git push:*),Bash(*git-push.sh*),Bash(python3 -m:*),ScheduleWakeup,SendMessage,Monitor"${{ inputs.model != '' && format(' --model {0}', inputs.model) || '' }} + "Bash(git add:*),Bash(git commit:*),Bash(git rm:*),Bash(git push:*),Bash(*git-push.sh*),Bash(gh pr comment:*),Bash(python3 -m:*),ScheduleWakeup,SendMessage,Monitor"${{ inputs.model != '' && format(' --model {0}', inputs.model) || '' }} --append-system-prompt "This is a review-only run. Report every issue you find as part of your review. Do NOT modify, stage, commit, or push files, and @@ -271,10 +280,20 @@ runs: check it. More generally: a denied tool call is never a reason to stop the review early. If any tool call is denied for any reason, continue with the information you already - have and still finish by posting your findings and the explicit - ### Verdict line required below — do not end the run silently or + have and still finish by writing out your findings and the explicit + ### Verdict line required below -- do not end the run silently or wait for approval. + Do NOT post a top-level PR comment yourself: the `gh pr comment` + tool is intentionally disallowed, and the workflow posts your + review for you from your final output. Write your complete review + -- every finding not tied to a specific line, plus the ### Verdict + line -- as your final assistant message. (Line-anchored inline + comments via the inline-comment tool are separate and still fine + when that tool is available.) Do not try to work around the + disabled `gh pr comment` with `gh api`, `--edit-last`, or any other + command; just write the review as your message. + This run is a single, synchronous CI job with a fixed timeout and no one to resume it afterward. Do NOT spawn a background agent or sub-task to continue the review in a future turn or after this @@ -294,18 +313,20 @@ runs: run, before you write your review. Complete the entire review yourself, inline, within this - run, and end by posting the ### Verdict line before the run ends.${{ inputs.is-retry == 'true' && ' + run, and end your final message with the ### Verdict line before + the run ends.${{ inputs.is-retry == 'true' && ' This is a RETRY: an earlier attempt at this same review completed without an API error but ended without ever stating a verdict — most likely it stopped early after a denied tool call instead of continuing (gha#185). Do not repeat that outcome. However far you - get, and whatever gets denied along the way, your LAST action in - this run must be to post your findings (best-effort, noting - anything you could not fully verify) followed by the explicit - ### Verdict line. Treat reaching a verdict as a higher priority - than being exhaustive.' || '' }}" + get, and whatever gets denied along the way, your final message in + this run must state your findings (best-effort, noting anything you + could not fully verify) followed by the explicit ### Verdict line. + Do not try to post a PR comment; write the review as your message + and the workflow posts it. Treat reaching a verdict as a higher + priority than being exhaustive.' || '' }}" # The job `if:` already lets dispatched runs review drafts (the # automatic pull_request path skips them via draft == false). But the # code-review skill has its OWN don't-review-drafts stop condition, so @@ -383,8 +404,9 @@ runs: **Confirming review (no new findings):** If you find no new issues — because no code changed since the last review, or every concern is already addressed — do not just say "no issues found." Find the - most recent prior review comment on this PR (look for a - `claude[bot]` comment containing a review verdict) and reference it + most recent prior review comment on this PR (the workflow posts each + round's review, so look for a comment containing a `### Verdict` + line) and reference it explicitly: state the prior verdict, link to that comment by its GitHub URL, and explain that it still stands. This lets readers know the confirming review ran and why it produced no new findings, diff --git a/.github/workflows/claude-code-review.yml b/.github/workflows/claude-code-review.yml index a58146f6..65546ba2 100644 --- a/.github/workflows/claude-code-review.yml +++ b/.github/workflows/claude-code-review.yml @@ -259,13 +259,17 @@ jobs: PR_NUMBER: ${{ github.event.pull_request.number || inputs.pr-number }} REPO: ${{ github.repository }} run: | - # Issue comments hold the summary/tracking comment (verdict). - # Filter to comments that contain a review heading to exclude - # @claude task-handler responses (which also post as claude[bot] but - # use "Claude finished…" / "Claude Code is working…" headers, not - # the "### Code Review:" / "### Verdict" heading the review workflow uses). + # Issue comments hold the summary/tracking comment (verdict). The + # review summary is posted by the workflow's own "Post review + # comment" step in agent mode (author github-actions[bot]), and by + # claude-code-action's tracking comment in tag mode (author + # claude[bot]) -- so match either bot. The "### Verdict" body test + # is what excludes non-review comments from either author: the cost + # and quota comments (github-actions[bot]) and the @claude + # task-handler responses (claude[bot], "Claude finished..." headers) + # carry no verdict heading. SUMMARY=$(gh api "repos/${REPO}/issues/${PR_NUMBER}/comments" \ - --jq '[.[] | select(.user.login == "claude[bot]" and (.body | test("### (Code Review|Verdict)")))] | .[-3:] | + --jq '[.[] | select((.user.login == "github-actions[bot]" or .user.login == "claude[bot]") and (.body | test("### (Code Review|Verdict)")))] | .[-3:] | map("=== Summary posted \(.created_at) ===\n\(.body)") | join("\n\n")' \ 2>/dev/null || true) # PR review comments hold the per-line inline findings — these are the @@ -883,11 +887,21 @@ jobs: # review (plus current cost comment) with history folded beneath it. # Only THIS workflow's comments are touched — identified by the run # each comment links to (its workflow `path` is this file) — so - # `@claude` task comments from claude.yml (same `claude[bot]` author) - # are left alone. This matches on the run-URL link alone, not comment - # content, so it folds both "Post review comment"'s and "Post cost - # comment"'s output identically (gha#219) — the cost comment includes - # its own `actions/runs/` link for exactly this reason. + # `@claude` task comments from claude.yml are left alone (their run's + # path is claude.yml, not this file). This matches on the run-URL link, + # so it folds "Post review comment"'s and "Post cost comment"'s output + # identically (gha#219) -- the cost comment includes its own + # `actions/runs/` link for exactly this reason. + # + # The author pre-filter must admit BOTH bots: the agent-mode review and + # cost comments are github-actions[bot] (posted by this workflow's own + # steps), while a tag-mode tracking comment is claude[bot] (posted by + # claude-code-action). An earlier version filtered claude[bot] alone, + # which silently folded nothing in agent mode -- the github-actions[bot] + # review/cost comments never matched, so every round's comments stayed + # expanded. The run->path check below is what actually scopes this to + # this workflow, so admitting both bots cannot touch another workflow's + # comments. # # Not gated to pull_request: a dispatched (workflow_dispatch / agent-mode) # review that wins the per-PR concurrency race must fold the earlier @@ -914,7 +928,7 @@ jobs: run: | gh api --paginate "repos/$REPO/issues/$PR_NUMBER/comments" \ --jq '.[] - | select(.user.login == "claude[bot]") + | select(.user.login == "github-actions[bot]" or .user.login == "claude[bot]") | "\(.node_id)\t\(((.body | capture("actions/runs/(?[0-9]+)").r)?) // "")"' \ | while IFS=$'\t' read -r NODE_ID RUN_ID; do [ -z "$RUN_ID" ] && continue diff --git a/.github/workflows/scripts/check-review-execution.sh b/.github/workflows/scripts/check-review-execution.sh index debe1b06..e1b877a3 100755 --- a/.github/workflows/scripts/check-review-execution.sh +++ b/.github/workflows/scripts/check-review-execution.sh @@ -103,15 +103,19 @@ fi # # denials is computed here (not just at its original use site further # below) because it also gates one of the "blocks" candidates next: a -# Bash `gh pr comment`/`gh api .../comments` call is NOT in -# run-claude-review-attempt's allowedTools (deliberately — see that -# composite action's own header comment), so any such call in the -# transcript was necessarily DENIED whenever this run had ANY permission -# denials at all (each one increments this counter). Trusting that call's -# argument text as evidence of a posted verdict is only safe when denials -# is exactly zero — otherwise a denied attempt (which never actually -# posted anything) could be mistaken for a genuine review and skip the -# stub/retry safety net entirely (gha#218 review, finding 1). +# Bash `gh pr comment`/`gh api .../comments` call. `gh pr comment` is now +# explicitly DENIED in run-claude-review-attempt's `--disallowedTools` +# (and the reviewer prompt tells the agent to OUTPUT its review rather +# than post it), and `gh api` was never on its allowlist -- so any such +# call in the transcript was necessarily DENIED, and each denial +# increments this counter. Trusting that call's argument text as evidence +# of a posted verdict is only safe when denials is exactly zero (i.e. no +# such call was attempted) -- otherwise a denied attempt (which never +# actually posted anything) could be mistaken for a genuine review and +# skip the stub/retry safety net entirely (gha#218 review, finding 1). +# With `gh pr comment` denied this branch is now largely defensive: the +# agent is instructed not to post, and the workflow's "Post review +# comment" step publishes review_text_file itself. denials="$(jq -r '.permission_denials_count // 0' <<< "$result")" # review_text_file (posted to the PR) and all_text_file (the pass/fail scan # below) must draw from the exact same candidate blocks, or a verdict this diff --git a/changelog.d/fix-duplicate-review-comments.fixed.md b/changelog.d/fix-duplicate-review-comments.fixed.md new file mode 100644 index 00000000..df67fc10 --- /dev/null +++ b/changelog.d/fix-duplicate-review-comments.fixed.md @@ -0,0 +1,12 @@ +- `claude-code-review`: the reusable review workflow no longer posts each + review twice. The reviewer agent's `gh pr comment` self-post + (`claude[bot]`) is now explicitly denied in `run-claude-review-attempt`'s + `--disallowedTools`, and the reviewer prompt tells the agent to output its + review rather than post it -- so the workflow's own "Post review comment" + step (`github-actions[bot]`) is the sole poster. This also retires the + raw-`gh pr comment`-republishing class (#312, #318, #381): with the tool + denied, the agent never issues the command there was nothing to republish. + As part of the same change, the prior-review-context fetch and the + older-comment collapse step now match the `github-actions[bot]` author the + workflow actually posts under (they previously matched `claude[bot]` alone, + so the collapse step silently folded nothing in agent mode). Closes #381.