ci: wake the loop for Copilot reviews, not just CodeRabbit

.github/workflows/claude-pr-loop.yml

@@ -2,8 +2,12 @@ name: Claude PR Loop
 
 # Review → fix → verify → converge loop for AI-authored PRs (label: ai-loop)
 # ONLY. Triggered by CodeRabbit reviews, CI completions on claude/* branches,
-# or manual dispatch. The `gate` job is cheap shell that re-derives the PR's
-# live state and picks a mode — Claude usage is spent only in fix/verify.
+# or manual dispatch; Copilot reviews are eligible too, but every run one has
+# started so far died before creating a job, jobs have appeared only when a
+# maintainer re-ran it, and since 2026-09-06 its reviews have created no run
+# at all (the ai-development guide has the data). The `gate` job is cheap
+# shell that re-derives the PR's live state and picks a mode — Claude usage
+# is spent only in fix/verify.
 #
 # Modes:
 #   fix-ci      CI is red on the head → Claude fixes the root cause, pushes.
@@ -90,7 +94,8 @@ jobs:
     if: |
       vars.AI_LOOP_ENABLED == 'true' && (
         (github.event_name == 'pull_request_review' &&
-          github.event.review.user.login == 'coderabbitai[bot]' &&
+          (github.event.review.user.login == 'coderabbitai[bot]' ||
+            github.event.review.user.login == 'copilot-pull-request-reviewer[bot]') &&
           github.event.review.user.type == 'Bot' &&
           github.event.pull_request.state == 'open' &&
           github.event.pull_request.head.repo.full_name == github.repository &&
@@ -507,10 +512,39 @@ jobs:
           bot_name: bestaxbot
           bot_id: '300268469'
           claude_code_oauth_token: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }}
-          # Trigger actors are bots by design: coderabbitai[bot] (reviews),
-          # claude[bot] (CI workflow_run after its own pushes), and
-          # github-actions[bot] (the loop's self-dispatch continuation).
-          allowed_bots: 'coderabbitai,claude,github-actions'
+          # Trigger actors are bots by design: coderabbitai[bot] and Copilot
+          # (reviews), claude[bot] (CI workflow_run after its own pushes), and
+          # github-actions[bot] (the loop's self-dispatch continuation). The
+          # action matches this list against the RUN's actor by exact
+          # normalised login, and fix/verify run in the gate's own run, so a
+          # login the gate triggers on but this list omits fails the session's
+          # human-actor check: a red run rather than a wait (#612). The
+          # action's write-permission check is a second, narrower consumer:
+          # it runs only in PR and issue contexts (here, the review arm) and
+          # returns true for any `[bot]`-suffixed actor before reading this
+          # list, so it reaches the list only for `Copilot`, which has no
+          # suffix. On workflow_run, dispatch and schedule only the
+          # human-actor check consults it.
+          #
+          # `copilot` is the Copilot reviewer app (id 175728472), which
+          # renders as `Copilot` to Actions (the run actor, verified across
+          # every pull_request_review run this repo has had from it) and as
+          # `copilot-pull-request-reviewer[bot]` in the event payload, which
+          # is what the gate's `if:` matches; each list carries only the
+          # spelling it is compared against. Matching here is by normalised
+          # login, not by id: GitHub's other Copilot app,
+          # copilot-swe-agent[bot] (id 198982749), also renders as `Copilot`,
+          # so this entry admits either. What confines it is the gate, not
+          # the precision of this string, and each arm holds different parts
+          # of it: the review and workflow_run arms of the `if:` carry the
+          # same-repo and claude/* head tests, the workflow_dispatch arm
+          # carries neither and relies on GitHub only letting write-access
+          # actors dispatch (see the note under the `if:`), and the ai-loop
+          # label, PR-open and protected-path tests are re-derived in the
+          # gate job's shell on every arm while the two head tests are not.
+          # A CI completion with Copilot as the actor, on a claude/* head,
+          # therefore still passes all of them before fix runs.
+          allowed_bots: 'coderabbitai,claude,github-actions,copilot'
           use_commit_signing: true
           # Stream the full turn-by-turn (tool calls + permission denials) into
           # the job log. The proxy blocks the execution-output artifact's blob
