diff --git a/.github/workflows/claude-pr-review.yaml b/.github/workflows/claude-pr-review.yaml index fc819598..68f1e46c 100644 --- a/.github/workflows/claude-pr-review.yaml +++ b/.github/workflows/claude-pr-review.yaml @@ -1,5 +1,10 @@ # For tips how to configure this workflow, see https://github.com/anthropics/claude-code-action. # Examples: https://github.com/anthropics/claude-code-action/blob/0cf5eeec4f908121edd03a81411b55485994f8d3/docs/solutions.md. +# +# Architecture: three jobs for least-privilege separation: +# 1. "setup" — posts a "reviewing…" tracking comment (write permissions) +# 2. "review" — runs Claude with read-only permissions, produces structured JSON +# 3. "post" — reads the JSON and posts comments to the PR (write permissions) name: Claude PR Review on: @@ -10,17 +15,76 @@ on: concurrency: group: claude-review-${{ github.event.pull_request.number }} cancel-in-progress: true + jobs: + setup: + # Posts a "reviewing..." comment immediately so the PR author gets instant feedback. + runs-on: ubuntu-latest + env: + PR_NUMBER: ${{ github.event.pull_request.number }} + permissions: + contents: read + pull-requests: write + outputs: + comment_id: ${{ steps.tracking.outputs.comment_id }} + steps: + - name: Create or update tracking comment + id: tracking + uses: actions/github-script@60a0d83039c74a4aee543508d2ffcb1c3799cdea + with: + script: | + const owner = context.repo.owner; + const repo = context.repo.repo; + const prNumber = parseInt(process.env.PR_NUMBER, 10); + const runUrl = `${process.env.GITHUB_SERVER_URL}/${process.env.GITHUB_REPOSITORY}/actions/runs/${process.env.GITHUB_RUN_ID}`; + const MARKER = ""; + + const {data: issueComments} = await github.rest.issues.listComments({ + owner, + repo, + issue_number: prNumber, + per_page: 100, + }); + + const existing = issueComments.find((c) => c.body && c.body.includes(MARKER)); + + let commentId; + if (existing) { + console.log(`Updating existing tracking comment ${existing.id}.`); + const prevBody = existing.body.replace(/\n\n\[Workflow run\]\([^)]*\)$/, ""); + await github.rest.issues.updateComment({ + owner, + repo, + comment_id: existing.id, + body: `> **Claude is re-reviewing this PR…** ([workflow run](${runUrl}))\n\n${prevBody}`, + }); + commentId = existing.id; + } else { + console.log("Creating new tracking comment."); + const {data: created} = await github.rest.issues.createComment({ + owner, + repo, + issue_number: prNumber, + body: `Claude is reviewing this PR… ([workflow run](${runUrl}))\n\n${MARKER}`, + }); + commentId = created.id; + } + + core.setOutput("comment_id", commentId); + review: + # Runs Claude with read-only permissions. Produces structured JSON output + # containing review comments and a summary. Does NOT post anything. runs-on: ubuntu-latest + needs: setup permissions: id-token: write # authentication for Claude GH bot - pull-requests: write # make comments on PR - # Note: `pull-requests: write` gives us write access to `issues/comments` API as well, - # as long as the comment is under PR. contents: read # clone repo + pull-requests: read # fetch PR comments and reviews + issues: read # fetch issue comments actions: read # see GH Actions outputs - issues: read # read issues (for context) + outputs: + structured_output: ${{ steps.claude.outputs.structured_output }} steps: # With _target mode, actions/checkout is checking out upstream, not the current PR. # This is fine, we don't want anyone to be able to inject custom CLAUDE.md. @@ -28,9 +92,47 @@ jobs: uses: actions/checkout@v6 # TODO(@pzmarzly): Unpin once https://github.com/anthropics/claude-code-action/issues/1013 is fixed. - - uses: anthropics/claude-code-action@v1.0.66 + - name: Run Claude Code + id: claude + uses: anthropics/claude-code-action@v1.0.66 env: ANTHROPIC_BASE_URL: https://openrouter.ai/api + REVIEW_SCHEMA: >- + { + "type": "object", + "required": ["comments", "summary"], + "properties": { + "summary": { + "type": "string", + "description": "Markdown summary for the top-level tracking comment" + }, + "comments": { + "type": "array", + "items": { + "type": "object", + "required": ["file", "line", "severity", "body"], + "properties": { + "file": { + "type": "string", + "description": "Path relative to repo root" + }, + "line": { + "type": "integer", + "description": "Line number on the new side of the diff" + }, + "severity": { + "type": "string", + "enum": ["must-fix", "suggestion", "nit"] + }, + "body": { + "type": "string", + "description": "Review comment body in markdown" + } + } + } + } + } + } with: # Anthropic/OpenRouter auth anthropic_api_key: ${{ secrets.ANTHROPIC_API_KEY }} @@ -43,25 +145,35 @@ jobs: PR NUMBER: ${{ github.event.pull_request.number }} HEAD SHA: ${{ github.event.pull_request.head.sha }} - Review this pull request. - Read `.claude/commands/review-pr.md` and `doc/developers/style.rst` for review guidelines. + You are a code reviewer for the ${{ github.repository }} project. Review this pull request and + produce a structured JSON result. Do NOT attempt to post comments yourself — just return the JSON. You are in the upstream repo without the patch applied. Do not apply it. + Read `.claude/commands/review-pr.md` and `doc/developers/style.rst` for review guidelines. + ## Phase 1: Gather context - Fetch the patch, PR title/body, and list of existing comments (top-level, inline, and reviews): - - `gh pr diff ${{ github.event.pull_request.number }} --patch` - - `gh pr view ${{ github.event.pull_request.number }} --json title,body` - - `gh api --paginate repos/${{ github.repository }}/issues/${{ github.event.pull_request.number }}/comments` - - `gh api --paginate repos/${{ github.repository }}/pulls/${{ github.event.pull_request.number }}/comments` - - `gh api --paginate repos/${{ github.repository }}/pulls/${{ github.event.pull_request.number }}/reviews` + Use the GitHub MCP server tools to fetch PR data. For all tools, pass + owner `${{ github.repository_owner }}`, repo `${{ github.event.repository.name }}`, + and pullNumber/issue_number ${{ github.event.pull_request.number }}: + - `mcp__github__get_pull_request_diff` to get the PR diff + - `mcp__github__get_pull_request` to get the PR title, body, and metadata + - `mcp__github__get_pull_request_comments` to get top-level PR comments + - `mcp__github__get_pull_request_reviews` to get PR reviews + - `mcp__github__get_pull_request_review_comments` to get inline review comments + + Also fetch issue comments using `mcp__github__get_issue_comments` with + issue_number ${{ github.event.pull_request.number }}. + + Look for an existing tracking comment (containing ``) + in the issue comments. If one exists, you will use it as the basis for + your `summary` in Phase 3. ## Phase 2: Parallel review subagents Read `.claude/commands/review-pr.md` for the review checklist. All build, test, coverage, and cleanup instructions do not apply — other CI workflows handle those. Also skip the "Output format" and "Report" sections - they are not applicable here. - For the remaining sections (Code review, Style, Documentation, Commit), launch subagents to review them in parallel. Group related sections as needed — use 2-8 subagents based on PR size and scope. @@ -73,11 +185,12 @@ jobs: those are for the standalone slash-command, not for subagent output. Each subagent must return a JSON array of issues: - `[{"file": "path", "line": , "severity": "must-fix|suggestion|nit", "title": "...", "body": "..."}]` + `[{"file": "path", "line": , "severity": "must-fix|suggestion|nit", "body": "..."}]` - Subagents must ONLY return the JSON array — they must NOT post comments, - call `gh`, or use `mcp__github_inline_comment__create_inline_comment`. - All posting happens in Phase 3. + `line` must be a line number from the NEW side of the diff **that appears inside + a diff hunk** (i.e. a line that is shown in the patch output). GitHub's review + comment API rejects lines outside the diff context, so never reference lines + that are not visible in the patch. Each subagent MUST verify its findings before returning them: - For style/convention claims, check at least 3 existing examples in the codebase to confirm @@ -85,65 +198,84 @@ jobs: - For "use X instead of Y" suggestions, confirm X actually exists and works for this case. - If unsure, don't include the issue. - ## Phase 3: Collect and post + ## Phase 3: Collect, deduplicate, and summarize After ALL subagents complete: 1. Collect all issues. Merge duplicates (same file, lines within 3 of each other, same problem). 2. Drop low-confidence findings. - 3. For CLAUDE.md violations that appear in 3+ existing places in the codebase, do NOT post inline comments. - Instead, add them to the 'CLAUDE.md improvements' section of the tracking comment - 4. Check existing inline review comments (fetched in Phase 1). Do NOT post an inline comment if - one already exists on the same file+line about the same problem. - 5. Check for author replies that dismiss or reject a previous comment. Do NOT re-raise an issue - the PR author has already responded to disagreeing with. - 6. Post new inline comments with `mcp__github_inline_comment__create_inline_comment`. - - Prefix ALL comments with "Claude: ". - Link format: https://github.com/${{ github.repository }}/blob/${{ github.event.pull_request.head.sha }}/README.md#L10-L15 - - Then maintain a single top-level "tracking comment" listing ALL issues as checkboxes. - Use a hidden HTML marker to find it: ``. - Look through the top-level comments fetched in Phase 1 for one containing that marker. - - **If no tracking comment exists (first run):** - Create one with `gh pr comment ${{ github.event.pull_request.number }} --body "..."` using this format: - ``` - Claude: review of # () - - - - ### Must fix - - [ ] **title** — `file:line` — short explanation - - ### Suggestions - - [ ] **title** — `file:line` — short explanation - - ### Nits - - [ ] **title** — `file:line` — short explanation - - ### CLAUDE.md improvements - - improvement suggestion - ``` - Omit empty sections. - - **If a tracking comment already exists (subsequent run):** - 1. Parse the existing checkboxes. For each old issue, check if the current patch still has - that problem (re-check the relevant lines in the new diff). If fixed, mark it `- [x]`. - If the author dismissed it, mark it `- [x] ~~title~~ (dismissed)`. - 2. Append any NEW issues found in this run that aren't already listed. - 3. Update the HEAD SHA in the header line. - 4. Edit the comment in-place. + 3. For CLAUDE.md violations that appear in 3+ existing places in the codebase, do NOT include + them in `comments`. Instead, add them to the 'CLAUDE.md improvements' section of the summary. + 4. Check the existing inline review comments fetched in Phase 1. Do NOT include a + comment if one already exists on the same file and line about the same problem. + Also check for author replies that dismiss or reject a previous comment — do NOT + re-raise an issue the PR author has already responded to disagreeing with. + 5. Prefix ALL comment bodies with a severity tag: `**must-fix**: `, `**suggestion**: `, + or `**nit**: `. + 6. Write a `summary` field in markdown for a top-level tracking comment. + + **If no existing tracking comment with review content was found (first run):** + Use this format: + ``` - printf '%s' "$BODY" > pr-review-body.txt - gh api --method PATCH repos/${{ github.repository }}/issues/comments/ -F body=@pr-review-body.txt + ## Claude review of ${{ github.repository }} #${{ github.event.pull_request.number }} (${{ github.event.pull_request.head.sha }}) + + + + ### Must fix + - [ ] **short title** — `file:line` — brief explanation + + ### Suggestions + - [ ] **short title** — `file:line` — brief explanation + + ### Nits + - [ ] **short title** — `file:line` — brief explanation + + ### CLAUDE.md improvements + - improvement suggestion ``` + + Omit empty sections. Each checkbox item must correspond to an entry in `comments`. + If there are no issues at all, write a short message saying the PR looks good. + + Throughout all phases, track any errors that prevented you from doing + your job fully: permission denials (403, "Resource not accessible by + integration"), tools that were not available, rate limits, or any other + failures that degraded the review quality. If there were any, append a + `### Errors` section listing each failed tool/action and the error + message, so maintainers can fix the workflow configuration. + + **If an existing tracking comment with review content was found (subsequent run):** + Use the existing comment as the starting point. Preserve the order and wording + of all existing items. Then apply these updates: + - Update the HEAD SHA in the header line. + - For each existing item, re-check whether the issue is still present in the + current diff. If it has been fixed, mark it checked: `- [x]`. + - If the PR author replied dismissing an item, mark it: + `- [x] ~~short title~~ (dismissed)`. + - Preserve checkbox state that was already set by previous runs or by hand. + - Append any NEW issues found in this run that aren't already listed, + in the appropriate severity section, after the existing items. + - Do NOT reorder, reword, or remove existing items. + + Return the final JSON object with your `comments` array and `summary` string. + Do NOT attempt to post comments or use any MCP tools to modify the PR. claude_args: | --max-turns 100 --model claude-opus-4-6 + --json-schema '${{ env.REVIEW_SCHEMA }}' --allowedTools " - Read,Write,Edit,MultiEdit,LS,Grep,Glob,Task, - Bash(cat:*),Bash(test:*),Bash(printf:*),Bash(jq:*),Bash(head:*),Bash(git:*),Bash(gh:*), - mcp__github_inline_comment__create_inline_comment, + Read,LS,Grep,Glob,Task, + Bash(cat:*),Bash(test:*),Bash(printf:*),Bash(jq:*),Bash(head:*),Bash(tail:*), + Bash(git:*),Bash(gh:*),Bash(grep:*),Bash(find:*),Bash(ls:*),Bash(wc:*), + Bash(diff:*),Bash(sed:*),Bash(awk:*),Bash(sort:*),Bash(uniq:*), + mcp__github__get_pull_request, + mcp__github__get_pull_request_diff, + mcp__github__get_pull_request_files, + mcp__github__get_pull_request_reviews, + mcp__github__get_pull_request_comments, + mcp__github__get_pull_request_review_comments, + mcp__github__get_pull_request_status, + mcp__github__get_issue_comments, " # Run even when triggered by 3rd party developer's PR @@ -156,3 +288,114 @@ jobs: # Enable access to other GH Actions outputs additional_permissions: | actions: read + + post: + # Reads Claude's structured JSON output and posts comments to the PR. + runs-on: ubuntu-latest + needs: [setup, review] + if: always() && needs.setup.result == 'success' + permissions: + pull-requests: write + steps: + - name: Post review comments + uses: actions/github-script@60a0d83039c74a4aee543508d2ffcb1c3799cdea + env: + STRUCTURED_OUTPUT: ${{ needs.review.outputs.structured_output }} + PR_NUMBER: ${{ github.event.pull_request.number }} + HEAD_SHA: ${{ github.event.pull_request.head.sha }} + COMMENT_ID: ${{ needs.setup.outputs.comment_id }} + with: + script: | + const owner = context.repo.owner; + const repo = context.repo.repo; + const prNumber = parseInt(process.env.PR_NUMBER, 10); + const headSha = process.env.HEAD_SHA; + const commentId = parseInt(process.env.COMMENT_ID, 10); + const runUrl = `${process.env.GITHUB_SERVER_URL}/${process.env.GITHUB_REPOSITORY}/actions/runs/${process.env.GITHUB_RUN_ID}`; + const MARKER = ""; + + /* If the review job failed or was cancelled, update the tracking + * comment to reflect that and bail out. */ + if ("${{ needs.review.result }}" !== "success") { + await github.rest.issues.updateComment({ + owner, + repo, + comment_id: commentId, + body: `Claude review failed — see [workflow run](${runUrl}) for details.\n\n${MARKER}`, + }); + core.setFailed("Review job did not succeed."); + return; + } + + /* Parse Claude's structured output. */ + const raw = process.env.STRUCTURED_OUTPUT; + console.log("Structured output from Claude:"); + console.log(raw || "(empty)"); + + let comments = []; + let summary = ""; + if (raw) { + try { + const review = JSON.parse(raw); + if (Array.isArray(review.comments)) + comments = review.comments; + if (typeof review.summary === "string") + summary = review.summary; + } catch (e) { + console.log(`Failed to parse structured output: ${e.message}`); + } + } + + console.log(`Claude produced ${comments.length} review comment(s).`); + + /* Post each inline comment individually. Using individual comments + * (rather than a review) means re-runs only add new comments instead + * of creating a whole new review. */ + let posted = 0; + for (const c of comments) { + console.log(` Posting comment on ${c.file}:${c.line}`); + try { + await github.rest.pulls.createReviewComment({ + owner, + repo, + pull_number: prNumber, + commit_id: headSha, + path: c.file, + line: c.line, + body: `Claude: ${c.body}`, + }); + posted++; + } catch (e) { + /* GitHub rejects comments on lines outside the diff context. Log + * and continue — the tracking comment still contains all findings. */ + console.log(` Warning: failed to post comment on ${c.file}:${c.line}: ${e.message}`); + } + } + + if (posted > 0) + console.log(`Posted ${posted}/${comments.length} inline comment(s).`); + else if (comments.length > 0) + console.log(`Could not post any of ${comments.length} inline comment(s) — see warnings above.`); + else + console.log("No inline comments to post."); + + const failed = comments.length > 0 && posted < comments.length; + + /* Update the tracking comment with Claude's summary. */ + if (!summary) + summary = "Claude review: no issues found :tada:\n\n" + MARKER; + else if (!summary.includes(MARKER)) + summary = summary.replace(/\n/, `\n${MARKER}\n`); + summary += `\n\n[Workflow run](${runUrl})`; + + await github.rest.issues.updateComment({ + owner, + repo, + comment_id: commentId, + body: summary, + }); + + console.log("Tracking comment updated successfully."); + + if (failed) + core.setFailed(`Failed to post ${comments.length - posted}/${comments.length} inline comment(s).`);