Skip to content
Merged
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
54 changes: 38 additions & 16 deletions .github/actions/run-claude-review-attempt/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@
steps:
- name: Run Claude Code Review
id: run
uses: anthropics/claude-code-action@be7b93b1907a4abad570368f3c74b6fe3807510b # v1.0.183

Check warning on line 99 in .github/actions/run-claude-review-attempt/action.yml

View workflow job for this annotation

GitHub Actions / lint-yaml

99:84 [comments] too few spaces before comment: expected 2
with:
claude_code_oauth_token: ${{ inputs.claude-code-oauth-token }}
anthropic_api_key: ${{ inputs.anthropic-api-key }}
Expand Down Expand Up @@ -175,11 +175,20 @@
# 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.
Comment thread
d-morrison marked this conversation as resolved.
#
# `WebFetch` and `Bash(curl:*)` let the reviewer fetch the rendered
# output a repo's `prompt-addendum` may tell it to check the diff
Expand Down Expand Up @@ -252,7 +261,7 @@
--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
Expand All @@ -271,10 +280,20 @@
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
Expand All @@ -294,18 +313,20 @@
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
Expand Down Expand Up @@ -383,8 +404,9 @@
**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,
Expand Down
38 changes: 26 additions & 12 deletions .github/workflows/claude-code-review.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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/<id>` 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/<id>` 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
Expand All @@ -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/(?<r>[0-9]+)").r)?) // "")"' \
| while IFS=$'\t' read -r NODE_ID RUN_ID; do
[ -z "$RUN_ID" ] && continue
Expand Down
22 changes: 13 additions & 9 deletions .github/workflows/scripts/check-review-execution.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 12 additions & 0 deletions changelog.d/fix-duplicate-review-comments.fixed.md
Original file line number Diff line number Diff line change
@@ -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

Check warning on line 2 in changelog.d/fix-duplicate-review-comments.fixed.md

View workflow job for this annotation

GitHub Actions / new-line-breaks

Line packs more than one sentence: review twice. The reviewer agent's `gh pr comment` self-post

Check warning on line 2 in changelog.d/fix-duplicate-review-comments.fixed.md

View workflow job for this annotation

GitHub Actions / new-line-breaks

Line packs more than one sentence: 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

Check warning on line 6 in changelog.d/fix-duplicate-review-comments.fixed.md

View workflow job for this annotation

GitHub Actions / new-line-breaks

Line packs more than one sentence: step (`github-actions[bot]`) is the sole poster. This also retires the

Check warning on line 6 in changelog.d/fix-duplicate-review-comments.fixed.md

View workflow job for this annotation

GitHub Actions / new-line-breaks

Line packs more than one sentence: 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.
Comment thread
d-morrison marked this conversation as resolved.
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.

Check warning on line 12 in changelog.d/fix-duplicate-review-comments.fixed.md

View workflow job for this annotation

GitHub Actions / new-line-breaks

Line packs more than one sentence: so the collapse step silently folded nothing in agent mode). Closes #381.

Check warning on line 12 in changelog.d/fix-duplicate-review-comments.fixed.md

View workflow job for this annotation

GitHub Actions / new-line-breaks

Line packs more than one sentence: so the collapse step silently folded nothing in agent mode). Closes #381.
Loading