AI code review in CI - a PR job, not ghost text (June 2024)

Whether AI is allowed to speak on a PR

It is 15 June. In April last year I kept a notebook of Copilot suggestions in two test projects for two weeks. The result was 41 multiline suggestions: 12 accepted unchanged, 22 edited before the commit, and 7 rejected after I read them. I wrote a sentence then that now reads like an expiry date: that was the whole of my “AI review”, not a bot commenting on a diff on GitHub, but discipline when accepting suggestions. That review happens in the buffer before anything reaches a commit, and I am the only person who sees it.

That post ended with a promise that I would describe the things announced on 22 March 2023 on my own stack and with my own numbers once I had access to them. Pull request summaries did not reach me because they went into a plan I do not have, and I return to that in the next section. Something that was absent from that announcement did arrive: bots that read a diff from a workflow and leave a comment under the pull request. I have my own material for those today: one synthetic pull request, four observations from a bot, and my decision on each one.

That shifted the question. A year ago it was: should I accept the gray text in front of the cursor? Today it is: should a model be allowed to speak on a pull request before I click merge, and if so, in what role? There are three roles, and this entire post depends on distinguishing them. A voice is a comment or label that someone will read. A veto is a required status that blocks the merge when its result is red. A signature is an approval that counts toward the required reviews.

My thesis for June 2024 is narrow, and I will defend it to the end. “AI review” in my QA is a job or a card on the PR: a Copilot Enterprise summary or a bot such as CodeRabbit or PR-Agent, with PR-Agent running explicitly as a job in Actions. It is not the ghost text from April 2023 because the comment sits on the pull request and the whole team can see it, rather than only the person with the buffer open. The model comments on the diff and gets a voice. It gets neither a veto nor a signature. A human remains the required reviewer and the only person who clicks merge.

Here is the workshop state so that a year from now it will be clear what I mean. The runner is @playwright/test version 1.44.1 from 23 May, a patch for 1.44.0 from 6 May that closed four regressions. Alongside the TypeScript specs, I keep a C# API test project using NUnit in the repository with examples for this blog. I pay 10 dollars a month for an individual Copilot subscription, the same one I have had since June 2022, and I have had chat in stable VS Code since 29 December, when it became generally available. In May last year the same chat required Insiders, the Nightly extension, and a place on the waitlist.

The rest of the background fits into a few sentences. Copilot Enterprise has been generally available since 27 February, and it is not on my account. Code scanning autofix has been in public beta since 20 March. CodeRabbit and PR-Agent have been operating since July last year. Copilot Workspace has been in technical preview behind a waitlist since 29 April and adds nothing to this post because it is a path from an issue to a change, not a pull request reviewer. A bot can use GPT-4o from 13 May or Claude 3 from 4 March through an API, and neither is a June novelty. The Cursor editor is still not my daily tool, and it would sit on the other side of the boundary in this post anyway because it is an IDE channel.

What will not be here. There will be no sentence claiming that Copilot reviews pull requests as a required reviewer because I have no such feature to enable and have seen it in no changelog. There will be no description of autofix as a generally available product because it is a beta. There will be no rewritten checklist from April 2023 because it still applies in the buffer and I have nothing to change in it. There will be no diff from a client’s pull request or log from a company pipeline. There will be a synthetic pr-agent.yml workflow, a synthetic pull request with a Playwright spec and an NUnit test, an excerpt from the bot’s comments, one accepted observation, and three strikeouts.

What is on github.com without a third-party bot

I will start with what GitHub itself provides without installing anything alongside it because this is where conversations most often go wrong.

On 27 February Copilot Enterprise became generally available with three features: chat on github.com, knowledge bases, and pull request summaries. It costs 39 dollars per user per month and is a plan for organizations on GitHub Enterprise Cloud. My 10-dollar subscription includes none of the three. The sentence “Copilot reviews my PRs” therefore fuses two price lists into one and is true for neither, because none of these features is a reviewer in Enterprise either.

