Merge pull request #181 from Morrison-Lab/claude/issue-178-migrate-review-workflow

Migrate claude-code-review.yml to the Morrison-Lab/gha reusable workflow

Author: d-morrison

.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=<n>` 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.