diff --git a/.github/workflows/claude-code-review.yml b/.github/workflows/claude-code-review.yml index b5e8cfd4..48a18738 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,29 @@ 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 && + 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/$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 uses: anthropics/claude-code-action@v1 @@ -42,3 +65,18 @@ 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() && + 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 }} + PR_NUMBER: ${{ github.event.pull_request.number }} + 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" + diff --git a/.github/workflows/claude.yml b/.github/workflows/claude.yml index d1f07d1f..645b19d4 100644 --- a/.github/workflows/claude.yml +++ b/.github/workflows/claude.yml @@ -30,15 +30,27 @@ 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 + 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 }}" - SHA=$(gh api "repos/${{ github.repository }}/pulls/$PR_NUMBER" --jq '.head.sha') - echo "sha=$SHA" >> "$GITHUB_OUTPUT" + # 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 + 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 \ + "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 @@ -58,21 +70,17 @@ 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) && + steps.remove_reviewer.outputs.had_reviewer == 'true' 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" \ + || 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 new file mode 100644 index 00000000..113a0e8b --- /dev/null +++ b/.github/workflows/copilot-review-request-handling.yml @@ -0,0 +1,185 @@ +name: Copilot Review Request Handling + +on: + pull_request: + 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. 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 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') && + 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 + 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 }} + PR_NUMBER: ${{ github.event.pull_request.number }} + run: | + 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" + 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" + + # 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: | + # --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" \ + -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 + + 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. + # + # 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' && + 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 + 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: + # 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' || + github.event.review.user.login == 'copilot-pull-request-reviewer') && + 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 + 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: | + # 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 + if [ "$HAS_LABEL" != "true" ]; then + echo "ai-review-in-progress label not present; d-morrison was not removed, nothing to restore" + exit 0 + fi + 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" + 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') && + github.event.pull_request.head.repo.full_name == github.repository + 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"