The changelog from 27 February says so directly, although you have to read to the end of the sentence. The item about pull requests promises faster review: a summary that has been more structured since 27 February, with a “Summary” section giving the overall picture and an “Outline” section walking through the code, plus chat that explains the diff. Its stated purpose is to help the reviewer get oriented faster and leave more time for their own observations. This describes a tool for the reviewer, not a reviewer.

These two features differ in their channel, and that matters more to me than their quality. The summary is generated on demand from the description field and lands in the pull request description, where the author can edit or remove it. This is the card on the PR from my thesis: text that everyone who opens the pull request can see. Chat about the diff works differently. Someone has to open it and ask a question, and the answer goes to the person asking, not to the team. From the process perspective, this is still one person’s conversation, only moved from the editor to the website.

Neither the summary nor the chat has a status in the checks list. Neither can be added to branch protection as required, there is nothing to turn red, and there is nothing to block. The February changelog has no reviewer. It has an author of a draft description and an on-demand conversation partner. I do not treat this as a criticism of the product, only as its exact description, and that description determines where these features can stand in my process.

I do not need access to make that decision because it does not depend on the quality of the text. A year ago, while waiting for Copilot for PRs, I wrote that a generated pull request description is a description, not a verdict: a summary answers “what changed”, while a review answers “is it safe and will the tests catch it”. I would read an Enterprise summary exactly like a table of contents for the diff, one that saves a minute of orientation and does not remove a single file from my reading list. The risk also remains the same one I described then: a polished summary at the top of a pull request tempts me to read the summary instead of the diff.

A job in Actions: bots that already exist

The second channel is older than Enterprise and does not depend on a GitHub plan.

A pipeline means three specific things here: a token, a workflow, and a comment under the diff. It does not mean a window in VS Code or a panel on a website.

CodeRabbit launched on Product Hunt on 10 July 2023 as a SaaS service that is installed on a repository and comments on pull requests. CodiumAI announced PR-Agent as an open source project on 24 July 2023. PR-Agent runs as a GitHub Action in my own workflow and also responds to commands entered in a comment, such as /review and /describe. The difference between them that interests me is not the quality of the comment because I did not compare it. It is the path the diff takes. With SaaS, the diff goes out to the vendor’s service. With Actions, it goes straight from my runner to the model API, using my key. I use PR-Agent in this demo for one reason: the whole run fits in a YAML file that I can show and in a secret that I control myself.

That same path is why I install neither bot silently on a client’s repository. A client diff sent to someone else’s API is the same kind of decision as a secret pasted into a chat window, and in my note about chat from May 2023 I wrote that such a decision is not made in silence. Conversation and consent come first, then the workflow.

My synthetic job, without a real token or an organization name, looks like this:

# .github/workflows/pr-agent.yml
name: pr-agent

on:
  pull_request:
    types: [opened, reopened, ready_for_review]
  issue_comment:

jobs:
  pr-agent:
    if: ${{ github.event.sender.type != 'Bot' }}
    runs-on: ubuntu-22.04
    permissions:
      issues: write
      pull-requests: write
      contents: read
    steps:
      # the tag pins a one-line Dockerfile, not the bot code
      - uses: Codium-ai/pr-agent@v0.22
        env:
          OPENAI_KEY: ${{ secrets.OPENAI_KEY }}
          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}

The skeleton comes from the PR-Agent documentation. I left the triggers and the sender condition unchanged. The job starts on a new, reopened, or newly ready-for-review pull request and on every comment that may contain a command, while the sender condition prevents the bot from answering itself. I changed three things. First, permissions: the documentation gives the action contents: write, while I give it read because this job is meant to comment, not commit, and the GitHub App variant in the same documentation requests read-only access to repository contents. Second, the runner: I entered ubuntu-22.04 instead of ubuntu-latest, following the same habit behind my note about parallel tests that ubuntu-latest is an alias that will one day move without my involvement. Third, a tag instead of @main, and this change needs its own paragraph because it does not do what it promises.

First, though, what the bot does after it starts. On a new pull request, the action runs three tools by default: change description, review, and code suggestions. The description tool adds a summary and change walkthrough to the author’s description, preserves the author’s title, and adds a change-type label. The review in the configuration from 13 June has fixed fields. It estimates review effort on a scale from one to five, answers yes or no to whether the pull request adds or updates appropriate tests, assesses security risk, and lists key places to inspect. It also adds a label with the estimated effort and a second one when there is a security observation. Automatic approval is disabled by default, and I leave it that way. After the next push, the bot stays silent until I write /review in a comment.

Now the tag. I found the trap by reading action.yaml, not the README. The action is a Docker image built from a file containing one line: FROM codiumai/pr-agent:github_action. That is a moving tag on Docker Hub. Pinning uses: to May’s v0.22 therefore pins this one-line file, not the code that reads my diff. The model is not pinned on my side either. In configuration.toml from 13 June, the default is gpt-4-turbo-2024-04-09, and that value changes with the image rather than with my repository. The bot in this demo uses an OpenAI key. The configuration could just as well specify Claude 3 or GPT-4o, and neither change would affect a single sentence in this post because these are sentences about permissions, not intelligence.

This leads to something more important than the YAML itself. This job is nondeterministic by nature because a language model responds, and unpinned by construction because its code and default model arrive in a moving image. In September 2022 I wrote that I admit only fast and deterministic checks into the pull request basket, and that a required check must be small and stable. The bot fails that condition twice before writing its first sentence. That does not bother me for a comment. It disqualifies the bot from being a gate.

Autofix is not a review of tests

The third channel also leaves a suggestion on a pull request, which is exactly why it needs to be separated from the first two.

Since 20 March, code scanning autofix has been in public beta for GitHub Advanced Security customers. When CodeQL reports an alert on a pull request, autofix proposes a correction generated with help from Copilot, includes an explanation, and leaves a human to accept, edit, or reject it. The announcement from 20 March lists four languages: JavaScript, TypeScript, Java, and Python. This is a beta, not a generally available product, and I will not guess when that changes.

The place for this feature is clear: security. It is a neighbour of November’s security testing basics, not a reviewer of the test suite. In November I wrote that, in my stack, injection is addressed mainly by an ORM with parameterized queries and by code review. Autofix sits in the same place in the process: beside a static analysis alert, as a suggested patch to read. I read it like any other diff, with one additional precaution. It is easier to rush here because the suggestion arrives with the stamp of a security tool.

It is not a review of tests, and this is the boundary I want recorded. CodeQL looks for vulnerabilities, not for the meaning of an assertion. A Playwright spec that asserts on a heading visible on every page checks very little, but it is not a vulnerability and will not generate an alert, so autofix has nothing to correct. An NUnit test that logs in with one account shared across the whole project is not a vulnerability either. The beta will not assess whether expect(login.error).toHaveText(...) checks anything because no one asked it that question. A reviewer asks it, and in June 2024 my reviewer is a human, with the bot from the previous section acting as an advisory voice.

How I wire it in without lying about the gate

The rule fits into two sentences. AI gets the right to a comment and a label. Tests and human approval remain required, exactly as they were before the bot appeared.

That sounds obvious, so I will show why it is not with one synthetic pull request from the repository containing examples for this blog. It has not one line from a client project. Its title is “Wrong password: message in the UI and 401 from the API”, and the changes are in three files. The page object e2e/pages/LoginPage.ts gets an error locator based on the alert role. The spec e2e/auth/login.spec.ts gets a wrong-password scenario using an account from the user fixture, which creates it before the test and deletes it afterwards:

test('złe hasło zostawia na formularzu z komunikatem', async ({ page, user }) => {
  const login = new LoginPage(page)

  await login.open()
  await login.signIn(user.email, 'zle-haslo')

  await expect(login.error).toHaveText('Nieprawidłowe hasło')
  await expect(login.submit).toBeVisible()
})

The API project gets an NUnit test for the same behaviour one level lower in api-tests/Auth/LoginTests.cs:

[Test]
public async Task Login_WithWrongPassword_Returns401()
{
    var response = await _api.LoginAsync(TestUsers.Default.Email, "zle-haslo");

    Assert.That(response.StatusCode, Is.EqualTo(HttpStatusCode.Unauthorized));
}

The bot responded with two comments: a review and code suggestions. I have extracted the parts about the tests, without the HTML table or emoji:

PR Reviewer Guide

Estimated effort to review [1-5]: 2
Relevant tests: No
Security concerns: No
Key issues to review:
  Possible flakiness: LoginTests.cs logs in with the shared
  TestUsers.Default account, so tests running in parallel may
  interfere with each other. Consider adding [Retry(3)].

PR Code Suggestions

e2e/auth/login.spec.ts: toHaveText('Nieprawidłowe hasło') couples
the test to UI copy. Use toBeVisible() on login.error to make the
test less brittle.

I count four observations in this excerpt because the paragraph about instability contains two: a diagnosis and a remedy. I accepted one and struck out three. I will start with the one I accepted because being fair to the bot is also part of the review.

Accepted: the shared account. The diagnosis is correct. TestUsers.Default is one account for the whole API project, and in the synthetic application a wrong password changes account state because the application counts failed attempts. With parallel classes, this test increments the counter on the same account used by other tests to log in. In October 2021 I called the shared database killer number three and wrote that two test slices relying on the same test account would delete data from under each other. Two classes running in parallel in one process do exactly the same thing. Locally, on one thread, the test passes and the runner reports nothing. The correction is a separate user for each test, created during setup and deleted after the test, exactly as the user fixture does in the neighbouring spec. A year ago I was adding the same pattern to Copilot suggestions by hand because Copilot did not know about my cleanup.

First strikeout: “Relevant tests: No”. The pull request consists of a page object and two files containing tests. The bot has a Playwright spec and an NUnit test in the diff, yet answers no to the question of whether the pull request adds appropriate tests. At the same time, the same bot marked the pull request with a Tests change-type label in its description. I do not know where the answer in the review came from. I suspect the model treated the page object as production code and the specs as something beside it, but that is a guess and I will not sell it as a diagnosis. The check took five seconds because I only had to look at the list of changed files. Something else is dangerous here. The field looks like a fact, not an opinion, and has a shape that makes automation easy to attach: a “no tests” label, a Slack reminder, or, in the worst case, a status. A field that is wrong on a pull request made entirely of tests teaches the team not to read it, and on the day it is right no one will look at it anymore.

Second strikeout: [Retry(3)]. This continues the accepted observation and best shows what this channel is. In one paragraph, the bot gave me the right diagnosis and the wrong remedy. A retry does not remove shared state. It only turns the one signal of that state into a green result after the second attempt. Retrying is not isolation, I wrote when discussing test organization in xUnit and NUnit, and in September 2022 I added the gate version: a test that needs three attempts to fit into the gate simply does not belong there. To the author of a change who wants a green pull request before the end of the day, the suggestion will sound reasonable. That is why it is struck out by someone responsible for gate policy, not by someone trying to close the task.

Third strikeout: toBeVisible() instead of toHaveText(). I consider this the most dangerous of the three proposals because it is presented as concern for quality. Here, “a less brittle test” means a test that passes for every message in the container with the alert role: for “Konto zablokowane”, for “Błąd serwera”, and for any other text someone may put there one day. The acceptance criterion says that a wrong password leaves the user on the form and displays a message reading “Nieprawidłowe hasło”, so the content is a requirement, not decoration. If someone changes that text, the test should fail and someone should decide whether the change was intentional. In the April notebook from last year, the most common reason for editing or rejecting a suggestion was an assertion that was too broad or always true: nine instances among the 29 edited and rejected suggestions. A year ago ghost text suggested it to me. Today a reviewer suggests it in the name of stability. The first question on my checklist is “can this test fail”, and the bot proposed a change after which it would fail less often than it should.

The most important observation from this pull request is not about the content, but the colour. If I accepted all three struck-out observations, the pipeline would remain green. [Retry(3)] compiles without complaint, toBeVisible() passes against the current application, and the “Relevant tests” field blocks no one. A year ago, the compiler or the first run caught some bad suggestions: the compiler immediately rejected an attribute from a different framework version, while the first run exposed a selector that did not exist in the application. Bad advice from a reviewer does not turn anything red. It produces a green suite that checks less. No runner will catch that, which is why approval remains with a human who knows the acceptance criterion.

