Repository navigation
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1016 +/- ##
==========================================
- Coverage 67.16% 67.10% -0.06%
==========================================
Files 123 123
Lines 43675 43675
Branches 11153 11153
==========================================
- Hits 29334 29310 -24
- Misses 11204 11219 +15
- Partials 3137 3146 +9
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
85a0c5d to
7b20a03
Compare
There was a problem hiding this comment.
Pull request overview
Adds a GitHub Actions workflow that waits for the repository’s existing CI workflow to complete successfully on a PR targeting main, then requests a Copilot code review exactly once per PR via a dedup check.
Changes:
- Introduces a
workflow_runlistener gated onconclusion == successandevent == pull_request. - Resolves the PR from the workflow_run payload (with a fork fallback) and requests
copilot-pull-request-reviewer[bot]. - Avoids repeat requests by skipping if Copilot is already requested or has already reviewed.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
7b20a03 to
1f83571
Compare
1f83571 to
d1a8544
Compare
…rift (#1066) ## What Adds a `codecov.yml` (23 lines, no code change): - `coverage.status.project.default.threshold: 1%` - `coverage.status.patch.default.target: 80%` ## Why ARC has no Codecov configuration at all — `.coveragerc` configures coverage.py's *collection*, not Codecov's *status checks*. With no config, Codecov falls back to its built-in defaults, under which the `project` status is `target: auto` with a **0% threshold**: total coverage may never dip by any amount relative to the base commit. The consequence is that nearly every PR adding code covered below the repository average (~67%) reports a failing check while every test job, CodeQL and the functional suite are green. Observed on the current open set, all red on `codecov/project` alone: | PR | status | |---|---| | #653 | `67.03% (-0.10%)` → `67.05% (-0.08%)` after adding a genuine unit test | | #860, #1016, #1039, #1050, #1054, #1055, #1056, #1057 | red on `codecov/project`, everything else green | On #653, `codecov/patch` simultaneously reported *"Coverage not affected"* — the gate that actually judges whether the PR's new code is tested was already satisfied. A check that fails on one-tenth of a percentage point trains reviewers to ignore it, which costs us the patch signal too. ## What this does and does not change - **Does:** lets total coverage drift up to 1% without failing the status. - **Does:** keep an explicit `patch` target, so the coverage of a PR's own new or changed lines remains a gating signal. This is the stricter half of the config, not a loosening. - **Does not:** touch any test or source file, change `.coveragerc`, or alter what coverage is collected. - **Does not:** raise ARC's actual coverage. That is a separate and much larger piece of work. Note that Codecov re-evaluates statuses on new uploads, so already-open PRs will pick this up on their next CI run rather than retroactively.
d1a8544 to
3d5e56b
Compare
3d5e56b to
5289816
Compare
5289816 to
ee55285
Compare
ee55285 to
cdaf295
Compare
| steps: | ||
| - name: Request Copilot review (once, on first green) | ||
| env: | ||
| GH_TOKEN: ${{ github.token }} |
There was a problem hiding this comment.
I have one concern about using the default GITHUB_TOKEN to request a Copilot review.
I found a reported case where the API request succeeded, but Copilot was not actually assigned as a reviewer when the request came from github-actions[bot]. If that happens here, the workflow would show green without Copilot ever being added, and since the "only once" check never sees Copilot, it would repeat the request on every green CI run.
Could we confirm that this works in ARC, or consider using a PAT from a Copilot-enabled user?
Reference: arthur-debert/electron-splashguard#5
There was a problem hiding this comment.
Good catch, and the "repeats on every green CI run" half was real — but for a different reason than
the token, which I only found by trying to confirm it as you asked. Fixed in 46b89b222.
What I measured. I requested Copilot on this PR and then asked for the pending request back four
ways:
| view | sees the pending Copilot request? |
|---|---|
gh pr view N --json reviewRequests |
no — returns only [{"__typename":"User","login":"calvinp0"}] |
GET /pulls/N/requested_reviewers |
no — the payload has users and teams keys only |
the POST's own response body |
no — echoes a requested_reviewers array that omits it |
GraphQL reviewRequests.nodes[].requestedReviewer with ... on Bot{login} |
yes — copilot-pull-request-reviewer |
Copilot is a Bot, and none of the REST-backed views report bot reviewers. The dedup was built on
the first of those, so its "already a requested reviewer" arm could never see a pending Copilot
request: on this PR, with a request genuinely attached, that arm answers false where the GraphQL
arm answers true. Every green run until Copilot posted its first review would therefore re-request
— and since each request triggers a fresh pass, that is exactly the quota this workflow exists to
protect. So your conclusion was right and the mechanism was one layer down.
Two changes. The dedup now asks over GraphQL with ... on Bot / ... on User fragments, keeping
the existing guards (a team request carries no login, a deleted account's review carries a null
author; both are filtered so set -e can't kill the step before the request). And the request is
now read back after the POST: if Copilot did not attach, the step emits a ::warning naming the
PR instead of exiting 0. That is precisely the "workflow shows green without Copilot ever being
added" case you described — it can no longer pass silently.
On the token itself, honestly: partly confirmed, partly still open. The request mechanism works
on this repo — the timeline shows Copilot requested on #1050 and #1039 with reviews arriving the
same day, and my request here attached. What I could not test from outside is your actual scenario:
whether a request made by github-actions[bot] with the default GITHUB_TOKEN attaches, since that
only happens once the workflow runs on main. I'd rather not guess at it — the read-back above is
what will answer it, as a visible warning on the first green run rather than silence.
So I've left this thread open: if that first run does warn, the PAT-from-a-Copilot-enabled-user route
you suggest is the fix, and it's a one-line GH_TOKEN swap to a secret. Creating that secret is
@alongd's call, so I've flagged it for him rather than pre-emptively wiring in a secret that may turn
out to be unnecessary.
One aside worth knowing, since it bears on the dedup's second arm: Copilot has already reviewed this
PR three times, so in practice the reviews arm closes the window shortly after the first review
even on the old code — which is probably why this never showed up as visible spam.
There was a problem hiding this comment.
🔵 Needs a closer look
One active moderate issue remains: the fork-PR fallback resolves PRs with a free-text gh pr list --search on the commit SHA, which may return nothing, so Copilot could be silently skipped for fork PRs even though the PR description says they are handled.
No open issues 🎉
1 resolved since last review
Review effort: Lite
46b89b2 to
8b46171
Compare
|
Addressing the fork-PR lookup finding from the Copilot review on Fixed in pr=$(gh api "repos/$REPO/commits/$HEAD_SHA/pulls" \
--jq '[.[] | select(.state == "open" and .base.ref == "main") | .number] | first // empty')The finding doesn't reproduce as "returns nothing" — I checked both lookups against this PR's head Two caveats worth stating rather than implying:
|
8b46171 to
b7d7dd5
Compare
There was a problem hiding this comment.
🟡 Changes recommended
The main blocker is a moderate issue: the workflow's substring match on "copilot" would treat any login containing that text, such as a human account, as Copilot engagement and wrongly block the request, while the remaining findings are comment-only nits.
3 open issues
Review effort: Lite
b7d7dd5 to
c4ebc26
Compare
49daacf to
1195c52
Compare
|
My main question is that if Copilot reviews every PR that passes CI, who fits the bill for Copilot? If it's each individual PR owner, then what about situations (like my own experience) where the Copilot monthly usage allowance is used up. Or if a PR creator is not aware their Copilot monthly allowance (or credit card) will be used.Also, I think I for example get a monthly allowance because I am a student - but not every PR creator will be. I could be mistaken that this is being run automatically though |
Adds .github/workflows/copilot-review.yml. It listens on workflow_run for the
"CI" workflow completing successfully and, for a PR targeting main, requests
a Copilot code review exactly once — the first time CI is green. A dedup check
(skip if Copilot is already a requested reviewer or has already reviewed) keeps
it to a single pass rather than re-firing on every subsequent green push.
Both the dedup and the post-request confirmation ask over GraphQL, because
Copilot is a Bot and a pending Bot review request is invisible to the
REST-backed views: `gh pr view --json reviewRequests` and
`GET /pulls/N/requested_reviewers` report only `users` and `teams`. Measured on
this PR with a live pending Copilot request, the REST arm answers false where
the GraphQL arm answers true, so a REST dedup would re-request on every green
run until Copilot posted its first review — and each request triggers a fresh
pass, which is the quota this workflow exists to protect. For the same reason
the POST's 200 proves nothing on its own: it echoes a requested_reviewers array
that never lists a Bot, so the request is read back and a warning annotation is
emitted if Copilot did not actually attach, rather than leaving the run green
with no reviewer added.
The fork-PR fallback asks `GET /commits/{sha}/pulls` which pull requests the
run's commit belongs to, rather than searching for the SHA as free text with
`gh pr list --search`. That search goes through the eventually-consistent issue
search index, so a fork PR's commit need not be indexed when its CI finishes,
and it matches the SHA anywhere in a PR, so it can return an unrelated PR that
merely quotes the commit. The head-SHA guard catches the latter and skips, but
silently — leaving a fork PR with no review, which is what the fallback exists
to prevent.
A workflow-level concurrency group keyed on the run's head SHA makes the dedup
a safe check-then-act. Two completed CI runs for the same head -- re-running a
run, since ci.yml has no push trigger and a PR commit otherwise gets exactly
one run -- would otherwise both read "Copilot not engaged" before either
requested. Runs for different commits are not a race, as the head-SHA guard
retires the superseded one. cancel-in-progress stays false so the second run
still executes and no-ops on seeing the first one's request, rather than being
discarded along with that check.
Both checks match the reviewer login exactly rather than as a substring.
test("copilot") also matches a human account such as copilot-tester, and one
of those requested on or reviewing the PR would mark Copilot engaged and
suppress the request permanently. GraphQL returns the bare
copilot-pull-request-reviewer, so the optional [bot] suffix is there to keep
the REST spelling matching as well.
Chosen over a copilot_code_review branch ruleset, which can only fire on PR open
or on every push and cannot gate on CI passing — so it would review red branches
and burn Copilot's premium-request quota on them.
1195c52 to
a96d1b0
Compare
|
Good question to ask before this merges, and you're right to check rather than assume — but I think It is automatic — that is what the workflow does — and that is what keeps it off contributors'
This workflow requests the review as What the org does pay, from the same page, is AI credits per review (1 credit = $0.01; GitHub's own That is also why the dedup in this workflow matters more than it might look: it requests once, on Two honest caveats:
|
What
Adds
.github/workflows/copilot-review.yml: the first time a PR tomainpasses CI, it requests a Copilot code review — once.How
Listens on
workflow_runfor the CI workflow completing withconclusion == successon apull_request, resolves the PR (event payload for same-repo PRs; head-SHA lookup for forks), and addscopilot-pull-request-reviewer[bot]as a reviewer. A dedup check (skip if Copilot is already requested or has already reviewed) keeps it to one pass per PR — it does not re-fire on later green pushes.Why gated on green, not a ruleset
A
copilot_code_reviewbranch ruleset can only fire on PR open or on every push — it cannot wait for CI. Gating on green keeps Copilot off red branches and off the quota until the branch actually builds, and holds it to a single review.Notes
ci.ymlor any source/test.main; it takes effect on the next PR.permissions: pull-requests: write,contents: read.