@@ -886,7 +920,7 @@ jobs:
         uses: anthropics/claude-code-action@51c47629174b5dbca955cf93690fd39c8a41dadf # v1
         with:
           claude_code_oauth_token: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }}
-          allowed_bots: 'coderabbitai,claude,github-actions'
+          allowed_bots: 'coderabbitai,claude,github-actions,copilot'
           # Stream tool calls + denials to the log (blob-hosted artifact is
           # blocked by the proxy) so a rebuttal pass is diagnosable.
           show_full_output: true
@@ -953,8 +987,23 @@ jobs:
       # Thread replies/resolutions fire no workflow events — continue the
       # loop explicitly, but only if this pass made progress (otherwise a
       # broken verifier would self-dispatch forever).
+      # A green step with an empty execution_file means the session never
+      # ran: verify supplies no github_token, so the action's OIDC exchange
+      # can hit workflow validation when the PR head's copy of this file
+      # differs from main's, and it returns green having done nothing.
+      # Counting that as zero progress would pause the loop for the wrong
+      # reason; skipping this step leaves the PR for the next event instead.
+      # A red or cancelled step can also have an empty execution_file (a
+      # failure or kill before the session wrote its output), and that must
+      # still reach this step,
+      # which is verify's only terminal state: the iteration cap never
+      # counts verify, so a skipped handler here would let the sweep re-run
+      # a dying verify forever.
       - name: Continue loop
-        if: always() && steps.claude.conclusion != 'skipped'
+        if: |
+          always() && steps.claude.conclusion != 'skipped' &&
+            (steps.claude.outputs.execution_file != '' ||
+              steps.claude.conclusion != 'success')
         env:
           GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
           BOT_TOKEN: ${{ secrets.AI_LOOP_PAT }}

docs/docs/guides/getting-started/ai-development.md

