Remove/re-request d-morrison review around AI review runs (#132)

[d-morrison]

Summary

• claude.yml: drop the head-SHA gating; remove d-morrison's review request before Claude starts and always re-request after — every Claude run now produces a fresh review notification, even on error or comment-only runs.
• claude-code-review.yml: same pattern; bump pull-requests permission to write so the workflow can manage reviewers.
• New copilot-review-request-handling.yml: mirrors the behavior for Copilot — removes d-morrison while Copilot is reviewing, re-requests when Copilot submits.

Closes #132.

Test plan

• Comment @claude <task> on a PR with d-morrison already requested — confirm the review request disappears at job start and reappears at job end (even if Claude errors)
• Open a PR (triggers claude-code-review.yml) — confirm same remove/re-request cycle around the review job
• Add Copilot as a reviewer on a PR where d-morrison is also a reviewer — confirm d-morrison is dropped, then re-requested once Copilot submits its review

🤖 Generated with Claude Code

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[Copilot]

Pull request overview

Removes the head-SHA gating in the Claude workflows so that d-morrison's review request is always dropped before AI work and always restored afterward, and introduces an analogous workflow for Copilot reviews. Together these implement the behavior requested in issue #132: a fresh review notification on every AI run, with the request suppressed while AI work is in progress.

Changes:

• claude.yml: replace SHA-capture / SHA-diff logic with unconditional remove-before / re-request-after steps.
• claude-code-review.yml: add matching remove/re-request steps and upgrade pull-requests permission from read to write.
• New copilot-review-request-handling.yml: mirrors the same remove/re-request cycle around Copilot reviews using review_requested and pull_request_review triggers.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.

File	Description
.github/workflows/claude.yml	Drops SHA gating; always removes d-morrison before Claude runs and re-requests after.
.github/workflows/claude-code-review.yml	Adds remove/re-request steps around Claude review job; bumps PR permission to write.
.github/workflows/copilot-review-request-handling.yml	New workflow that performs the same remove/re-request cycle around Copilot reviews.

Comments suppressed due to low confidence (1)

.github/workflows/copilot-review-request-handling.yml:29

• Same overly permissive substring match issue here: contains(github.event.review.user.login, 'opilot') will also match logins like tropilot or any other string containing "opilot". Prefer an explicit check against the known Copilot reviewer login(s).

    if: |
      github.event_name == 'pull_request_review' &&
      contains(github.event.review.user.login, 'opilot')

[d-morrison]

@claude review

[claude]

Claude finished @d-morrison's task in 1m 50s —— View job

---

Code Review

• Gather context and read changed files
• Review .github/workflows/claude.yml
• Review .github/workflows/claude-code-review.yml
• Review .github/workflows/copilot-review-request-handling.yml
• Post final review

---

The Copilot-identified issues were all addressed cleanly in 0566016. The overall design — use a label as cross-event state for the Copilot workflow, gate re-requests on had_reviewer — is solid. I found a few remaining concerns worth addressing.

---

.github/workflows/copilot-review-request-handling.yml

Race condition (likely to bite in practice) — The finish job gates on contains(github.event.pull_request.labels.*.name, 'ai-review-in-progress'), which reads the label list from the event payload captured when Copilot submitted its review. Copilot often submits its review within seconds of being requested; the start job takes 30–60 s to queue and run. If Copilot beats start, the pull_request_review payload will not yet contain the label, the finish condition evaluates false, and d-morrison is never re-added.

Fix: in the finish job, query the current label state via API instead of relying on the event payload:

# finish job
- name: Re-request review from d-morrison when Copilot finishes reviewing
  env:
    GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
    PR_NUMBER: ${{ github.event.pull_request.number }}
  run: |
    # Re-check live label state to avoid race with the start job
    LABELS=$(gh api "repos/${{ github.repository }}/issues/$PR_NUMBER/labels" \
      --jq '[.[].name] | any(. == "ai-review-in-progress")')
    if [ "$LABELS" != "true" ]; then
      echo "ai-review-in-progress label not present; skipping re-request"
      exit 0
    fi
    gh api -X POST ...
    gh api -X DELETE ...  # remove label

Fix this →

---

[bot] suffix in login — GitHub's REST API returns bot logins without the [bot] suffix in the login field; the suffix only appears in display contexts. So github.event.requested_reviewer.login == 'copilot-pull-request-reviewer[bot]' will likely never match. The actual login is probably copilot-pull-request-reviewer (no suffix). Verify empirically, but the [bot] branch in both start and finish may be dead code. Fix this →

---

ai-review-in-progress label must be pre-created — The label API call will 404 and emit only a warning if the label doesn't already exist in the repo. There's no doc note or bootstrapping step. Consider either documenting it (e.g., in the workflow comment block) or adding a gh label create with --force before the POST:

gh label create "ai-review-in-progress" --color "ededed" --force \
  || echo "::warning::failed to create/verify ai-review-in-progress label"

---

No cleanup on PR close — If Copilot is requested but never submits a review (PR closed, draft re-drafted, review dismissed), the ai-review-in-progress label persists. Minimal fix: add a pull_request: [closed] trigger that strips the label if it's present.

---

.github/workflows/claude.yml

Silent failure when checking reviewer status — Lines 40–41:

HAD_REVIEWER=$(gh api "repos/${{ github.repository }}/pulls/$PR_NUMBER" \
  --jq '[.requested_reviewers[].login] | any(. == "d-morrison")')

If this gh api call fails (rate-limit, transient error), HAD_REVIEWER is empty. echo "had_reviewer=$HAD_REVIEWER" then sets had_reviewer to "" — not "false" — so the re-request gate steps.remove_reviewer.outputs.had_reviewer == 'true' silently skips the re-request. Consider:

HAD_REVIEWER=$(gh api ... --jq '...' 2>&1) || { echo "::warning::..."; HAD_REVIEWER="false"; }

Fix this →

---

.github/workflows/claude-code-review.yml

Inconsistency with claude.yml: This file reads HAD_REVIEWER from the event payload (contains(github.event.pull_request.requested_reviewers.*.login, 'd-morrison')), whereas claude.yml makes a live API call. The live-call approach is slightly more accurate (no stale payload risk) but the current approach works for the pull_request trigger since the payload is fresh at event time. Low risk, worth noting for future maintainers.

---

Overall

The core design is clean and all Copilot review items were properly addressed. The race condition in copilot-review-request-handling.yml is the one issue that's likely to reproduce in normal use and worth a targeted fix.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

.github/workflows/copilot-review-request-handling.yml:39

• The start job adds the ai-review-in-progress label via POST /repos/{owner}/{repo}/issues/{N}/labels. GitHub's API will return 422 if the label does not already exist in the repository — it does not create the label on the fly. Because the call is followed by || echo "::warning::...", the failure will be silently surfaced only as a workflow warning, but more importantly the finish job (which gates on contains(github.event.pull_request.labels.*.name, 'ai-review-in-progress')) will never re-request d-morrison, breaking the round-trip. Consider creating the label in the repository (or via a pre-step using gh label create ai-review-in-progress --force) so the marker mechanism works reliably.

          gh api -X POST \
            "repos/${{ github.repository }}/issues/$PR_NUMBER/labels" \
            -f "labels[]=ai-review-in-progress" \
            || echo "::warning::failed to add ai-review-in-progress label to PR #$PR_NUMBER"

[d-morrison]

Addressed in 9aa2442:

1. Copilot workflow race — finish now queries live label state (gh api .../labels) instead of the event payload. start gains a recovery step that re-checks for an existing Copilot review after labeling and undoes the remove if found. The two paths converge regardless of order.
2. [bot] suffix — removed copilot-pull-request-reviewer[bot] variant from both start and finish conditions; only Copilot and copilot-pull-request-reviewer remain.
3. Pre-create label — added gh label create --force at the top of start, with description.
4. Cleanup on close — new cleanup_on_close job triggered by pull_request.closed strips the label if it lingers.
5. claude.yml silent failure — HAD_REVIEWER defaults to "false" on any gh api error or empty result, so the re-request step doesn't silently skip after a transient failure.

The inconsistency between claude.yml (live API check) and claude-code-review.yml (event payload) is left as-is — payload is fresh at PR-event time, so the risk is low.

[d-morrison]

@claude review and then fix

[claude]

Claude finished @d-morrison's task in 2m 39s —— View job

---

Review (post-9aa2442)

• Read all 3 changed workflow files
• Review current state

---

All issues from the first review round and all items addressed in 9aa2442 look correct. Two items from Copilot's second review were not addressed and are the main remaining concerns.

---

.github/workflows/copilot-review-request-handling.yml

Unaddressed: reviewer-ordering gap in start condition (line 24)

The start job only fires when both (Copilot is requested) && (d-morrison is already in requested_reviewers). This covers the normal case — d-morrison was added earlier and Copilot is requested later. But it misses:

d-morrison is added as a reviewer after Copilot is already reviewing (label is present).

In that scenario, the start event never fires (because d-morrison wasn't in the list when Copilot was requested), so d-morrison's request is never removed. When Copilot submits its review, finish sees the label, re-requests d-morrison — but d-morrison was never removed, so they get double-notified.

The larger problem: if d-morrison is added while Copilot's review is in flight, d-morrison gets an immediate notification and could review before Copilot finishes. The original goal — suppress d-morrison's notification while AI is reviewing — isn't achieved.

Fix: Add a trigger on review_requested for d-morrison when the ai-review-in-progress label is already present:

on:
  pull_request:
    types: [review_requested, closed]
  pull_request_review:
    types: [submitted]

jobs:
  # ... existing start / finish / cleanup_on_close jobs ...

  start_dmorrison_added_late:
    # Handles the case where d-morrison is added as reviewer while Copilot is already reviewing.
    if: |
      github.event_name == 'pull_request' &&
      github.event.action == 'review_requested' &&
      github.event.requested_reviewer.login == 'd-morrison'
    runs-on: ubuntu-latest
    permissions:
      pull-requests: write
      issues: write
    steps:
      - name: Remove d-morrison if AI review is in progress
        env:
          GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
          PR_NUMBER: ${{ github.event.pull_request.number }}
        run: |
          HAS_LABEL=$(gh api "repos/${{ github.repository }}/issues/$PR_NUMBER/labels" \
            --jq '[.[].name] | any(. == "ai-review-in-progress")' \
            2>/dev/null) || HAS_LABEL=false
          if [ "$HAS_LABEL" != "true" ]; then
            echo "No AI review in progress; nothing to do"
            exit 0
          fi
          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"

---

Mild: finish skip message assumes start hasn't run yet (line 98)

echo "ai-review-in-progress label not present; skipping (start job has not yet removed d-morrison)"

This message is only accurate for the race-condition path. If start never ran at all (d-morrison wasn't a reviewer when Copilot was requested), the message is misleading. Suggest: "ai-review-in-progress label not present; d-morrison was not removed, nothing to restore".

---

.github/workflows/claude-code-review.yml

Unaddressed: ${{ github.event.pull_request.number }} repeated 4 times (lines 44, 46, 69, 70)

Copilot's second review flagged this. claude.yml captures PR_NUMBER as an env var; claude-code-review.yml inlines the expression in both remove_reviewer and re-request steps. Easy to fix for consistency:

      - name: Remove review request from d-morrison while Claude is reviewing
        id: remove_reviewer
        if: github.event.pull_request.number
        env:
          GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
          PR_NUMBER: ${{ github.event.pull_request.number }}
          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

Same change for the re-request step.

---

What's well-handled in 9aa2442

• Race condition in finish (live label query + recovery step in start) ✓
• [bot] suffix removed from Copilot login checks ✓
• Label pre-creation with gh label create --force ✓
• Cleanup on PR close ✓
• Silent failure in claude.yml (HAD_REVIEWER defaults to false) ✓
• PR-existence guard in claude-code-review.yml ✓

---

Fix limitation

I'm unable to implement these fixes. The GitHub App powering this action does not have permission to push changes to files in .github/workflows/. See the FAQ for more context. You can apply the two changes above manually:

1. Add a start_dmorrison_added_late job to copilot-review-request-handling.yml
2. Capture PR_NUMBER as an env var in both steps of claude-code-review.yml

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

[github-actions]

PR Preview Action v1.8.1
🚀 View preview at https://d-morrison.github.io/rpt/pr-preview/pr-133/
Built to branch gh-pages at 2026-05-19 07:48 UTC. Preview will be ready when the GitHub Pages deployment is complete.