diff --git a/.github/workflows/claude-pr-action.yml b/.github/workflows/claude-pr-action.yml index aa6c203e2..43129d036 100644 --- a/.github/workflows/claude-pr-action.yml +++ b/.github/workflows/claude-pr-action.yml @@ -1,16 +1,34 @@ name: Claude PR Action -# Worker workflow: performs a code review or an explanatory summary on a PR. -# Triggered by claude-pr-trigger.yml via repository_dispatch, or manually. +# Single workflow: PR review or summary, triggered by label, comment, or manually. # -# client_payload schema: -# action: "review" | "summary" -# pull_number: number -# base: string (PR's merge target ref, e.g. "dev" or "release_v2.0_rocm") +# Triggers: +# - Label `claude-review` / `claude-summary` on a PR +# - Comment `/claude review` / `/claude summary` from a writer on a PR +# - Manual workflow_dispatch (re-runs) +# +# Auth model: +# - Anthropic: subscription via CLAUDE_CODE_OAUTH_TOKEN. +# - GitHub: workflow's GITHUB_TOKEN passed as `github_token` to +# claude-code-action. This skips the Anthropic OIDC App-token +# exchange (which rejects pull_request_target / issue_comment +# subjects), so this workflow can run directly on those events +# with no repository_dispatch indirection and no PAT. Cost: +# comments post as `github-actions[bot]` instead of +# `claude[bot]`. Dedup across runs uses an HTML marker +# (``) appended to every Claude-posted +# comment, so the filter is login-agnostic. +# +# Migrating to a custom GitHub App later: replace `secrets.GITHUB_TOKEN` in +# the two `github_token:` inputs (and the `GH_TOKEN` env on those steps) with +# an installation token from `actions/create-github-app-token@v1`. No other +# changes needed — the marker-based dedup keeps working across the swap. on: - repository_dispatch: - types: [claude-pr-action] + pull_request_target: + types: [labeled] + issue_comment: + types: [created] workflow_dispatch: inputs: action: @@ -27,26 +45,117 @@ on: required: false type: string -concurrency: - # One Claude job per (PR, action) at a time; cancel superseded runs. - group: claude-pr-${{ github.event.client_payload.pull_number || inputs.pr_number }}-${{ github.event.client_payload.action || inputs.action }} - cancel-in-progress: true +permissions: + contents: read + pull-requests: write + issues: write jobs: + resolve: + # Fast dispatcher: parse the event, decide whether to act, ack the user. + # Kept lightweight so PR label/comment churn doesn't queue heavy jobs. + runs-on: ubuntu-latest + if: > + github.event_name == 'workflow_dispatch' || + github.event_name == 'pull_request_target' || + (github.event_name == 'issue_comment' && github.event.issue.pull_request != null) + outputs: + action: ${{ steps.resolve.outputs.action }} + pr: ${{ steps.resolve.outputs.pr }} + base: ${{ steps.resolve.outputs.base }} + steps: + - name: Resolve action, PR number, and base branch + id: resolve + env: + GH_TOKEN: ${{ github.token }} + EVENT_NAME: ${{ github.event_name }} + LABEL_NAME: ${{ github.event.label.name }} + COMMENT_BODY: ${{ github.event.comment.body }} + AUTHOR_ASSOC: ${{ github.event.comment.author_association }} + PR_FROM_LABEL: ${{ github.event.pull_request.number }} + PR_FROM_COMMENT: ${{ github.event.issue.number }} + BASE_FROM_LABEL: ${{ github.event.pull_request.base.ref }} + INPUT_ACTION: ${{ inputs.action }} + INPUT_PR: ${{ inputs.pr_number }} + INPUT_BASE: ${{ inputs.base }} + run: | + set -euo pipefail + action=""; pr=""; base="" + + case "$EVENT_NAME" in + pull_request_target) + case "$LABEL_NAME" in + claude-review) action="review" ;; + claude-summary) action="summary" ;; + esac + pr="$PR_FROM_LABEL" + base="$BASE_FROM_LABEL" + ;; + issue_comment) + # Only writers can trigger — drop bots and outside contributors. + case "$AUTHOR_ASSOC" in + OWNER|MEMBER|COLLABORATOR) ;; + *) echo "Ignoring comment from $AUTHOR_ASSOC"; exit 0 ;; + esac + # Match `/claude ` as the first non-whitespace token. + cmd=$(printf '%s' "$COMMENT_BODY" | awk 'NR==1 && $1=="/claude" {print $2}') + case "$cmd" in + review) action="review" ;; + summary) action="summary" ;; + esac + pr="$PR_FROM_COMMENT" + ;; + workflow_dispatch) + action="$INPUT_ACTION" + pr="$INPUT_PR" + base="$INPUT_BASE" + ;; + esac + + if [[ -z "$action" ]]; then + echo "No matching action; nothing to do." + exit 0 + fi + + # Comment triggers (and workflow_dispatch w/o base) — look up the + # PR's actual merge target so the worker diffs against it. + if [[ -z "$base" ]]; then + base=$(gh pr view "$pr" \ + --repo "${{ github.repository }}" \ + --json baseRefName -q .baseRefName) + fi + + echo "action=$action" >> "$GITHUB_OUTPUT" + echo "pr=$pr" >> "$GITHUB_OUTPUT" + echo "base=$base" >> "$GITHUB_OUTPUT" + + - name: React to comment (acknowledge) + if: steps.resolve.outputs.action != '' && github.event_name == 'issue_comment' + env: + GH_TOKEN: ${{ github.token }} + run: | + gh api \ + -H "Accept: application/vnd.github+json" \ + --method POST \ + "/repos/${{ github.repository }}/issues/comments/${{ github.event.comment.id }}/reactions" \ + -f content=eyes || true + claude: + needs: resolve + if: needs.resolve.outputs.action != '' runs-on: ubuntu-latest permissions: contents: read pull-requests: write issues: write - id-token: write # Required for claude-code-action OIDC exchange. + concurrency: + # One Claude job per (PR, action) at a time; cancel superseded runs. + group: claude-pr-${{ needs.resolve.outputs.pr }}-${{ needs.resolve.outputs.action }} + cancel-in-progress: true env: - ACTION: ${{ github.event.client_payload.action || inputs.action }} - PR_NUMBER: ${{ github.event.client_payload.pull_number || inputs.pr_number }} - # Diff against the PR's actual merge target. Falls back to the repo - # default branch only if the dispatcher (or workflow_dispatch input) did - # not provide one — keeps re-runs and manual invocations functional. - BASE_REF: ${{ github.event.client_payload.base || inputs.base || github.event.repository.default_branch }} + ACTION: ${{ needs.resolve.outputs.action }} + PR_NUMBER: ${{ needs.resolve.outputs.pr }} + BASE_REF: ${{ needs.resolve.outputs.base }} steps: # refs/pull//merge is GitHub's synthetic merge commit (base tip # merged with PR head). Checking it out gives us both parents in one @@ -93,8 +202,8 @@ jobs: timeout 60 claude --print -p "Say OK" || echo "Warmup complete" # claude-code-action only auto-configures the inline-comment MCP server - # for pull_request* events. We trigger via repository_dispatch, so wire - # it up manually with the PR number from the payload. + # for pull_request* events. Wire it up manually so it works regardless + # of trigger event. - name: Configure inline-comment MCP id: mcp run: | @@ -126,9 +235,19 @@ jobs: timeout-minutes: 30 uses: anthropics/claude-code-action@v1 env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + # Same token is exposed to the model's `gh` subprocess so it can + # comment on the PR. Mirrors the `github_token:` input below. + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} with: claude_code_oauth_token: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }} + # Setting github_token short-circuits the Anthropic OIDC → App-token + # exchange in claude-code-action (src/github/token.ts). Without this + # the action would try to exchange the workflow's OIDC subject for + # the official `claude[bot]` App token, which Anthropic rejects on + # pull_request_target / issue_comment events. Trade-off: comments + # post as github-actions[bot]. Dedup uses the HTML marker in the + # prompt rather than the bot login, so this is identity-portable. + github_token: ${{ secrets.GITHUB_TOKEN }} allowed_bots: "github-actions[bot]" show_full_output: true claude_args: | @@ -146,16 +265,24 @@ jobs: diff/comparison — this works regardless of whether the merge target is the default branch or a release branch. + ## Identity & dedup + This workflow posts as `github-actions[bot]` (until a dedicated + GitHub App is provisioned). To make prior-Claude lookups robust + across that future swap, every Claude-posted comment carries the + HTML marker ``. You MUST append that marker on + its own line at the end of every comment you post in step 3. + ## 1. Gather prior context Use `gh` to enumerate signals that should shape this review: a. Prior Claude inline comments (top-level only): ``` gh api --paginate "repos/${{ github.repository }}/pulls/${{ env.PR_NUMBER }}/comments" \ - | jq -s 'add // [] | [.[] | select(.user.login == "claude[bot]" and .in_reply_to_id == null)]' + | jq -s 'add // [] | [.[] | select((.body | test("")) and .in_reply_to_id == null)]' ``` b. Prior human reviews and review comments — note any unresolved threads or themes already raised by reviewers; do not duplicate. - c. Top-level PR comments from `claude[bot]` (prior summaries). + c. Top-level PR comments containing `` (prior + summaries / review verdicts). ## 2. Produce findings Run BOTH skills below and merge their findings before posting. Each @@ -167,8 +294,8 @@ jobs: If a prior Claude review exists (step 1a returned non-empty), instruct the skill to focus on commits added since the most recent - claude[bot] inline-comment timestamp — re-reading the entire diff - is wasteful and produces duplicate noise. + marker-tagged inline-comment timestamp — re-reading the entire + diff is wasteful and produces duplicate noise. **2b. Copyright header audit** — `/copyright-check` (vendored in `.claude/skills/`). This is the AMD-side counterpart to @@ -187,15 +314,18 @@ jobs: ## 3. Post results - For each finding (from 2a or 2b), call `mcp__github_inline_comment__create_inline_comment` on the - relevant diff line. Skip findings that duplicate any comment - from step 1 (Claude's or a human reviewer's). + relevant diff line. End every comment body with a newline and + `` so subsequent runs can identify it. + Skip findings that duplicate any comment from step 1 + (Claude's or a human reviewer's). - Post ONE short top-level summary via `gh pr comment` describing - what was reviewed and the high-level verdict. Mention the - copyright audit result as a single line (e.g. "Copyright - headers: OK" or "Copyright headers: 3 files need updates — - see inline comments"). Do not repeat individual findings. + what was reviewed and the high-level verdict; end with + ``. Mention the copyright audit result as a + single line (e.g. "Copyright headers: OK" or "Copyright + headers: 3 files need updates — see inline comments"). Do not + repeat individual findings. - If this is a re-review and there are no new findings, post a - brief top-level comment saying so. + brief top-level comment saying so (still with the marker). - Do NOT post intermediate analysis or thinking to the PR. # ---- SUMMARY / WALKTHROUGH ---- @@ -205,9 +335,11 @@ jobs: timeout-minutes: 20 uses: anthropics/claude-code-action@v1 env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} with: claude_code_oauth_token: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }} + # See the review step above for why github_token is set explicitly. + github_token: ${{ secrets.GITHUB_TOKEN }} allowed_bots: "github-actions[bot]" show_full_output: true claude_args: | @@ -229,13 +361,16 @@ jobs: explanatory artifact, NOT a review — do not flag issues here. ## 1. Check for prior summaries + This workflow posts as `github-actions[bot]`; prior Claude + artifacts are tagged with the HTML marker ``. ``` gh api --paginate "repos/${{ github.repository }}/issues/${{ env.PR_NUMBER }}/comments" \ - | jq -s 'add // [] | [.[] | select(.user.login == "claude[bot]") | .body] | .[] | select(test("Claude Walkthrough"))' + | jq -s 'add // [] | [.[] | select(.body | test("")) | select(.body | test("Claude Walkthrough"))]' ``` - If a prior summary exists, edit it (gh api PATCH on the comment id) - instead of posting a new one — keep one canonical walkthrough that - reflects the current state of the PR. Otherwise, post a new one. + If a prior summary exists, edit it (gh api PATCH on the comment + id from the response above) instead of posting a new one — keep + one canonical walkthrough that reflects the current state of the + PR. Otherwise, post a new one. ## 2. Build the walkthrough Read the PR title/description and `git diff HEAD^1...HEAD^2`. @@ -266,6 +401,7 @@ jobs: --- _Generated by Claude. To request a code review, comment `/claude review`._ + ``` Keep it tight. A reader should be able to skim it in under a minute @@ -281,7 +417,7 @@ jobs: path: ${{ steps.review.outputs.execution_file || steps.summary.outputs.execution_file }} - name: Remove trigger label - if: always() && github.event_name == 'repository_dispatch' + if: always() && github.event_name == 'pull_request_target' env: GH_TOKEN: ${{ github.token }} run: | diff --git a/.github/workflows/claude-pr-trigger.yml b/.github/workflows/claude-pr-trigger.yml deleted file mode 100644 index 644c67aa9..000000000 --- a/.github/workflows/claude-pr-trigger.yml +++ /dev/null @@ -1,119 +0,0 @@ -name: Claude PR Trigger - -# Forwards PR review/summary requests to the Claude worker workflow. -# -# Two trigger surfaces: -# 1. Labels: `claude-review`, `claude-summary` -# 2. Comments on a PR: `/claude review`, `/claude summary` -# -# Why the indirection (repository_dispatch -> worker)? -# claude-code-action exchanges the GitHub OIDC token for a GitHub App -# installation token. The OIDC subject from `pull_request_target` and -# `issue_comment` events is rejected by Anthropic's token exchange endpoint; -# `repository_dispatch` produces a ref-based subject that is accepted. -# (See ROCm/xla claude_trigger.yml for prior art.) - -on: - pull_request_target: - types: [labeled] - issue_comment: - types: [created] - -permissions: - contents: read - pull-requests: read - issues: write - -jobs: - dispatch: - # Lightweight dispatcher — kept on ubuntu-latest (which has `gh` - # preinstalled) to avoid tying up a self-hosted GPU runner on every PR - # label/comment event. Mirrors the pattern in rocm-ci-dispatch.yml's - # determine_level job. - runs-on: ubuntu-latest - # Skip non-PR issue comments early. - if: > - github.event_name == 'pull_request_target' || - (github.event_name == 'issue_comment' && github.event.issue.pull_request != null) - steps: - - name: Resolve action, PR number, and base branch - id: resolve - env: - GH_TOKEN: ${{ github.token }} - EVENT_NAME: ${{ github.event_name }} - LABEL_NAME: ${{ github.event.label.name }} - COMMENT_BODY: ${{ github.event.comment.body }} - AUTHOR_ASSOC: ${{ github.event.comment.author_association }} - PR_FROM_LABEL: ${{ github.event.pull_request.number }} - PR_FROM_COMMENT: ${{ github.event.issue.number }} - # Available on pull_request_target only; empty on issue_comment. - BASE_FROM_LABEL: ${{ github.event.pull_request.base.ref }} - run: | - set -euo pipefail - action="" - pr="" - base="" - - if [[ "$EVENT_NAME" == "pull_request_target" ]]; then - case "$LABEL_NAME" in - claude-review) action="review" ;; - claude-summary) action="summary" ;; - esac - pr="$PR_FROM_LABEL" - base="$BASE_FROM_LABEL" - else - # Comment trigger: only accept from users with write access. - case "$AUTHOR_ASSOC" in - OWNER|MEMBER|COLLABORATOR) ;; - *) echo "Ignoring comment from $AUTHOR_ASSOC"; exit 0 ;; - esac - # Match `/claude ` as the first non-whitespace token. - cmd=$(printf '%s' "$COMMENT_BODY" | awk 'NR==1 && $1=="/claude" {print $2}') - case "$cmd" in - review) action="review" ;; - summary) action="summary" ;; - esac - pr="$PR_FROM_COMMENT" - fi - - if [[ -z "$action" ]]; then - echo "No matching action; nothing to dispatch." - exit 0 - fi - - # For comment triggers (and as a safety net for label triggers), look - # up the PR's actual base ref so the worker diffs against the merge - # target, not the repo default branch. - if [[ -z "$base" ]]; then - base=$(gh pr view "$pr" \ - --repo "${{ github.repository }}" \ - --json baseRefName -q .baseRefName) - fi - - echo "action=$action" >> "$GITHUB_OUTPUT" - echo "pr=$pr" >> "$GITHUB_OUTPUT" - echo "base=$base" >> "$GITHUB_OUTPUT" - - - name: React to comment (acknowledge) - if: steps.resolve.outputs.action != '' && github.event_name == 'issue_comment' - env: - GH_TOKEN: ${{ github.token }} - run: | - gh api \ - -H "Accept: application/vnd.github+json" \ - --method POST \ - "/repos/${{ github.repository }}/issues/comments/${{ github.event.comment.id }}/reactions" \ - -f content=eyes || true - - - name: Dispatch worker - if: steps.resolve.outputs.action != '' - env: - # repository_dispatch requires a PAT (or GitHub App token) — the - # default GITHUB_TOKEN returns 403 here by design (recursion guard). - GH_TOKEN: ${{ secrets.WORKFLOW_DISPATCH_TOKEN }} - run: | - gh api repos/${{ github.repository }}/dispatches \ - -f event_type=claude-pr-action \ - -f "client_payload[action]=${{ steps.resolve.outputs.action }}" \ - -f "client_payload[pull_number]=${{ steps.resolve.outputs.pr }}" \ - -f "client_payload[base]=${{ steps.resolve.outputs.base }}"