@@ -213,6 +213,85 @@ separate store and none are documented here.
 | `AI_SCAN_DAILY_LIMIT`   | `20`                  | integer                | Auto scans per UTC day                                                                                                                     |
 | `AI_LOOP_COPILOT`       | `false`               | must be exactly `true` | Requests a Copilot review on loop PRs (Copilot's own automatic review skips bot-authored PRs on personal repos, so it has to be asked for) |
 
+The gate job's `if:` names Copilot alongside CodeRabbit: a submitted review from either is
+**eligible** to fire the gate on an `ai-loop` PR. That condition reads no `AI_LOOP_COPILOT`, only
+`AI_LOOP_ENABLED`, so it is live for any Copilot review, including one a maintainer requests by
+hand, which is how every Copilot review on a loop PR has arrived so far (#573, #624).
+`AI_LOOP_COPILOT` decides only whether `claude-implement.yml` asks for the review; unsetting it is
+not a kill switch for this trigger. `AI_LOOP_ENABLED=false` and removing `ai-loop` are. Eligible is
+the honest word, and the split between runs that start and runs that do not is not random. Whether a
+`Claude PR Loop` run for a Copilot review ever creates a job is decided by whether a maintainer
+re-ran it. Every run Copilot has triggered arrived as `run_attempt: 1` with `triggering_actor:
+Copilot` and died before a job existed (100 `failure` with zero jobs and 21 `action_required`, 121
+so far), so no job `if:` was evaluated. The only ones that reached a job are among the 19 a
+maintainer re-ran by hand: on a re-run `triggering_actor` becomes the re-runner, which is what the
+`allxsmith` rows in that query are, and not a record of who requested the review. Eighteen of those
+created all six jobs and evaluated the gate's `if:`, which before this change matched only
+`coderabbitai[bot]`; run `33041194214` was cancelled with none. Runs `33586960606`, `33232938014`
+and `33035152465` are three of the eighteen, on `claude/*` heads, and `33586960606` is the worked
+example: attempt 1 was `action_required` with zero jobs, attempt 2 produced the six. So there is no
+observed case of a Copilot review starting a loop run unaided. Since 2026-09-06 the shape has
+changed again: Copilot's reviews on PR #643 that day (all of them its "encountered an error"
+submissions) created no `Claude PR Loop` run at all, so the query below can come back with nothing
+new rather than a fresh startup failure, and absence of a run is the expected reading, not a broken
+query. Whether `AI_LOOP_COPILOT=true` changes any of this is untested: the variable has never been
+on, so no review requested by `claude-implement.yml` has been observed. The request itself starts
+nothing (it is made under the workflow's `GITHUB_TOKEN`, and nothing here listens for
+`review_requested`), but the review Copilot then submits is its own event under its own credentials,
+so treat that path as unknown rather than ruled out. In every case observed so far, a Copilot-only
+finding has still waited for the next natural event: a CI or deep-review completion, a CodeRabbit
+review, or the 2-hourly watchdog sweep. The widened condition removes our side of the obstacle; it
+is not a latency guarantee. Treat a `Claude PR Loop` run **with jobs** as the proof. Keep the
+`--paginate` and the `run_attempt` column: the most recent page alone is all startup failures, which
+is how the absolute version of this claim was first written, and without the attempt number the
+re-runs look like a second kind of trigger.
+
+```bash
+gh api --paginate \
+  "repos/allxsmith/bestax/actions/workflows/claude-pr-loop.yml/runs?event=pull_request_review" \
+  --jq '.workflow_runs[] | select(.actor.login=="Copilot")
+        | [.id, .run_attempt, .triggering_actor.login, .conclusion, .head_branch] | @tsv'
+```
+
+Two lists have to name the reviewer for that to work end to end, and they are matched against
+different strings. The `pull_request_review` branch of the gate job's `if:` is matched against the
+event payload (`review.user.login`); the `allowed_bots` list on the fix and verify sessions is
+matched against the **run's actor**, and nothing else. A reviewer in one but not the other either
+waits for an unrelated event (a CI or deep-review completion, the other reviewer, or the 2-hourly
+watchdog sweep) or fails the session's human-actor check outright. The action's write-permission
+check also reads `allowed_bots`, but only in PR and issue contexts and only for an actor without a
+`[bot]` suffix, which here means `Copilot` on the review arm; every other entry and every other arm
+is decided by the human-actor check alone.
+
+Those two strings are not always the same. The Copilot reviewer app renders as
+`copilot-pull-request-reviewer[bot]` in the payload and as `Copilot` to Actions, so the payload
+spelling is what makes the gate condition match and the actor spelling is what makes `allowed_bots`
+match. `allowed_bots` compares normalised logins, not account ids, and GitHub's other Copilot app
+(`copilot-swe-agent[bot]`) also renders as `Copilot`, so that entry admits either; the confinement
+is the gate, not the precision of the string, and each arm holds different parts of it. The review
+and CI-completion arms of the `if:` carry the same-repo and `claude/*` head tests; the
+manual-dispatch arm carries neither and relies on GitHub only letting write-access users dispatch.
+The `ai-loop` label, PR-open and protected-path tests are re-derived in the gate job's shell on
+every arm, and the two head tests are not. So a CI completion with `Copilot` as the actor, on a
+`claude/*` head, still passes all of them before `fix` runs. Each list carries only the spelling it
+is compared against; a hedge entry in the other place would match nothing today and, in
+`allowed_bots`, would widen an allowlist on the job holding `AI_LOOP_PAT` for no observed reason.
+CodeRabbit needs one spelling because both surfaces agree on it. When adding a reviewer, read its
+login off a real run (`gh api repos/<repo>/actions/runs/<id> --jq .actor.login`, since `gh run list`
+exposes no actor field) as well as off the API, and put each spelling where it is actually compared
+(#612).
+
+One divergence deliberately remains. The gate's `if:` names exact logins, while the `ACTIONABLE`
+counter, and the fix job's terminal check that repeats it, match the case-insensitive prefix
+`^(coderabbitai|copilot)`. A thread opened by any other `copilot-*` app, `copilot-swe-agent[bot]`
+today or a future reviewer app, is therefore still counted as work the loop owes while its review
+fires no gate run (its run actor can still reach `fix` through the CI-completion arm described
+above, where the gate confines it), which is the same wait of up to two hours that #612 describes.
+That is the safe direction: a new `copilot-*` app cannot admit itself to a session holding
+`AI_LOOP_PAT` just by existing. But it means the two agree only for the logins named above, so
+adding a reviewer means adding it to the `if:` and to `allowed_bots`, not only to the counter's
+regex.
+
 Anything that spends model usage is **explicit opt-in** — it must be present and set, and
 deleting it turns the feature off rather than on: