From 19db021ff18c523775714a43a68172f927f9e06c Mon Sep 17 00:00:00 2001 From: Douglas Ezra Morrison Date: Mon, 3 Aug 2026 18:03:32 -0700 Subject: [PATCH] Migrate claude-code-review.yml to the Morrison-Lab/gha reusable workflow (closes #178) Replace the bespoke anthropics/claude-code-action@v1 review workflow with a thin caller of Morrison-Lab/gha/.github/workflows/claude-code-review.yml@v2, so rpt inherits upstream hardening automatically as @v2 slides (stub-review retry, verdict/is_error guard, prior-review-context dedup, older-comment collapse, cost comment, gha#400's duplicate-review fix). Gap 2 (reviewer toggling) is resolved for free: the reusable workflow already stashes/clears all requested reviewers (human and AI) while Claude reviews and re-requests them afterward -- a superset of the removed d-morrison-only toggle. Gap 1 (tag vs agent mode): migrate in agent mode (track-progress defaults false) since upstream read_only support (anthropics/claude-code-action#1415) is unshipped and agent mode can't post inline comments (anthropics/claude-code-action#635). Reviews are summary-only for now; flip track-progress: true once one of those upstream fixes lands. rpt's R-package review guidance is preserved via the prompt-addendum input. --- .github/workflows/claude-code-review.yml | 225 +++++++---------------- 1 file changed, 67 insertions(+), 158 deletions(-) diff --git a/.github/workflows/claude-code-review.yml b/.github/workflows/claude-code-review.yml index c1806122..99b7edea 100644 --- a/.github/workflows/claude-code-review.yml +++ b/.github/workflows/claude-code-review.yml @@ -1,177 +1,86 @@ -# Automatic Claude Code review of pull requests. +# Automatic Claude Code review of pull requests -- thin caller of the central +# Morrison-Lab/gha reusable review workflow. # -# Runs the upstream `code-review@claude-code-plugins` plugin so we pick up -# any improvements to the canonical review skill, then layers an -# R-package-specific addendum on top. +# Migrated from a bespoke anthropics/claude-code-action@v1 workflow (see #178) +# so rpt inherits the upstream hardening automatically as @v2 slides: +# stub-review retry, the is_error/verdict guard, prior-review-context dedup, +# older-comment collapse, the cost comment, and gha#400's duplicate-review +# fix. The reviewer stash/restore dance is handled inside the reusable +# workflow too: all requested reviewers (human and AI, e.g. Copilot) are +# cleared while Claude reviews and re-requested when it finishes, which +# generalizes the d-morrison-only toggle this file used to carry. # -# Each run posts a fresh review comment; prior reviews are left in place -# so the PR keeps a visible history rather than a rolling sticky. +# The reusable workflow runs in AGENT MODE (track-progress: false), so reviews +# are summary-only: the upstream inline-comment tool is not initialized in +# agent mode (anthropics/claude-code-action#635), and tag mode is not safe +# until anthropics/claude-code-action#1415 (a read_only input) ships. Set +# track-progress: true here once one of those lands to restore inline comments. # -# Skips drafts, Dependabot bumps, and fork PRs (fork PRs can't read repo -# secrets). Project guidance is in CLAUDE.md. Requires the -# CLAUDE_CODE_OAUTH_TOKEN repository secret. - +# Keep this file named claude-code-review.yml and keep the workflow_dispatch +# pr_number input: claude.yml re-dispatches a review with +# `gh workflow run claude-code-review.yml -f pr_number=` after an @claude +# run pushes commits. Requires the CLAUDE_CODE_OAUTH_TOKEN repository secret. +# See Morrison-Lab/gha examples/claude-code-review.yml for the upstream stub. name: Claude Code Review on: pull_request: types: [opened, synchronize, ready_for_review, reopened] - # Allow manual re-review from the Actions UI (e.g. after an @claude run - # pushes commits). GITHUB_TOKEN pushes don't fire `synchronize`, so a - # manual dispatch path is useful. + # Lets claude.yml re-dispatch a fresh review after an @claude run pushes + # commits (GITHUB_TOKEN pushes do not fire `synchronize`), and allows a + # manual re-review from the Actions UI. workflow_dispatch: inputs: pr_number: description: 'Pull request number to review' required: true - type: number - -# Serialize reviews per PR: a newer push cancels the in-progress review -# of the now-stale diff so only the freshest review runs (and posts a -# comment). Plain `cancel-in-progress: true` is safe here — this workflow -# is read-only (its allowedTools grant no git push / commit), so it never -# pushes a fix. -# `|| inputs.pr_number` keeps a workflow_dispatch review in the same -# group as the PR's pull_request reviews so they dedupe. -concurrency: - group: claude-review-${{ github.event.pull_request.number || inputs.pr_number }} - cancel-in-progress: true + # String, not number: the reusable workflow's `pr-number` input is a + # string and `gh workflow run -f` sends one. + type: string jobs: - claude-review: - # workflow_dispatch is a manual re-review from the Actions UI, so always - # run it — this intentionally bypasses the draft/Dependabot/fork guard - # below. A human with write access explicitly requesting a review can - # review any PR. For pull_request events, skip drafts, Dependabot bumps, - # and fork PRs (fork PRs can't read repo secrets via pull_request events, - # so CLAUDE_CODE_OAUTH_TOKEN would be empty and the run would fail with a - # noisy red check). - if: | - github.event_name == 'workflow_dispatch' || - (github.event.pull_request.draft == false && - github.event.pull_request.user.login != 'dependabot[bot]' && - github.event.pull_request.head.repo.full_name == github.repository) - runs-on: ubuntu-latest - timeout-minutes: 50 - env: - # Resolve the PR number once: from the pull_request event, or the - # workflow_dispatch input when a manual re-review is triggered. - PR_NUMBER: ${{ github.event.pull_request.number || inputs.pr_number }} + review: permissions: contents: read pull-requests: write - issues: read + issues: write id-token: write - - steps: - # A PR that changes THIS workflow file can't be reviewed by the action: - # its App-token exchange requires the review workflow to match the - # default branch, so it 401s — the action itself says this is normal - # and to ignore it. Rather than post a failing check, detect that case - # and skip the review; it runs normally once merged. - - name: Skip self-review when the PR edits this workflow - id: selfmod - env: - GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} - REPO: ${{ github.repository }} - run: | - files=$(gh api "repos/$REPO/pulls/$PR_NUMBER/files" --paginate --jq '.[].filename' 2>/dev/null || true) - if printf '%s\n' "$files" | grep -qxF '.github/workflows/claude-code-review.yml'; then - echo "self_mod=true" >> "$GITHUB_OUTPUT" - echo "::notice::PR #$PR_NUMBER edits claude-code-review.yml — skipping self-review (runs after merge)." - else - echo "self_mod=false" >> "$GITHUB_OUTPUT" - fi - - - name: Checkout repository - uses: actions/checkout@v7 - with: - fetch-depth: 1 - - # Skip reviewer toggling on fork PRs: GitHub downgrades GITHUB_TOKEN - # to read-only for fork events even when `pull-requests: write` is - # declared, so the DELETE / POST below would 403. The job-level - # reviewer (Run Claude Code Review) still runs because it uses its - # own OAuth token. - - name: Remove review request from d-morrison while Claude is reviewing - id: remove_reviewer - if: | - github.event.pull_request.number && - github.event.pull_request.head.repo.full_name == github.repository - env: - GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} - HAD_REVIEWER: ${{ contains(github.event.pull_request.requested_reviewers.*.login, 'd-morrison') }} - run: | - echo "had_reviewer=$HAD_REVIEWER" >> "$GITHUB_OUTPUT" - if [ "$HAD_REVIEWER" = "true" ]; then - gh api -X DELETE \ - "repos/${{ github.repository }}/pulls/$PR_NUMBER/requested_reviewers" \ - -f "reviewers[]=d-morrison" \ - || echo "::warning::failed to remove d-morrison from reviewers on PR #$PR_NUMBER" - fi - - - name: Run Claude Code Review - id: claude-review - if: steps.selfmod.outputs.self_mod != 'true' - uses: anthropics/claude-code-action@v1 - with: - claude_code_oauth_token: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }} - # A `claude` remote session pushing a commit to a PR fires - # `synchronize` with the `claude` bot as actor; another CI - # workflow pushing a commit fires it as `github-actions[bot]`. - # Without allowing these bots the action aborts with "Workflow - # initiated by non-human actor" and posts no review. The job - # `if:` already restricts runs to same-repo, non-Dependabot PRs, - # so accepting these bots here is safe. - allowed_bots: "github-actions[bot],claude" - plugin_marketplaces: 'https://github.com/anthropics/claude-code.git' - plugins: 'code-review@claude-code-plugins' - track_progress: ${{ github.event_name == 'pull_request' && 'true' || 'false' }} - prompt: | - /code-review:code-review ${{ github.repository }}/pull/${{ env.PR_NUMBER }} - - In addition to the standard checks above, this is an R package - template following UCD-SERG standards, so also prioritize: - - 1. **R package correctness** - - Roxygen2 docs are in sync (`devtools::document()` is required - before committing; `R-check-docs.yml` enforces this). - - `NAMESPACE` and `man/` are not edited by hand. - - `README.md` is generated from `README.Rmd`; edits go there. - - 2. **R style** (`.lintr.R` is authoritative) - - snake_case names, line length ≤ 80 chars, no `T`/`F` for - TRUE/FALSE, no `:::`-style internal calls. - - Tidyverse idioms and native `|>` pipe. - - 3. **Testing** - - New/changed behaviour is covered by testthat. - - Random outputs use `set.seed()` so snapshots are deterministic. - - 4. **Changelog** - - Every user-facing change has a `NEWS.md` bullet - (`news.yaml` enforces this; a missing entry is a CI failure). - - 5. **CI hygiene** - - No new dependencies without a `DESCRIPTION` entry. - - Spell-check / lint failures are fixed at the source, - not suppressed. - - **Post line-specific findings as inline review comments** anchored - to the relevant line(s). Reserve the top-level summary for a brief - overall verdict plus any finding not tied to a specific line; - don't restate each inline comment there. Skip generic praise. - - - name: Re-request review from d-morrison when Claude finishes reviewing - if: | - always() && - github.event.pull_request.number && - github.event.pull_request.head.repo.full_name == github.repository && - steps.remove_reviewer.outputs.had_reviewer == 'true' - env: - GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} - run: | - gh api -X POST \ - "repos/${{ github.repository }}/pulls/$PR_NUMBER/requested_reviewers" \ - -f "reviewers[]=d-morrison" \ - || echo "::warning::failed to re-request d-morrison as reviewer on PR #$PR_NUMBER" + actions: read # lets the reviewer read CI status (github_ci MCP server) + uses: Morrison-Lab/gha/.github/workflows/claude-code-review.yml@v2 + secrets: + CLAUDE_CODE_OAUTH_TOKEN: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }} + with: + # Wire the workflow_dispatch input through so claude.yml can re-dispatch + # a review on Claude's commits; empty (and ignored) for pull_request runs. + pr-number: ${{ inputs.pr_number }} + # Accept the `claude` and github-actions[bot] actors so a review + # dispatched after an @claude run (or another CI push) is not rejected as + # "non-human". Matches the pre-migration workflow's allowed_bots. + allowed-bots: "github-actions[bot],claude" + prompt-addendum: | + This is an R package template following UCD-SERG standards, so in + addition to the standard checks also prioritize: + + 1. **R package correctness** + - Roxygen2 docs are in sync (`devtools::document()` is required + before committing; `R-check-docs.yml` enforces this). + - `NAMESPACE` and `man/` are not edited by hand. + - `README.md` is generated from `README.Rmd`; edits go there. + + 2. **R style** (`.lintr.R` is authoritative) + - snake_case names, line length <= 80 chars, no `T`/`F` for + TRUE/FALSE, no `:::`-style internal calls. + - Tidyverse idioms and native `|>` pipe. + + 3. **Testing** + - New/changed behaviour is covered by testthat. + - Random outputs use `set.seed()` so snapshots are deterministic. + + 4. **Changelog** + - Every user-facing change has a `NEWS.md` bullet + (`news.yaml` enforces this; a missing entry is a CI failure). + + 5. **CI hygiene** + - No new dependencies without a `DESCRIPTION` entry. + - Spell-check / lint failures are fixed at the source, + not suppressed.