Repository navigation
ci: wake the loop for Copilot reviews, not just CodeRabbit - #643
Conversation
The gate counted Copilot threads as actionable work but its pull_request_review arm hard-coded coderabbitai[bot], so a Copilot-only finding waited for an unrelated event — up to the 2-hourly sweep — on a loop whose point is prompt convergence. The trigger now names both review logins. allowed_bots on the fix and verify sessions names them too: the action matches that list against the run's own actor by exact normalised login, and fix/verify run inside the gate's run, so widening the trigger alone would have turned a latency bug into a failed actor check. Every other guard is unchanged — user.type Bot, the claude/* head, the same-repo head, the ai-loop label, PR open. Closes #612
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. WalkthroughThe Claude PR loop now accepts the Copilot reviewer login, normalizes Copilot actors for fix and verify jobs, and handles verify sessions without execution output based on session status. The guide records updated observations and actor-matching behavior. ChangesCopilot review routing
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to Copilot review submissions now start the AI loop under the existing guards, and Copilot-triggered fix and verification sessions use the expected actor identity. Verification handling preserves follow-up for failed or cancelled sessions while skipping successful no-op sessions, with no current merge-blocking risk identified. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Preview DeploymentPreview URL: https://9bc81fbd.bestax.pages.dev |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Deep review — 4 blocking · 3 advisory
| # | Severity | Area | Finding | Location |
|---|---|---|---|---|
| 1 | 🔴 Critical | Correctness | allowed_bots names copilot-pull-request-reviewer, but the run actor a Copilot review produces is Copilot — the actor check still fails, which is the exact "red run instead of latency" outcome the PR says it prevents |
.github/workflows/claude-pr-loop.yml:520 |
| 2 | 🟠 Major | Correctness | Same mismatch on the verify session, where the OIDC path makes the actor check unambiguously reachable |
.github/workflows/claude-pr-loop.yml:896 |
| 3 | 🟡 Minor | Robustness | The trigger matches only the reviews-API spelling; the payload spelling was never verified against a webhook, and the other alias is proven live | .github/workflows/claude-pr-loop.yml:94-95 |
| 4 | 🟡 Minor | Correctness | The new docs paragraph teaches "name the same login in both lists" — the assumption that produced finding #1 | docs/docs/guides/getting-started/ai-development.md:216-222 |
| 5 | 🔵 Advisory | Correctness | handoff gates on CR_STATE only, so the loop can hand off before Copilot has reviewed the head; a Copilot review arriving after the ai-loop label is removed dies at skip: no ai-loop label |
.github/workflows/claude-pr-loop.yml:288 |
| 6 | 🔵 Advisory | Robustness | ACTIONABLE's ^(coderabbitai|copilot) prefix stays broader than the trigger's exact logins — a one-way divergence that degrades to latency, never a red run |
.github/workflows/claude-pr-loop.yml:221 |
| 7 | 🔵 Advisory | Robustness | A second reviewer re-reviewing on every push consumes MAX_ITERATIONS=4 faster; cr-stalled's halt message still names CodeRabbit only |
.github/workflows/claude-pr-loop.yml:1084 |
Overall: The diagnosis is right and the first half of the fix is right — the trigger genuinely was the gap, and the PR's reasoning about why allowed_bots also had to move is sound. The second half lands on the wrong string. GitHub renders the Copilot bot account (175728472) under two logins: copilot-pull-request-reviewer[bot] on the reviews API, and Copilot as the Actions run actor — and allowed_bots is matched against the actor. CodeRabbit agrees across both surfaces, which is why nothing has caught this before. Focus on finding #1: allowed_bots needs copilot, and the fix is one token in two lines plus the doc paragraph that codified the wrong invariant.
Residual risk:
- The trigger's own login string. Verified only against
/pulls/N/reviews, not apull_request_reviewpayload.review.useralmost certainly carries the reviews-API spelling, but I could not observe a webhook payload from CI, and the run-actor evidence proves the alias is live on at least one surface — so this is stated as finding #3 rather than refuted. Naming both spellings closes it without needing the answer. - The counter/trigger divergence the issue asked about is closed for the real reviewer. Verified live: GraphQL
reviewThreadsreturnscopilot-pull-request-reviewerfor Copilot threads (PR #605, 3 threads), which^(coderabbitai|copilot)matches, and the fix prompt at:599/:647-648already knows to reply-and-resolve them. What remains is a prefix-vs-exact asymmetry in the safe direction only (advisory #6). - Post-handoff findings still fall on the floor. Refuted as blocking, not as a risk: once
handoffswapsai-loop→needs-human-review, a later Copilot review fires the gate and hitsskip: no ai-loop label. Nothing is silently lost, since a human owns the PR by then — but the loop will not act on it (advisory #5). - No new untrusted-input exposure. Copilot bodies reach the fix agent through the same pinned
gh-threadhelper (staged fromorigin/main) as CodeRabbit's, the prompt already marks review bodies as untrusted data, and noBash(gh api:*)was added. Job composition, identities, credential adjacency,--allowedToolsand the action SHA are all untouched, as claimed.
🏄 Right wave, right line, wrong board — the trigger paddle-out is clean, but
allowed_botsis holding the reviews-API name while Actions is out there calling itCopilot. One word in two spots and this thing barrels; ship it after that, dude.
The account (id 175728472) renders as copilot-pull-request-reviewer[bot] through the reviews API and as Copilot to Actions. allowed_bots is matched against the run's actor, so the entry added in the previous commit never matched and fix/verify would have failed their actor check — the exact red run this change set out to avoid. Verified against seven pull_request_review runs on this repo, all actored Copilot. Both spellings are now listed in both sessions, the gate accepts either in the payload (its spelling there is not directly observable until a Copilot review reaches the widened trigger), and the guide says to read a reviewer's login off a real run as well as off the API.
Preview DeploymentPreview URL: https://bbc153af.bestax.pages.dev |
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Deep review — 1 blocking · 3 advisory
| # | Severity | Area | Finding | Location |
|---|---|---|---|---|
| 1 | 🟠 Major | Correctness | Copilot pull_request_review events do not start a Claude PR Loop run in this repo (30/33 runs startup-fail with 0 jobs; today's Copilot review on #643 created no run at all), so the widened trigger is inert and the new docs paragraph asserts parity that is not observable |
docs/docs/guides/getting-started/ai-development.md:216-218 |
| 2 | 🔵 Advisory | Security | allowed_bots is matched against the run's actor on every event shape, not just the review trigger — so copilot also now passes checkWritePermissions + checkHumanActor on the workflow_run arm, which checks no actor at all. Narrow (needs a maintainer to put Copilot on a claude/* branch) but broader than the PR body's "exactly one new event shape" |
.github/workflows/claude-pr-loop.yml:528 |
| 3 | 🔵 Advisory | Robustness | Trigger uses three exact logins while ACTIONABLE still uses the ^(coderabbitai|copilot) prefix — a future copilot-* reviewer app would be counted but not woken, i.e. #612's exact shape at smaller scale. Deliberate per the PR body; recorded so the next reader does not re-derive it |
.github/workflows/claude-pr-loop.yml:94-96 vs :222 |
| 4 | 🔵 Advisory | Robustness | verify supplies no github_token, so on the pull_request_review path it hits the OIDC workflow-validation skip while this PR's copy of claude-pr-loop.yml differs from main — the loop's own verify pass on this PR is silently a no-op (already documented in CLAUDE.md; noted because this PR is the case that triggers it) |
.github/workflows/claude-pr-loop.yml:901-904 |
Overall: The mechanism is right and the research behind it is unusually good — I re-derived every claim in the PR body against the pinned action's source and the live API, and they all hold: agent mode calls checkHumanActor unconditionally (src/modes/agent/index.ts), pull_request_review is an entity context so checkWritePermissions runs first, and both paths resolve Copilot through their is not a user / 404 catch branches into isAllowedBot — which the new list satisfies (GET /repos/allxsmith/bestax/collaborators/Copilot/permission → "Copilot is not a user", GET /users/Copilot → 404, account 175728472 → login=Copilot type=Bot). So the "second half is not optional" argument is correct, and the widening-alone version really would have converted latency into a red run. The riskiest part is not the code, it is the claim: the one thing nobody verified end to end is that a Copilot review starts a run at all, and the evidence says it does not. Look at finding 1 first — the workflow change is worth keeping either way, the docs sentence is what needs to change.
Residual risk:
- The addressed failure class (a Copilot finding sitting until an unrelated event) still occurs, and now silently. Not refuted — reproduced. 30 of 33 Copilot-actor
Claude PR Loopruns endconclusion: failurewithtotal_count: 0jobs and 0 check runs (a normally-skipped run reports 6), no other actor produces that shape across ~300 sampledpull_request_reviewruns, and the controlled pair on this very PR (allxsmithreview 15:06 → run created andskipped; Copilot review 15:28:47 → no run created in the 15:20–15:45 window) shows the gap is actor-specific and current. - The counter/trigger divergence is narrowed, not closed.
ACTIONABLEmatches^(coderabbitai|copilot)case-insensitively and GraphQL renders the thread author ascopilot-pull-request-reviewer, so today's reviewer is covered on both sides. A newcopilot-*app would be counted and not woken — the accepted trade named in the PR body, recorded as advisory 3 rather than refuted. - Downstream guards hold. Re-read rather than assumed: the gate re-derives PR state, labels, CI, threads and the iteration counter after the trigger fires, so a spurious wake is a
skipthat spends no model usage;user.type == 'Bot'still gates the review arm and both new logins resolve to the single Bot account175728472;users/Copilot404s, so no human can hold that login; the protected-path halt,--disallowedTools, the action SHA (2 occurrences, repo-wide pin intact) and everypermissions:block are untouched. CI is green (Build and Test, React 18/19, Dependency Review, CodeQL) and prettier is clean on both changed files.
🏄 Clean paddle-out on this one — the homework's legit, the action's source got read instead of guessed, and the
allowed_botscatch was a genuine save. Only thing is the wave never showed: Copilot's reviews aren't even reaching the lineup in this repo, so soften that docs line before you claim you caught it, then it's good to go.
Every pull_request_review run this repo has had actored Copilot either failed at startup with zero jobs — so no job if: is ever evaluated — or was never created; the review Copilot submitted on this PR at 15:28 created no run at all. The workflow change is still right, but the guide claimed a parity that is not observable, which is the misreading #612 was filed to prevent. It now says eligible, names the residual latency, and gives the query to re-check it.
Preview DeploymentPreview URL: https://01b5b9d6.bestax.pages.dev |
Preview DeploymentPreview URL: https://e7a21d89.bestax.pages.dev |
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Deep review — 0 blocking · 4 advisory
I chased the PR's mechanism claims to the pinned action's source and to live API data rather than reading the diff. All of them hold. No blocking defects.
| # | Severity | Area | Finding | Location |
|---|---|---|---|---|
| 1 | 🔵 Advisory | Robustness | verify's review-arm no-op is now entirely silent — green job, ~3 min of runner time, nothing on the PR; only the sweep rescues it |
.github/workflows/claude-pr-loop.yml:1002 |
| 2 | 🔵 Advisory | Correctness | The widened trigger has no observed path to execution: 121/121 Copilot-actored runs died before a job existed | .github/workflows/claude-pr-loop.yml:97 |
| 3 | 🔵 Advisory | Security | copilot normalises to a login two Copilot apps share, on the job holding AI_LOOP_PAT, across every gate arm |
.github/workflows/claude-pr-loop.yml:547 |
| 4 | 🔵 Advisory | Maintenance | Point-in-time run statistics are now baked into a published user guide and a workflow header | docs/docs/guides/getting-started/ai-development.md:225 |
What I verified empirically
| Claim | How I checked it | Result |
|---|---|---|
Copilot's review payload login is copilot-pull-request-reviewer[bot], type: Bot |
9 real reviews on this PR, all Bot, app id 175728472 — matches the comment |
✅ |
allowed_bots is consulted for the run actor on every event, not just PR contexts |
src/modes/agent/index.ts calls checkHumanActor unconditionally; run.ts gates checkWritePermissions behind isEntityContext |
✅ |
Actor Copilot reaches the allowlist rather than a permission level |
gh api users/Copilot → 404 Not Found; collaborators/Copilot/permission → 404 Copilot is not a user — the exact two catch-branch strings |
✅ |
Omitting the entry would have made a Copilot-triggered fix red, not slow |
checkHumanActor throws Workflow initiated by non-human actor on that 404 path when isAllowedBot is false |
✅ |
execution_file is empty only when no session ran |
action.yml declares it; run.ts sets it solely from runClaude, and WorkflowValidationSkipError returns before runClaude. !containsTrigger is unreachable here (a prompt is always supplied) and the failure path writes it and goes red |
✅ |
fix needs no analogous guard |
setupGitHubToken() returns OVERRIDE_GITHUB_TOKEN before the OIDC exchange, so fix never reaches workflow validation |
✅ |
| The guide's run statistics | 100 failure + 21 action_required at attempt 1; 18 skipped + 1 cancelled at attempt 2; 33586960606 attempt 2 on a claude/* head; 33041194214 the cancelled one |
✅ exact |
| All Copilot reviews on #643 are error submissions | All 9 bodies begin Copilot encountered an error and was unable to review… |
✅ |
| Pin invariant untouched | 9 occurrences, one SHA; repo-wide multi-SHA grep returns nothing | ✅ |
| Gates | check:conformance clean, format:check clean, workflow YAML parses |
✅ |
Advisory detail
1 · The silent verify no-op — the guard is correct and I could not construct a false skip. But its success case produces no PR-visible or job-visible signal: step skipped, job green, the only trace Skipping action due to workflow validation buried in the action's log. Two things make that worth naming: the moment this merges, every in-flight ai-loop branch carries a stale copy of this file, so a review-arm verify is the expected state for a while, each burning harden-runner + checkout + pnpm install to do nothing. And the rescue is real but implicit — I traced it: the sweep dispatches with no --ref, so the run executes main's copy, workflow_not_found_on_default_branch cannot fire, and the session runs. Worst case is the same ≤2 h #612 already accepts. A one-line ::warning:: on the complementary condition would make a green verify self-explanatory. Not a blocker.
2 · The trigger may still be inert — all 121 attempt-1 Copilot-actored runs died with zero jobs; the check-suite for a sampled one (33988467772) has no check-runs at all, so no if: was ever evaluated. The 19 that reached jobs were maintainer re-runs. The PR says this outright, which is the right call — I flag it so the acceptance is read correctly: #612's second acceptance path (document the divergence) is what actually lands; the first remains contingent on GitHub-side behaviour. The 21 action_required runs are the thread worth pulling later — an approval gate would be a fixable cause, unlike a transient.
3 · The allowlist growth — recorded because .github/CLAUDE.md rule 2 asks for it, not because I think it is wrong. copilot normalises to a login shared by copilot-pull-request-reviewer[bot] (175728472) and copilot-swe-agent[bot] (198982749), on every arm the gate fires on including workflow_run. I confirmed the confinement is real: the review and workflow_run arms carry the same-repo and claude/* head tests, and the gate shell re-derives PR-open, ai-loop and the protected-path halt on all arms. Worth holding onto that the pre-change failure mode for that actor was a red run, so this converts an error into a session rather than opening a previously closed path.
4 · Statistics in published docs — the counts and four run ids reproduce exactly today, and shipping the --paginate query alongside them is the right mitigation. They remain a snapshot in a user-facing guide, and the header's since 2026-09-06 its reviews have created no run at all will age with nothing flagging it.
Overall: Sound and unusually well-evidenced — the central argument (that widening the trigger without widening allowed_bots would have converted a latency bug into a red run) is exactly right, and I confirmed it from the pinned source and the two live 404 messages its catch branches key on. The riskiest part is the Continue loop guard, since a wrong predicate there would silently break continuation for every verify pass; I traced execution_file through run.ts → run-claude-sdk.ts → execution-file.ts and it discriminates precisely the intended case. Focus first on advisory 1 — it is what every in-flight ai-loop branch hits, and it is designed to be quiet about it.
Residual risk:
- Same bug shape in
fix? No.fixpassesgithub_token: AI_LOOP_PAT, andsetupGitHubToken()returns it before the OIDC exchange, so the validation skip is structurally unreachable there — its unguardedTerminal state checkis correct as written. - Green-but-empty
execution_filethat is not the validation skip? Only!containsTrigger(unreachable — a prompt is always supplied) and awriteExecutionFileI/O failure (pathological; the sweep still rescues). Every genuine session writes the file before any throw, and every throw reddens the step, which theconclusion != 'success'clause admits. - Trigger/counter divergence still latent? Yes, deliberately:
ACTIONABLE's^(coderabbitai|copilot)still counts threads from any othercopilot-*app whose reviews fire no gate run. That is the safe direction, and the guide names it with the latency it implies — which is #612's stated acceptance. - Spoofing the new login? Not reachable: logins cannot contain
[or], the arm also requiresuser.type == 'Bot', and it still carries the same-repo andclaude/*head tests.
🏄 Clean little re-entry — it spotted that widening the gate alone would have dumped the loop straight onto the reef, so it paddled
allowed_botsout alongside it and read the action's actual source instead of guessing the break. Only bummer is the wave might not even show: 121 Copilot runs have wiped out before the lineup, and the PR tells you that to your face instead of selling you a forecast. Ship it. 🤙
Re-applying the deep-review label ran a fresh, memoryless full review each time, so a PR relabeled after every fix push was re-reviewed from scratch and the reviewer could reverse its own earlier suggestion. On #643 that happened six times across 13 rounds. When a marker review already exists, a relabel now runs in VERIFY mode: the reviewer re-examines its own open threads (verified fixed, still wrong, or concede versus one rebuttal) and reviews only the compare range from the last reviewed commit to the head, raising new findings only for executable changes there. It posts the marker summary so the next relabel diffs from this head, and it runs under a smaller turn budget. A steer comment whose first word is fresh opts back into a full review. The last reviewed commit comes from the reviews API commit_id the dedupe step already fetched; the delta is read through the compare endpoint since the session has gh api but no git.
Re-applying the deep-review label ran a fresh, memoryless full review each time, so a PR relabeled after every fix push was re-reviewed from scratch and the reviewer could reverse its own earlier suggestion. On #643 that happened six times across 13 rounds. When a marker review already exists, a relabel now runs in VERIFY mode: the reviewer re-examines its own open threads (verified fixed, still wrong, or concede versus one rebuttal) and reviews only the compare range from the last reviewed commit to the head, raising new findings only for executable changes there. It posts the marker summary so the next relabel diffs from this head, and it runs under a smaller turn budget. A steer comment whose first word is fresh opts back into a full review. The last reviewed commit comes from the reviews API commit_id the dedupe step already fetched; the delta is read through the compare endpoint since the session has gh api but no git.
|
🎉 This PR is included in version 4.2.7 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 2.3.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 1.2.4 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Re-applying the deep-review label ran a fresh, memoryless full review each time, so a PR relabeled after every fix push was re-reviewed from scratch and the reviewer could reverse its own earlier suggestion. On #643 that happened six times across 13 rounds. When a marker review already exists, a relabel now runs in VERIFY mode: the reviewer re-examines its own open threads (verified fixed, still wrong, or concede versus one rebuttal) and reviews only the compare range from the last reviewed commit to the head, raising new findings only for executable changes there. It posts the marker summary so the next relabel diffs from this head, and it runs under a smaller turn budget. A steer comment whose first word is fresh opts back into a full review. The last reviewed commit comes from the reviews API commit_id the dedupe step already fetched; the delta is read through the compare endpoint since the session has gh api but no git.
Re-applying the deep-review label ran a fresh, memoryless full review each time, so a PR relabeled after every fix push was re-reviewed from scratch and the reviewer could reverse its own earlier suggestion. On #643 that happened six times across 13 rounds. When a marker review already exists, a relabel now runs in VERIFY mode: the reviewer re-examines its own open threads (verified fixed, still wrong, or concede versus one rebuttal) and reviews only the compare range from the last reviewed commit to the head, raising new findings only for executable changes there. It posts the marker summary so the next relabel diffs from this head, and it runs under a smaller turn budget. A steer comment whose first word is fresh opts back into a full review. The last reviewed commit comes from the reviews API commit_id the dedupe step already fetched; the delta is read through the compare endpoint since the session has gh api but no git.
Re-applying the deep-review label ran a fresh, memoryless full review each time, so a PR relabeled after every fix push was re-reviewed from scratch and the reviewer could reverse its own earlier suggestion. On #643 that happened six times across 13 rounds. When a marker review already exists, a relabel now runs in VERIFY mode: the reviewer re-examines its own open threads (verified fixed, still wrong, or concede versus one rebuttal) and reviews only the compare range from the last reviewed commit to the head, raising new findings only for executable changes there. It posts the marker summary so the next relabel diffs from this head, and it runs under a smaller turn budget. A steer comment whose first word is fresh opts back into a full review. The last reviewed commit comes from the reviews API commit_id the dedupe step already fetched; the delta is read through the compare endpoint since the session has gh api but no git.
Re-applying the deep-review label ran a fresh, memoryless full review each time, so a PR relabeled after every fix push was re-reviewed from scratch and the reviewer could reverse its own earlier suggestion. On #643 that happened six times across 13 rounds. When a marker review already exists, a relabel now runs in VERIFY mode: the reviewer re-examines its own open threads (verified fixed, still wrong, or concede versus one rebuttal) and reviews only the compare range from the last reviewed commit to the head, raising new findings only for executable changes there. It posts the marker summary so the next relabel diffs from this head, and it runs under a smaller turn budget. A steer comment whose first word is fresh opts back into a full review. The last reviewed commit comes from the reviews API commit_id the dedupe step already fetched; the delta is read through the compare endpoint since the session has gh api but no git.
The deep reviewer treated every Minor as blocking and could not post an Advisory inline, so an imprecise but true comment became an inline thread and a full fix round. Its residual-risk hunt had no cap and no exemption for prose. On #643 that produced 13 review rounds for a change whose surviving logic fits in a few lines, with three quarters of the threads about wording. Prose findings (comments, docs, CLAUDE.md files, the PR body) are now Advisory unless the text is false about who can trigger a job or what a credentialed job can do; the suggested fix is a deletion, never an added qualifier, count, run id, or date. The residual-risk hunt is scoped to executable paths and capped at three posted findings. A prior-round rule is added for the verify mode a follow-up wires in. The loop's fix prompt no longer steers toward compliance on workflow comments and guide prose: it acts only when the text is false, fixes by deletion, and otherwise refutes in one line.
Re-applying the deep-review label ran a fresh, memoryless full review each time, so a PR relabeled after every fix push was re-reviewed from scratch and the reviewer could reverse its own earlier suggestion. On #643 that happened six times across 13 rounds. When a marker review already exists, a relabel now runs in VERIFY mode: the reviewer re-examines its own open threads (verified fixed, still wrong, or concede versus one rebuttal) and reviews only the compare range from the last reviewed commit to the head, raising new findings only for executable changes there. It posts the marker summary so the next relabel diffs from this head, and it runs under a smaller turn budget. A steer comment whose first word is fresh opts back into a full review. The last reviewed commit comes from the reviews API commit_id the dedupe step already fetched; the delta is read through the compare endpoint since the session has gh api but no git.
|
🎉 This PR is included in version 5.15.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Closes #612.
The gate counted Copilot threads as actionable work while its
pull_request_reviewarm hard-codedcoderabbitai[bot], so a Copilot-only finding sat until an unrelated event fired the gate — a CI or deep-review completion, a CodeRabbit review, or the 2-hourly watchdog. Up to ~2 hours of silence on a loop whose point is prompt convergence.What changed
allowed_botson the fix and verify sessions names Copilot's run-actor login too. Each list carries only the spelling it is compared against:copilot-pull-request-reviewer[bot]in the gate'sif:(the payload spelling, verified from real reviews on feat(bestax-migrate): add bloomer as a migration source #642) andcopilotinallowed_bots(the run actor, verified across every Copilot-actored run here). Earlier revisions carried the other spelling in each place as a hedge; three reviews called those entries inert, and one sat on the job holdingAI_LOOP_PAT, so they are gone.verify's Continue loop step now requires the session to have run.verifysupplies nogithub_token, so on a review-triggered run whose head copy of the workflow differs frommainthe action's OIDC exchange hits workflow validation and returns green without running; the step then read that as zero progress and paused the loop. It now also requires a non-emptyexecution_fileoutput, which the action sets only after a session ran.The second half is not optional, and the issue said otherwise
#612 concluded no
allowed_botschange was needed, reasoning that the input governs trigger actors while the sessions read threads throughgh-thread, which is author-agnostic. The first half of that is exactly why it matters:fixandverifyareneeds: gate, so they run inside the gate's own run, whose actor is the review submitter. The pinned action resolves the actor's type and, for a non-User, requires an exact normalised login match:With
'coderabbitai,claude,github-actions', a Copilot-triggered run reachesfixand fails the actor check. Widening the trigger alone would have converted a latency bug into a red run — strictly worse. The thread-reading half of the issue's reasoning stands:gh-thread.sh's GraphQL query is author-agnostic and needed nothing.What the widened trigger admits
Exactly one new event shape: a submitted review from
copilot-pull-request-reviewer[bot](verifieduser.type == Bot, same ascoderabbitai[bot]) on a PR that is open, on aclaude/*head in this repository, labelledai-loop, withAI_LOOP_ENABLED == 'true'. Exact logins rather than the counter's^(coderabbitai|copilot)prefix, so a futurecopilot-*app is not admitted by accident; the guide records the residue that leaves.Every other guard is unchanged, and the gate re-derives all live state, so a spurious trigger is a cheap
skipthat spends no model usage. The trigger set and theACTIONABLEtest now agree for the reviewer logins named in the gate; the counter's^(coderabbitai|copilot)prefix stays broader on purpose, so any othercopilot-*app is still counted but wakes nothing, and the guide documents that residue with the latency it implies, which is the issue's other acceptance path.Live on merge, not gated on
AI_LOOP_COPILOT: the gate reads onlyAI_LOOP_ENABLED, and a maintainer already requests Copilot reviews on loop PRs by hand (#573, #624), so those fire it.AI_LOOP_COPILOTdecides only whetherclaude-implement.ymlasks for the review, which it has to because Copilot's automatic review skips bot-authored PRs on personal repos. The kill switches for this path areAI_LOOP_ENABLED=falseand removingai-loop.Invariants
Neither I1 nor I2 is touched: no job composition changes, no identity changes, no new credential adjacency, no
--allowedToolschange, and theanthropics/claude-code-actionSHA stays on the repo-wide pin (9 occurrences, one SHA).The one allowlist that grows is
allowed_botsonfix, the job holdingAI_LOOP_PATandAI_LOOP_SSH_SIGNING_KEY(and onverify, which holds neither). The entry admits GitHub'sCopilotactor login, which two Copilot apps share, on every arm the gate can fire on, not only the review arm; beforefixruns, the gate still re-derives same-repoclaude/*head,ai-looplabel, PR open and the protected-path halt from live data, and that is the confinement rather than the precision of the login.Docs
The ai-development guide now says, next to
AI_LOOP_COPILOT, that the two lists have to agree and what happens when they do not — a reviewer in one but not the other either waits for an unrelated event or fails the actor check.Verification
pnpm check:conformance— 19/19.pnpm format:check— clean.login=copilot-pull-request-reviewer[bot] type=Bot.Summary by CodeRabbit
Bug Fixes
Documentation