From 506fd8169253a08ef4e80ea7baeb955d1c676604 Mon Sep 17 00:00:00 2001 From: Douglas Ezra Morrison Date: Mon, 18 May 2026 19:02:13 -0700 Subject: [PATCH 01/10] feat: remove and re-request d-morrison review around AI review runs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #132. claude.yml: drop the head-SHA gating; remove d-morrison's review request before Claude starts and always re-request it after, so each Claude run on a PR produces a fresh review notification (even when Claude errors out or makes comment-only changes). claude-code-review.yml: same pattern. Bump pull-requests permission from read to write so the workflow can manage reviewers. copilot-review-request-handling.yml (new): mirrors the behavior for Copilot — remove d-morrison when Copilot is added as a reviewer, re-request when Copilot submits its review. Co-Authored-By: Claude Opus 4.7 (1M context) --- .github/workflows/claude-code-review.yml | 19 ++++++++- .github/workflows/claude.yml | 26 +++++------- .../copilot-review-request-handling.yml | 40 +++++++++++++++++++ 3 files changed, 67 insertions(+), 18 deletions(-) create mode 100644 .github/workflows/copilot-review-request-handling.yml diff --git a/.github/workflows/claude-code-review.yml b/.github/workflows/claude-code-review.yml index b5e8cfd4..f7b3ed68 100644 --- a/.github/workflows/claude-code-review.yml +++ b/.github/workflows/claude-code-review.yml @@ -21,7 +21,7 @@ jobs: runs-on: ubuntu-latest permissions: contents: read - pull-requests: read + pull-requests: write issues: read id-token: write @@ -31,6 +31,14 @@ jobs: with: fetch-depth: 1 + - name: Remove review request from d-morrison while Claude is reviewing + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: | + gh api -X DELETE \ + "repos/${{ github.repository }}/pulls/${{ github.event.pull_request.number }}/requested_reviewers" \ + -f "reviewers[]=d-morrison" || true + - name: Run Claude Code Review id: claude-review uses: anthropics/claude-code-action@v1 @@ -42,3 +50,12 @@ jobs: # See https://github.com/anthropics/claude-code-action/blob/main/docs/usage.md # or https://code.claude.com/docs/en/cli-reference for available options + - name: Re-request review from d-morrison when Claude finishes reviewing + if: always() + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: | + gh api -X POST \ + "repos/${{ github.repository }}/pulls/${{ github.event.pull_request.number }}/requested_reviewers" \ + -f "reviewers[]=d-morrison" || true + diff --git a/.github/workflows/claude.yml b/.github/workflows/claude.yml index d1f07d1f..bc89e297 100644 --- a/.github/workflows/claude.yml +++ b/.github/workflows/claude.yml @@ -30,15 +30,15 @@ jobs: with: fetch-depth: 1 - - name: Capture PR head SHA before Claude - id: head_before + - name: Remove review request from d-morrison while Claude is working if: github.event.pull_request.number || github.event.issue.pull_request env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} run: | PR_NUMBER="${{ github.event.pull_request.number || github.event.issue.number }}" - SHA=$(gh api "repos/${{ github.repository }}/pulls/$PR_NUMBER" --jq '.head.sha') - echo "sha=$SHA" >> "$GITHUB_OUTPUT" + gh api -X DELETE \ + "repos/${{ github.repository }}/pulls/$PR_NUMBER/requested_reviewers" \ + -f "reviewers[]=d-morrison" || true - name: Run Claude Code id: claude @@ -58,21 +58,13 @@ jobs: # or https://code.claude.com/docs/en/cli-reference for available options # claude_args: '--allowed-tools Bash(gh pr *)' - - name: Re-request review from d-morrison if Claude pushed commits - if: always() && (github.event.pull_request.number || github.event.issue.pull_request) && steps.head_before.outputs.sha != '' + - name: Re-request review from d-morrison when Claude finishes + if: always() && (github.event.pull_request.number || github.event.issue.pull_request) env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} - SHA_BEFORE: ${{ steps.head_before.outputs.sha }} run: | PR_NUMBER="${{ github.event.pull_request.number || github.event.issue.number }}" - SHA_AFTER=$(gh api "repos/${{ github.repository }}/pulls/$PR_NUMBER" --jq '.head.sha') - echo "before=$SHA_BEFORE after=$SHA_AFTER" - if [ "$SHA_AFTER" != "$SHA_BEFORE" ]; then - echo "Claude pushed new commits; re-requesting review." - gh api -X POST \ - "repos/${{ github.repository }}/pulls/$PR_NUMBER/requested_reviewers" \ - -f "reviewers[]=d-morrison" || true - else - echo "No new commits on PR head; skipping re-request." - fi + gh api -X POST \ + "repos/${{ github.repository }}/pulls/$PR_NUMBER/requested_reviewers" \ + -f "reviewers[]=d-morrison" || true diff --git a/.github/workflows/copilot-review-request-handling.yml b/.github/workflows/copilot-review-request-handling.yml new file mode 100644 index 00000000..dd42b6b3 --- /dev/null +++ b/.github/workflows/copilot-review-request-handling.yml @@ -0,0 +1,40 @@ +name: Copilot Review Request Handling + +on: + pull_request: + types: [review_requested] + pull_request_review: + types: [submitted] + +jobs: + start: + if: | + github.event_name == 'pull_request' && + contains(github.event.requested_reviewer.login, 'opilot') + runs-on: ubuntu-latest + permissions: + pull-requests: write + steps: + - name: Remove review request from d-morrison while Copilot is reviewing + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: | + gh api -X DELETE \ + "repos/${{ github.repository }}/pulls/${{ github.event.pull_request.number }}/requested_reviewers" \ + -f "reviewers[]=d-morrison" || true + + finish: + if: | + github.event_name == 'pull_request_review' && + contains(github.event.review.user.login, 'opilot') + runs-on: ubuntu-latest + permissions: + pull-requests: write + steps: + - name: Re-request review from d-morrison when Copilot finishes reviewing + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: | + gh api -X POST \ + "repos/${{ github.repository }}/pulls/${{ github.event.pull_request.number }}/requested_reviewers" \ + -f "reviewers[]=d-morrison" || true From 05660166b5a0e717e73cf8e96f6e2ce5b93bfc5d Mon Sep 17 00:00:00 2001 From: Douglas Ezra Morrison Date: Mon, 18 May 2026 19:46:11 -0700 Subject: [PATCH 02/10] fix(#133): address Copilot review feedback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Five Copilot review comments on PR #133: 1. Substring match `contains(..., 'opilot')` was overly broad — replaced with explicit equality against the two known Copilot reviewer logins (`Copilot` and `copilot-pull-request-reviewer[bot]`). 2,4. Re-request fired unconditionally on every AI completion, including PRs where d-morrison was never a reviewer. The issue text says "any review requests aimed at me" — implying d-morrison is already a reviewer — so gate the re-request on whether d-morrison was a reviewer when the run started. For claude.yml and claude-code-review.yml that state lives within a single workflow run, so use a step output (`had_reviewer`). For the Copilot workflow `start` and `finish` are separate events, so use a label (`ai-review-in-progress`) as cross-event state. 3. Replaced silent `|| true` swallowing on every gh api call with `|| echo "::warning::..."` so failures surface as workflow annotations. 5. Added `github.event.pull_request.number` guard to the re-request step in claude-code-review.yml, matching the style in claude.yml. Co-Authored-By: Claude Opus 4.7 (1M context) --- .github/workflows/claude-code-review.yml | 21 ++++++++--- .github/workflows/claude.yml | 21 ++++++++--- .../copilot-review-request-handling.yml | 35 +++++++++++++++---- 3 files changed, 61 insertions(+), 16 deletions(-) diff --git a/.github/workflows/claude-code-review.yml b/.github/workflows/claude-code-review.yml index f7b3ed68..ec90ef57 100644 --- a/.github/workflows/claude-code-review.yml +++ b/.github/workflows/claude-code-review.yml @@ -32,12 +32,19 @@ jobs: fetch-depth: 1 - 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 }} + HAD_REVIEWER: ${{ contains(github.event.pull_request.requested_reviewers.*.login, 'd-morrison') }} run: | - gh api -X DELETE \ - "repos/${{ github.repository }}/pulls/${{ github.event.pull_request.number }}/requested_reviewers" \ - -f "reviewers[]=d-morrison" || true + echo "had_reviewer=$HAD_REVIEWER" >> "$GITHUB_OUTPUT" + if [ "$HAD_REVIEWER" = "true" ]; then + gh api -X DELETE \ + "repos/${{ github.repository }}/pulls/${{ github.event.pull_request.number }}/requested_reviewers" \ + -f "reviewers[]=d-morrison" \ + || echo "::warning::failed to remove d-morrison from reviewers on PR #${{ github.event.pull_request.number }}" + fi - name: Run Claude Code Review id: claude-review @@ -51,11 +58,15 @@ jobs: # or https://code.claude.com/docs/en/cli-reference for available options - name: Re-request review from d-morrison when Claude finishes reviewing - if: always() + if: | + always() && + github.event.pull_request.number && + steps.remove_reviewer.outputs.had_reviewer == 'true' env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} run: | gh api -X POST \ "repos/${{ github.repository }}/pulls/${{ github.event.pull_request.number }}/requested_reviewers" \ - -f "reviewers[]=d-morrison" || true + -f "reviewers[]=d-morrison" \ + || echo "::warning::failed to re-request d-morrison as reviewer on PR #${{ github.event.pull_request.number }}" diff --git a/.github/workflows/claude.yml b/.github/workflows/claude.yml index bc89e297..be8ebd07 100644 --- a/.github/workflows/claude.yml +++ b/.github/workflows/claude.yml @@ -31,14 +31,21 @@ jobs: fetch-depth: 1 - name: Remove review request from d-morrison while Claude is working + id: remove_reviewer if: github.event.pull_request.number || github.event.issue.pull_request env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} run: | PR_NUMBER="${{ github.event.pull_request.number || github.event.issue.number }}" - gh api -X DELETE \ - "repos/${{ github.repository }}/pulls/$PR_NUMBER/requested_reviewers" \ - -f "reviewers[]=d-morrison" || true + HAD_REVIEWER=$(gh api "repos/${{ github.repository }}/pulls/$PR_NUMBER" \ + --jq '[.requested_reviewers[].login] | any(. == "d-morrison")') + 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 id: claude @@ -59,12 +66,16 @@ jobs: # claude_args: '--allowed-tools Bash(gh pr *)' - name: Re-request review from d-morrison when Claude finishes - if: always() && (github.event.pull_request.number || github.event.issue.pull_request) + if: | + always() && + (github.event.pull_request.number || github.event.issue.pull_request) && + steps.remove_reviewer.outputs.had_reviewer == 'true' env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} run: | PR_NUMBER="${{ github.event.pull_request.number || github.event.issue.number }}" gh api -X POST \ "repos/${{ github.repository }}/pulls/$PR_NUMBER/requested_reviewers" \ - -f "reviewers[]=d-morrison" || true + -f "reviewers[]=d-morrison" \ + || echo "::warning::failed to re-request d-morrison as reviewer on PR #$PR_NUMBER" diff --git a/.github/workflows/copilot-review-request-handling.yml b/.github/workflows/copilot-review-request-handling.yml index dd42b6b3..12328813 100644 --- a/.github/workflows/copilot-review-request-handling.yml +++ b/.github/workflows/copilot-review-request-handling.yml @@ -6,35 +6,58 @@ on: pull_request_review: types: [submitted] +# `start` and `finish` fire on different events, so we can't pass state via +# step outputs. We use the label `ai-review-in-progress` as a marker: `start` +# adds it after removing d-morrison; `finish` only re-requests d-morrison if +# the marker is present (and clears it). This keeps the workflow from adding +# d-morrison to PRs where they were never originally a reviewer. + jobs: start: if: | github.event_name == 'pull_request' && - contains(github.event.requested_reviewer.login, 'opilot') + (github.event.requested_reviewer.login == 'Copilot' || + github.event.requested_reviewer.login == 'copilot-pull-request-reviewer[bot]') && + contains(github.event.pull_request.requested_reviewers.*.login, 'd-morrison') runs-on: ubuntu-latest permissions: pull-requests: write + issues: write steps: - name: Remove review request from d-morrison while Copilot is reviewing env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + PR_NUMBER: ${{ github.event.pull_request.number }} run: | gh api -X DELETE \ - "repos/${{ github.repository }}/pulls/${{ github.event.pull_request.number }}/requested_reviewers" \ - -f "reviewers[]=d-morrison" || true + "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" + 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" finish: if: | github.event_name == 'pull_request_review' && - contains(github.event.review.user.login, 'opilot') + (github.event.review.user.login == 'Copilot' || + github.event.review.user.login == 'copilot-pull-request-reviewer[bot]') && + contains(github.event.pull_request.labels.*.name, 'ai-review-in-progress') runs-on: ubuntu-latest permissions: pull-requests: write + issues: write steps: - 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: | gh api -X POST \ - "repos/${{ github.repository }}/pulls/${{ github.event.pull_request.number }}/requested_reviewers" \ - -f "reviewers[]=d-morrison" || true + "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" + gh api -X DELETE \ + "repos/${{ github.repository }}/issues/$PR_NUMBER/labels/ai-review-in-progress" \ + || echo "::warning::failed to remove ai-review-in-progress label from PR #$PR_NUMBER" From 9aa2442984b405a9e55ba76053b12ada0de2e82c Mon Sep 17 00:00:00 2001 From: Douglas Ezra Morrison Date: Mon, 18 May 2026 20:38:08 -0700 Subject: [PATCH 03/10] fix(#133): address Claude code-review feedback (race + bot login + label) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Five issues from the @claude review on PR #133: 1. (copilot workflow) Race condition: Copilot can submit a review faster than the `start` job queues. Previously, if `finish` ran first it checked `github.event.pull_request.labels.*.name` from the event payload (no label yet), exited, and then `start` ran late and removed d-morrison without anyone re-adding them. Fix: `finish` now queries live label state via the API instead of the event payload. `start` gains a recovery step that re-checks for an existing Copilot review after applying the label — if found, it undoes the remove. The two paths converge regardless of order. 2. (copilot workflow) `copilot-pull-request-reviewer[bot]` was likely dead code — GitHub's REST `login` field strips the `[bot]` suffix. Removed the `[bot]` variant from both `start` and `finish` conditions. 3. (copilot workflow) `ai-review-in-progress` label was assumed to exist. Added a `gh label create --force` step at the top of `start`. 4. (copilot workflow) No cleanup if a PR closes without Copilot ever submitting a review (the label would linger). Added a `cleanup_on_close` job triggered by `pull_request.closed`. 5. (claude.yml) The HAD_REVIEWER `gh api` call could fail and leave the variable empty — the re-request step (gated on `== 'true'`) would silently skip, leaving d-morrison removed. Default to "false" on any API error or empty result. Co-Authored-By: Claude Opus 4.7 (1M context) --- .github/workflows/claude.yml | 7 +- .../copilot-review-request-handling.yml | 77 +++++++++++++++++-- 2 files changed, 76 insertions(+), 8 deletions(-) diff --git a/.github/workflows/claude.yml b/.github/workflows/claude.yml index be8ebd07..f90cae4c 100644 --- a/.github/workflows/claude.yml +++ b/.github/workflows/claude.yml @@ -37,8 +37,13 @@ jobs: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} run: | PR_NUMBER="${{ github.event.pull_request.number || github.event.issue.number }}" + # Default to "false" on any API/parse error so the re-request step + # (gated on had_reviewer == 'true') doesn't silently skip on a + # transient failure that left d-morrison removed. HAD_REVIEWER=$(gh api "repos/${{ github.repository }}/pulls/$PR_NUMBER" \ - --jq '[.requested_reviewers[].login] | any(. == "d-morrison")') + --jq '[.requested_reviewers[].login] | any(. == "d-morrison")' \ + 2>/dev/null) || HAD_REVIEWER=false + if [ -z "$HAD_REVIEWER" ]; then HAD_REVIEWER=false; fi echo "had_reviewer=$HAD_REVIEWER" >> "$GITHUB_OUTPUT" if [ "$HAD_REVIEWER" = "true" ]; then gh api -X DELETE \ diff --git a/.github/workflows/copilot-review-request-handling.yml b/.github/workflows/copilot-review-request-handling.yml index 12328813..df99e0d3 100644 --- a/.github/workflows/copilot-review-request-handling.yml +++ b/.github/workflows/copilot-review-request-handling.yml @@ -2,28 +2,40 @@ name: Copilot Review Request Handling on: pull_request: - types: [review_requested] + types: [review_requested, closed] pull_request_review: types: [submitted] # `start` and `finish` fire on different events, so we can't pass state via -# step outputs. We use the label `ai-review-in-progress` as a marker: `start` +# step outputs. The `ai-review-in-progress` label is the marker: `start` # adds it after removing d-morrison; `finish` only re-requests d-morrison if -# the marker is present (and clears it). This keeps the workflow from adding -# d-morrison to PRs where they were never originally a reviewer. +# the marker is currently present (queried live via API, not from the event +# payload, because Copilot can submit a review before the `start` job has +# applied the label). `cleanup_on_close` strips the label if a PR closes +# without Copilot ever submitting a review, so it doesn't linger. jobs: start: if: | github.event_name == 'pull_request' && + github.event.action == 'review_requested' && (github.event.requested_reviewer.login == 'Copilot' || - github.event.requested_reviewer.login == 'copilot-pull-request-reviewer[bot]') && + github.event.requested_reviewer.login == 'copilot-pull-request-reviewer') && contains(github.event.pull_request.requested_reviewers.*.login, 'd-morrison') runs-on: ubuntu-latest permissions: pull-requests: write issues: write steps: + - name: Ensure ai-review-in-progress label exists + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: | + gh label create "ai-review-in-progress" --color "ededed" --force \ + --description "Copilot/Claude is reviewing this PR; d-morrison's review request has been temporarily removed" \ + --repo "${{ github.repository }}" \ + || echo "::warning::failed to create ai-review-in-progress label" + - name: Remove review request from d-morrison while Copilot is reviewing env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} @@ -38,12 +50,34 @@ jobs: -f "labels[]=ai-review-in-progress" \ || echo "::warning::failed to add ai-review-in-progress label to PR #$PR_NUMBER" + # Race recovery: if Copilot submitted its review before this job got to + # run, the `finish` job will have already checked for the label, found + # it missing, and exited — leaving d-morrison removed. Re-check now + # that we hold the label, and undo if so. + - name: Recover if Copilot already submitted a review + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + PR_NUMBER: ${{ github.event.pull_request.number }} + run: | + HAS_REVIEW=$(gh api "repos/${{ github.repository }}/pulls/$PR_NUMBER/reviews" \ + --jq '[.[] | select(.user.login == "Copilot" or .user.login == "copilot-pull-request-reviewer")] | length > 0' \ + 2>/dev/null) || HAS_REVIEW=false + if [ "$HAS_REVIEW" = "true" ]; then + echo "Copilot already submitted a review; restoring d-morrison and removing label" + gh api -X POST \ + "repos/${{ github.repository }}/pulls/$PR_NUMBER/requested_reviewers" \ + -f "reviewers[]=d-morrison" \ + || echo "::warning::failed to recover d-morrison as reviewer on PR #$PR_NUMBER" + gh api -X DELETE \ + "repos/${{ github.repository }}/issues/$PR_NUMBER/labels/ai-review-in-progress" \ + || echo "::warning::failed to remove ai-review-in-progress label from PR #$PR_NUMBER" + fi + finish: if: | github.event_name == 'pull_request_review' && (github.event.review.user.login == 'Copilot' || - github.event.review.user.login == 'copilot-pull-request-reviewer[bot]') && - contains(github.event.pull_request.labels.*.name, 'ai-review-in-progress') + github.event.review.user.login == 'copilot-pull-request-reviewer') runs-on: ubuntu-latest permissions: pull-requests: write @@ -54,6 +88,16 @@ jobs: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} PR_NUMBER: ${{ github.event.pull_request.number }} run: | + # Query live label state; the event payload can be stale if `start` + # hasn't applied the label yet (in which case `start`'s recovery + # step will handle restoration). + 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 "ai-review-in-progress label not present; skipping (start job has not yet removed d-morrison)" + exit 0 + fi gh api -X POST \ "repos/${{ github.repository }}/pulls/$PR_NUMBER/requested_reviewers" \ -f "reviewers[]=d-morrison" \ @@ -61,3 +105,22 @@ jobs: gh api -X DELETE \ "repos/${{ github.repository }}/issues/$PR_NUMBER/labels/ai-review-in-progress" \ || echo "::warning::failed to remove ai-review-in-progress label from PR #$PR_NUMBER" + + cleanup_on_close: + if: | + github.event_name == 'pull_request' && + github.event.action == 'closed' && + contains(github.event.pull_request.labels.*.name, 'ai-review-in-progress') + runs-on: ubuntu-latest + permissions: + pull-requests: write + issues: write + steps: + - name: Strip stale ai-review-in-progress label + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + PR_NUMBER: ${{ github.event.pull_request.number }} + run: | + gh api -X DELETE \ + "repos/${{ github.repository }}/issues/$PR_NUMBER/labels/ai-review-in-progress" \ + || echo "::warning::failed to remove stale ai-review-in-progress label from PR #$PR_NUMBER" From 3ab255bea41ac232f378b201ae0901d376f0dbdc Mon Sep 17 00:00:00 2001 From: Douglas Ezra Morrison Date: Tue, 19 May 2026 00:41:22 -0700 Subject: [PATCH 04/10] fix(#133): address round-3 review (fork guard + author guard + comment) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - claude-code-review.yml: skip the remove/re-request steps on fork PRs. GITHUB_TOKEN is downgraded to read-only for fork events regardless of the declared `pull-requests: write` permission, so both gh api calls would 403 and emit noisy warnings. The Claude review action itself still runs (uses its own OAuth token). - copilot-review-request-handling.yml: guard the `finish` job against PRs authored by d-morrison. If a label somehow lands on a d-morrison PR, the re-request POST would return 422 (can't request the author). - claude.yml: clarify the HAD_REVIEWER fallback comment — the prior wording implied a scenario that can't happen (transient failure leaving d-morrison removed; the DELETE only runs when HAD_REVIEWER is "true", so an upstream GET failure can't have removed anything). Co-Authored-By: Claude Opus 4.7 (1M context) --- .github/workflows/claude-code-review.yml | 10 +++++++++- .github/workflows/claude.yml | 6 +++--- .github/workflows/copilot-review-request-handling.yml | 3 ++- 3 files changed, 14 insertions(+), 5 deletions(-) diff --git a/.github/workflows/claude-code-review.yml b/.github/workflows/claude-code-review.yml index ec90ef57..8d1effe7 100644 --- a/.github/workflows/claude-code-review.yml +++ b/.github/workflows/claude-code-review.yml @@ -31,9 +31,16 @@ jobs: 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 + 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') }} @@ -61,6 +68,7 @@ jobs: 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 }} diff --git a/.github/workflows/claude.yml b/.github/workflows/claude.yml index f90cae4c..645b19d4 100644 --- a/.github/workflows/claude.yml +++ b/.github/workflows/claude.yml @@ -37,9 +37,9 @@ jobs: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} run: | PR_NUMBER="${{ github.event.pull_request.number || github.event.issue.number }}" - # Default to "false" on any API/parse error so the re-request step - # (gated on had_reviewer == 'true') doesn't silently skip on a - # transient failure that left d-morrison removed. + # On any GET error / empty response, treat as "not a reviewer" so + # both the DELETE below and the matching re-request step skip, + # leaving the PR state untouched rather than partially modified. HAD_REVIEWER=$(gh api "repos/${{ github.repository }}/pulls/$PR_NUMBER" \ --jq '[.requested_reviewers[].login] | any(. == "d-morrison")' \ 2>/dev/null) || HAD_REVIEWER=false diff --git a/.github/workflows/copilot-review-request-handling.yml b/.github/workflows/copilot-review-request-handling.yml index df99e0d3..7b3c9091 100644 --- a/.github/workflows/copilot-review-request-handling.yml +++ b/.github/workflows/copilot-review-request-handling.yml @@ -77,7 +77,8 @@ jobs: if: | github.event_name == 'pull_request_review' && (github.event.review.user.login == 'Copilot' || - github.event.review.user.login == 'copilot-pull-request-reviewer') + github.event.review.user.login == 'copilot-pull-request-reviewer') && + github.event.pull_request.user.login != 'd-morrison' runs-on: ubuntu-latest permissions: pull-requests: write From 7b0634f9bdd87d589f6f68ab3136517c5dd16eb2 Mon Sep 17 00:00:00 2001 From: Douglas Ezra Morrison Date: Tue, 19 May 2026 00:43:16 -0700 Subject: [PATCH 05/10] fix(#133): paginate Copilot review lookup in start recovery The race-recovery step in copilot-review-request-handling.yml's `start` job was using a non-paginated `gh api .../reviews` call (default 30 per page). On a PR with more than 30 reviews where Copilot's submission isn't on page 1, the recovery silently misses it and leaves d-morrison removed. Switch to `--paginate` and use `head -n1` to short-circuit on the first match across all pages. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../workflows/copilot-review-request-handling.yml | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/.github/workflows/copilot-review-request-handling.yml b/.github/workflows/copilot-review-request-handling.yml index 7b3c9091..cfa7a630 100644 --- a/.github/workflows/copilot-review-request-handling.yml +++ b/.github/workflows/copilot-review-request-handling.yml @@ -59,10 +59,14 @@ jobs: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} PR_NUMBER: ${{ github.event.pull_request.number }} run: | - HAS_REVIEW=$(gh api "repos/${{ github.repository }}/pulls/$PR_NUMBER/reviews" \ - --jq '[.[] | select(.user.login == "Copilot" or .user.login == "copilot-pull-request-reviewer")] | length > 0' \ - 2>/dev/null) || HAS_REVIEW=false - if [ "$HAS_REVIEW" = "true" ]; then + # --paginate to avoid missing Copilot's review on busy PRs where + # it falls beyond the first page (30 reviews); --jq runs per page + # and outputs concatenate, so `head -n1` collapses to non-empty + # iff any page yields a match. + HAS_REVIEW_ID=$(gh api --paginate "repos/${{ github.repository }}/pulls/$PR_NUMBER/reviews" \ + --jq '.[] | select(.user.login == "Copilot" or .user.login == "copilot-pull-request-reviewer") | .id' \ + 2>/dev/null | head -n1) + if [ -n "$HAS_REVIEW_ID" ]; then echo "Copilot already submitted a review; restoring d-morrison and removing label" gh api -X POST \ "repos/${{ github.repository }}/pulls/$PR_NUMBER/requested_reviewers" \ From 150028008969d8f3cbdb5e2849a04616ccb88a7a Mon Sep 17 00:00:00 2001 From: Douglas Ezra Morrison Date: Tue, 26 May 2026 21:16:48 -0700 Subject: [PATCH 06/10] fix(#133): handle late reviewer add; clarify skip msg; dedupe PR_NUMBER MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address the remaining review findings: - copilot-review-request-handling.yml: add a `start_dmorrison_added_late` job so that if d-morrison is added as a reviewer *after* Copilot is already reviewing (ai-review-in-progress label present), their request is removed too — closing the reviewer-ordering gap where the `start` job never fires. - copilot-review-request-handling.yml: reword the `finish` skip message so it is accurate whether `start` raced or simply never ran. - claude-code-review.yml: capture PR_NUMBER as an env var in both the remove- and re-request-reviewer steps instead of inlining the expression. Co-Authored-By: Claude Opus 4.7 (1M context) --- .github/workflows/claude-code-review.yml | 10 +++--- .../copilot-review-request-handling.yml | 35 ++++++++++++++++++- 2 files changed, 40 insertions(+), 5 deletions(-) diff --git a/.github/workflows/claude-code-review.yml b/.github/workflows/claude-code-review.yml index 8d1effe7..48a18738 100644 --- a/.github/workflows/claude-code-review.yml +++ b/.github/workflows/claude-code-review.yml @@ -43,14 +43,15 @@ jobs: github.event.pull_request.head.repo.full_name == github.repository 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/${{ github.event.pull_request.number }}/requested_reviewers" \ + "repos/${{ github.repository }}/pulls/$PR_NUMBER/requested_reviewers" \ -f "reviewers[]=d-morrison" \ - || echo "::warning::failed to remove d-morrison from reviewers on PR #${{ github.event.pull_request.number }}" + || echo "::warning::failed to remove d-morrison from reviewers on PR #$PR_NUMBER" fi - name: Run Claude Code Review @@ -72,9 +73,10 @@ jobs: steps.remove_reviewer.outputs.had_reviewer == 'true' env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + PR_NUMBER: ${{ github.event.pull_request.number }} run: | gh api -X POST \ - "repos/${{ github.repository }}/pulls/${{ github.event.pull_request.number }}/requested_reviewers" \ + "repos/${{ github.repository }}/pulls/$PR_NUMBER/requested_reviewers" \ -f "reviewers[]=d-morrison" \ - || echo "::warning::failed to re-request d-morrison as reviewer on PR #${{ github.event.pull_request.number }}" + || echo "::warning::failed to re-request d-morrison as reviewer on PR #$PR_NUMBER" diff --git a/.github/workflows/copilot-review-request-handling.yml b/.github/workflows/copilot-review-request-handling.yml index cfa7a630..be2019c2 100644 --- a/.github/workflows/copilot-review-request-handling.yml +++ b/.github/workflows/copilot-review-request-handling.yml @@ -77,6 +77,39 @@ jobs: || echo "::warning::failed to remove ai-review-in-progress label from PR #$PR_NUMBER" fi + start_dmorrison_added_late: + # Closes the reviewer-ordering gap: if d-morrison is added as a reviewer + # *after* Copilot is already reviewing (the ai-review-in-progress label is + # already present), the `start` job never fires for this event — its + # requested_reviewer is d-morrison, not Copilot. Remove d-morrison here so + # their notification stays suppressed until `finish` re-requests them. + 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 an AI review is already in progress + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + PR_NUMBER: ${{ github.event.pull_request.number }} + run: | + # Query live label state rather than trusting the event payload. + 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; leaving d-morrison's review request in place" + 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" + finish: if: | github.event_name == 'pull_request_review' && @@ -100,7 +133,7 @@ jobs: --jq '[.[].name] | any(. == "ai-review-in-progress")' \ 2>/dev/null) || HAS_LABEL=false if [ "$HAS_LABEL" != "true" ]; then - echo "ai-review-in-progress label not present; skipping (start job has not yet removed d-morrison)" + echo "ai-review-in-progress label not present; d-morrison was not removed, nothing to restore" exit 0 fi gh api -X POST \ From 934ad426e50fbac161a003873506049fee9f8e6b Mon Sep 17 00:00:00 2001 From: Douglas Ezra Morrison Date: Tue, 26 May 2026 21:30:54 -0700 Subject: [PATCH 07/10] fix(#133): guard copilot-review-request-handling against fork PRs Fork-triggered pull_request / pull_request_review events get a read-only GITHUB_TOKEN, so the reviewer DELETE/POST and label mutations would 403 (masked as warnings). Gate all four jobs on head.repo.full_name == github.repository, matching claude-code-review.yml. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../workflows/copilot-review-request-handling.yml | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/.github/workflows/copilot-review-request-handling.yml b/.github/workflows/copilot-review-request-handling.yml index be2019c2..c32e3ae5 100644 --- a/.github/workflows/copilot-review-request-handling.yml +++ b/.github/workflows/copilot-review-request-handling.yml @@ -21,7 +21,8 @@ jobs: github.event.action == 'review_requested' && (github.event.requested_reviewer.login == 'Copilot' || github.event.requested_reviewer.login == 'copilot-pull-request-reviewer') && - contains(github.event.pull_request.requested_reviewers.*.login, 'd-morrison') + contains(github.event.pull_request.requested_reviewers.*.login, 'd-morrison') && + github.event.pull_request.head.repo.full_name == github.repository runs-on: ubuntu-latest permissions: pull-requests: write @@ -86,7 +87,8 @@ jobs: if: | github.event_name == 'pull_request' && github.event.action == 'review_requested' && - github.event.requested_reviewer.login == 'd-morrison' + github.event.requested_reviewer.login == 'd-morrison' && + github.event.pull_request.head.repo.full_name == github.repository runs-on: ubuntu-latest permissions: pull-requests: write @@ -115,7 +117,8 @@ jobs: github.event_name == 'pull_request_review' && (github.event.review.user.login == 'Copilot' || github.event.review.user.login == 'copilot-pull-request-reviewer') && - github.event.pull_request.user.login != 'd-morrison' + github.event.pull_request.user.login != 'd-morrison' && + github.event.pull_request.head.repo.full_name == github.repository runs-on: ubuntu-latest permissions: pull-requests: write @@ -148,7 +151,8 @@ jobs: if: | github.event_name == 'pull_request' && github.event.action == 'closed' && - contains(github.event.pull_request.labels.*.name, 'ai-review-in-progress') + contains(github.event.pull_request.labels.*.name, 'ai-review-in-progress') && + github.event.pull_request.head.repo.full_name == github.repository runs-on: ubuntu-latest permissions: pull-requests: write From 3408f5d17d0f0efd059c820b650686b8bb794437 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 9 Jun 2026 20:40:10 +0000 Subject: [PATCH 08/10] docs: document known edge cases in copilot review-request workflow Capture the three observations from the latest review as inline comments: the simultaneous-reviewer-request race in start_dmorrison_added_late, the idempotent double re-request in finish, and the rationale for the author guard on the finish job. No behavior change. --- .../copilot-review-request-handling.yml | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/.github/workflows/copilot-review-request-handling.yml b/.github/workflows/copilot-review-request-handling.yml index c32e3ae5..113a0e8b 100644 --- a/.github/workflows/copilot-review-request-handling.yml +++ b/.github/workflows/copilot-review-request-handling.yml @@ -84,6 +84,15 @@ jobs: # already present), the `start` job never fires for this event — its # requested_reviewer is d-morrison, not Copilot. Remove d-morrison here so # their notification stays suppressed until `finish` re-requests them. + # + # Known gap: if Copilot and d-morrison are requested in a single action, + # GitHub emits two `review_requested` events in an arbitrary order. When + # Copilot's fires first, `start` sees d-morrison not yet in + # requested_reviewers and skips (so no label is applied); d-morrison's + # event then reaches this job, finds no label, and also skips — leaving + # d-morrison notified immediately. Closing this would require polling, and + # failing open (an extra notification) is the safe failure mode, so we + # accept it. if: | github.event_name == 'pull_request' && github.event.action == 'review_requested' && @@ -113,6 +122,10 @@ jobs: || echo "::warning::failed to remove d-morrison from reviewers on PR #$PR_NUMBER" finish: + # The `user.login != 'd-morrison'` guard is a cheap belt-and-suspenders: + # the live label check below would exit early anyway when d-morrison is the + # author (GitHub won't let an author be a requested reviewer, so `start` + # never applies the label), but the guard keeps the intent explicit. if: | github.event_name == 'pull_request_review' && (github.event.review.user.login == 'Copilot' || @@ -132,6 +145,10 @@ jobs: # Query live label state; the event payload can be stale if `start` # hasn't applied the label yet (in which case `start`'s recovery # step will handle restoration). + # + # If Copilot submits between `start`'s label-add and its recovery + # check, both this step and `start`'s recovery may re-request + # d-morrison; the POST is idempotent, so the duplicate is harmless. HAS_LABEL=$(gh api "repos/${{ github.repository }}/issues/$PR_NUMBER/labels" \ --jq '[.[].name] | any(. == "ai-review-in-progress")' \ 2>/dev/null) || HAS_LABEL=false From ec5486ff47349e5f12be4b8ca13872aad6e4459f Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 16 Jun 2026 06:22:36 +0000 Subject: [PATCH 09/10] ci: fix docs build by sourcing quarto R package from quarto-dev/quarto-r MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The docs workflow listed `quarto-dev/quarto` as an extra-package, but that GitHub repo is the Quarto CLI monorepo (no R package at its root) — pak fails with 'Can't find R package in GitHub repo quarto-dev/quarto' and the bogus remote conflicts with the CRAN `quarto` required by the package and altdoc. The R `quarto` package now lives at quarto-dev/quarto-r; repoint to it so dependency resolution succeeds. The Quarto CLI is still installed separately by the 'Set up Quarto' step. --- .github/workflows/docs.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/docs.yaml b/.github/workflows/docs.yaml index 98fab545..2c18b5e2 100644 --- a/.github/workflows/docs.yaml +++ b/.github/workflows/docs.yaml @@ -83,7 +83,7 @@ jobs: website extra-packages: | d-morrison/altdoc@recursive-qmd-search - quarto-dev/quarto + quarto-dev/quarto-r any::sessioninfo local::. From d575b92d28834856b843845443ce0c074fb3fbad Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 16 Jun 2026 06:33:05 +0000 Subject: [PATCH 10/10] Revert "ci: fix docs build by sourcing quarto R package from quarto-dev/quarto-r" This reverts commit ec5486ff47349e5f12be4b8ca13872aad6e4459f. --- .github/workflows/docs.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/docs.yaml b/.github/workflows/docs.yaml index 2c18b5e2..98fab545 100644 --- a/.github/workflows/docs.yaml +++ b/.github/workflows/docs.yaml @@ -83,7 +83,7 @@ jobs: website extra-packages: | d-morrison/altdoc@recursive-qmd-search - quarto-dev/quarto-r + quarto-dev/quarto any::sessioninfo local::.