The wiring follows directly from these three strikeouts. I work alone in the examples repository, so I am both the reviewer who cannot be skipped and the only person who merges. I do not require approval there because GitHub does not allow an author to approve their own pull request. In a team repository, the same rule takes the form of four settings:

• Protection for main requires a pull request and one approval from a human, while a new commit dismisses the previous approval.
• Required statuses are test jobs, which are what I called the pull request gate in September 2022. The pr-agent job is not on that list.
• In the Actions settings, “Allow GitHub Actions to create and approve pull requests” remains disabled. GitHub added it in 2022 specifically to prevent Actions from putting a change into a protected branch without review by another person.
• Bot labels are information. No automation listens to them, neither auto-merge nor reviewer assignment.

I treat one side effect of this arrangement as an argument, not a cost. When the model API has a bad day or the key reaches its limit, the pr-agent job turns red and no one waits. A year ago I wrote that a summary wired in as a required check would give me my first pipeline failure on the day someone else’s API had a bad day. One decision from the list above switches off that scenario.

I add one habit that is not a setting. In the pull request thread, I reply with one line to every bot observation: accepted with a commit, or struck out with a reason. Here that produced one “accepted” line and three “struck out” lines. It costs a minute, and the next reviewer does not have to guess whether anyone read the comment at all.

At the end of this section, I return to the 2023 summary. In December I wrote that my CI had no “LLM” job and listed four conditions by which I recognize that a tool has entered the team rather than the hype: a named task, a place in the process, a row in the pipeline or a conscious reason for its absence, and a human responsible for the result. The bot in this post is the first model to receive its own job in the examples repository. Its task is named because it comments on the diff and drafts a description. It has a place in the process, before human review rather than instead of it. It has a row in the pipeline, without the right to act as a gate. It has an owner because someone accepts or strikes out every observation and signs their name to that decision in the thread. Four out of four, provided the third point is read precisely: a row in the pipeline is not the same thing as a place on the list of required checks.

Summary

In June 2024, review in CI is a new channel, not a new reviewer. A model’s comment sits where the team can see it, on the pull request rather than in one person’s buffer. Ownership of the merge remains human. Copilot Workspace, in technical preview since 29 April, is not a pull request reviewer and changes nothing in this picture.

Three sentences I want to be able to read a year from now. Since 27 February, Copilot Enterprise has provided a pull request summary and chat about the diff, both on demand, both without a status in the checks list, and both outside my 10-dollar subscription. PR-Agent in Actions, pinned to a v0.22 tag that pins a one-line Dockerfile rather than the bot code, left four observations on a synthetic pull request, of which I accepted one and struck out three: “Relevant tests: No”, [Retry(3)], and toBeVisible(), each of which would have left the pipeline green. Code scanning autofix has been a CodeQL beta since 20 March, belongs to security, and does not review tests.

What does not follow from this. It does not mean that I will never make a bot a required status because a narrow, pinned, and repeatable rule may one day deserve that place. Approval and merge will remain with a human regardless. It does not mean that I am going back to Tab alone because the checklist from April 2023 still works in the buffer, while a bot on a pull request is a second channel rather than a replacement for the first. It also does not mean that Copilot reviews pull requests as a separate product because I have no such feature to enable.

In December I wanted the answer to “where is AI in this process” to remain expressible in one sentence and for that sentence to contain the verb “review”. In June it reads like this: a model comments on the diff in a separate, non-required job, while I review the diff together with its comment and click merge myself.

This is how I state the condition that will tell me the subject has matured. A bot can receive a required status on the day its code and model are pinned to versions, two runs on the same pull request produce the same observations, it does not claim that tests are missing when the spec is in the diff, and it never proposes a retry or a weaker assertion as a way to achieve stability. Even then it will get a veto, not a signature. Until that day, its comment is an advisory voice, and I sign the merge.