Skip to main content
FB.

A green review is not a merge gate

A review signal matters only when its provider, commit SHA, coverage, execution state, publication, and place in branch policy are explicit.

Jump to summary
• 20 min read • Updated Oct 14, 2026
Florian Bruniaux

Written by Florian Bruniaux

AI Founding Engineer at Méthode Aristote, 13 years scaling engineering teams from developer to CTO. Builds open-source developer tools, see what else I've shipped.

TL;DR

I need a completed procedure, a known producer and evidence for the current revision before a review can influence merge. A green interface does not answer those questions, so I would make them explicit in the branch policy and its verification cases.

ObjectWhat it proves
Bot commentAn identity published text on a pull request
Workflow runA named automation reached its recorded conclusion
GitHub check or statusA context reported a state for a commit
Required checkBranch policy requires an accepted state before merge
Usable review proofProvider, SHA, scope, execution state, and publication all match

A pull request can look green without containing a completed code review. The interface compresses comments, workflow conclusions, check states, and branch rules into a few colors. Those objects have different producers and different authority.

If a team treats any green review-like signal as evidence that the change was inspected, an unavailable or stale reviewer disappears inside an otherwise successful delivery path. The repair starts by naming the objects and attaching every decision to the current commit.

4 green objects with different authority

A bot comment is published text. It may contain findings, a summary, a placeholder, a rate-limit notice, or an error. Its presence does not establish that the declared review procedure completed.

A workflow run is an automation execution. A successful conclusion means the workflow’s own steps satisfied its configured exit policy. A workflow can deliberately finish successfully after an optional reviewer times out.

A GitHub check or commit status is a named state associated with a commit. It can come from GitHub Actions or another integration. Its name and color do not reveal the full semantics of the producer.

A required check is selected by branch protection or a ruleset. It has merge authority because the branch policy requires an accepted state, not because its name contains “review.”

GitHub’s protected-branch documentation, checked on September 27, 2026, states that required status checks must be successful, skipped, or neutral before merging. Administrators can also bind a required check to the GitHub App expected to set it. In strict mode, the branch must be up to date before merge; loose mode evaluates the required check without that update requirement.

GitHub enforces the state and the expected source configured in branch policy. The repository owner has to ensure that the source’s accepted states mean what the team thinks they mean. A skipped or neutral state may satisfy a required-check rule while providing no completed review.

A perfect structural score can coexist with a failed check

On a pinned revision of the Walkinglabs harness-engineering course, I reproduced 3 cases on September 26, 2026. The evaluation record lists the fixtures and runtimes. A generated repository whose verification command was false received 100/100 from the structural scorer. A separate shell fixture passed 7/7 critical checks and scored 9/71 overall, because the 2 tools count different things. In the course’s maker-checker lecture code, which is labelled a placeholder, the checker reads approval with "approved" in review, so a review saying not approved passes. No real merge was performed.

The course is teaching material, and these cases say nothing about how often such defects occur elsewhere. They show why the consumer of a verdict needs its own failure tests. A file inventory answers whether expected components exist. Acceptance still needs an executed result, the relevant revision, a recognized producer and the authority assigned by repository policy.

Functional success can still miss a review constraint

SWE-Gate derives constraints from real pull-request review comments and synthesizes 303 repair tasks across 75 Python repositories, each with separate functional and constraint tests. A gate needs the same split, testing both the bug fix and the compatibility or error behavior a reviewer would require. The benchmark result is not a failure-rate estimate for production PRs.

A review proof needs 5 attributes

Before using an automated review in a merge decision, establish:

  1. Provider identity. Which GitHub App, workflow, CLI, or human produced the record?
  2. Commit SHA. Does the review cover the current head, not an earlier revision?
  3. Scope and coverage. Which files, batches, or rules were actually evaluated, and which were excluded?
  4. Execution state. Did the procedure complete, fail, abort, time out, or end unknown?
  5. Output publication. Were the normalized findings or explicit clear outcome attached to the pull request or retained artifact?

All 5 are required for a usable review record. None proves that the code is correct. They prove only that a named procedure completed on a named revision within a declared boundary and exposed its result.

Bind the result before the review starts

Consider an illustrative race: the controller dispatches a review of revision R1, a new push creates R2, then the reviewer finishes. Its result still covers R1. Reading the current head at completion and labelling the result R2 would invent coverage the reviewer never had.

At dispatch, freeze the repository, base and head revisions, reviewer identity, and the versions or digests of the procedure and loaded configuration. Retain those bindings with the output. Check the current revision again when consuming the record; do not rewrite the original binding. Keep the R1 result as history and collect evidence for R2. Reuse only what an explicit assessment establishes was unaffected by the change.

The same rule applies when a reviewer configuration changes while a run is active. A new prompt version does not retroactively describe the procedure that produced the old result.

A generic branch policy can leave review advisory

Suppose an illustrative repository requires tests and secret-scan before merge. A reviewer configuration file is also present, but its result is not required by branch policy. Successful required checks can satisfy that policy while reviewer execution remains UNKNOWN. The configuration file alone establishes neither installation nor execution nor merge authority.

Inspect the effective branch policy and an actual completed run separately. A comment saying that review was skipped cannot substitute for the latter. This example describes a possible configuration, not the audited state of a customer repository or a claim about a vendor’s current behavior.

Execution, outcome, and merge effect are separate columns

A workflow can choose to remain green when an advisory reviewer is unavailable. That design can be reasonable, since a service outage need not stop every low-risk change. The mistake is publishing the degraded path as if the review had completed.

For reviewer rows, the table reuses the execution states, outcomes, and stop causes defined in part 3. The deterministic-test row describes a separate check result: a test can finish executing and fail its assertion without producing a reviewer outcome.

EventExecution stateOutcomeMerge effect
Required deterministic test failsCOMPLETED (the test ran)Not applicable; test assertion failedBlock
Reviewer completes with blocking findingsCOMPLETEDFINDINGSHuman disposition required
Advisory reviewer times outTIMEOUTNoneContinue only under the declared degraded policy
Reviewer is rate limitedFAILED, stop cause UNAVAILABLENoneContinue only under the declared degraded policy
Output not bound to the current headUNKNOWNNoneNot review evidence
Reviewer completes with no findingsCOMPLETEDCLEARRecord completion; infer nothing about correctness

Skipped and neutral check states need the same scrutiny. GitHub accepts both for a required check, and neither proves that a review ran.

The next action falls into 3 groups. Decision possible means that the required evidence is usable and findings have the dispositions demanded by policy; it does not mean automatic merge. Correction required means a confirmed defect needs a change and another evidence assessment. Evidence incomplete means the controller must recover missing evidence or escalate. It must not ask the author to repair code merely because a review timed out.

The status shown to users should preserve both review state and merge effect. Advisory reviewer unavailable; deterministic checks passed is accurate. Review passed is not.

The repository measured in part 7 follows this pattern. Its AI review check stays green by design, its summary states that the review is advisory and never decides mergeability, and a warning names an unavailable or incomplete review, while deterministic CI alone decides.

Fail closed in proportion to the evidence

Deterministic controls with stable failure semantics are strong candidates for required checks. Tests, type checks, secret scanners, and policy validators can block when their explicit assertions fail or their output is invalid.

An LLM reviewer has different failure modes. It may emit false positives, omit defects, disagree with another reviewer, or be unable to inspect required context. Making every model finding an automatic hard stop can transfer authority to an uncalibrated classifier and invite bypasses when noise rises.

Use a 2-stage decision:

  1. The reviewer publishes structured findings and an explicit execution state for the current head.
  2. A deterministic policy checks whether the run is usable and whether every finding in a configured blocking class has a recorded disposition from an authorized human.

In this split, the model supplies evidence, the policy checks that the evidence is complete, and a person owns ambiguity, risk acceptance, and any irreversible decision.

If review availability is mandatory for a risk class, make every execution state other than COMPLETED block that class. If the reviewer is advisory, allow the merge path to continue while showing the degraded state. Never report unavailability as a successful check.

A review started on R1 remains bound to R1 after a push creates R2. Current evidence leads to a possible authorized decision, correction, or evidence recovery.
Readiness allows the policy and authorized decision to proceed. Confirmed defects need correction; missing evidence needs recovery or escalation. A new revision requires a new evidence assessment. View the full-size diagram.

Verify the combination that will be integrated

A current-head review is necessary evidence for that PR, but it does not cover every interaction with other changes. Record the base revision, merge base, candidate integration revision and exact set of PR revisions tested together. When the base or that set changes, invalidate the previous integration result and re-run the affected checks and risk classification. Reusing an individual review requires an explicit assessment of what remains unaffected.

In a constructed example, one PR changes a data producer and another changes its consumer. Both pass against the old base while their combination disagrees on a field’s meaning. Testing a temporary combined branch covers that recorded state; merging the PRs separately later can produce a different state.

Plan-execute: running 20+ agents on a migration illustrates this rule with 24 agent branches. Its migration is a constructed example, not a recorded run: the coordinator merges ready branches one at a time and reruns the quality gate after each merge, because a branch that passed alone can still break the build once the others have landed.

An integration check can also have narrower semantics than its name suggests. CodeRabbit’s GitHub documentation, checked on October 14, 2026, says it publishes a check for the synthetic merge-group commit without rerunning the review. That check therefore does not demonstrate that the combined change received a fresh model review. Inspect the integration tests and review coverage separately.

GitHub merge queues run required checks on a temporary merge group that includes the target branch and queued changes. CI must handle that group, including the merge_group event when using GitHub Actions. A green check for a PR alone does not establish that integration coverage. These checks still cover only their encoded properties; ownership of sensitive behavior remains part of the review policy.

Test the gate in both directions

An inspectable adjacent example is Agents Shipgate, whose documentation, checked on October 14, 2026, limits its verdict to statically discoverable agent capability changes. Its advisory mode can exit successfully while reporting a blocked change; blocking CI requires an explicit policy. The published release also distinguishes constructed fixtures from its incomplete real-history qualification. It illustrates why the report, exit code and installed merge rule need separate checks. It does not establish runtime permissions or the correctness of an ordinary application PR, and I have not run it for this article.

François Zaninotto’s survey of 246 harness repositories for Marmelab recommends testing both the cases where a behavior should occur and those where it should not. For this review policy, the following are proposed fixtures, not recorded GitHub results:

FixtureExpected policy decision
Expected producer, current revision, completed review, no unresolved blocking findingPermit the review-policy step when its other requirements are satisfied
Same record attached to the previous headReject it as evidence for the current head
Accepted status from an unexpected producerReject it as trusted review evidence
Required reviewer unavailable or output malformedBlock the required-review path
Advisory reviewer unavailablePreserve the degraded state and apply the declared advisory policy

Run the permitted fixture as well as the rejected ones. A policy that blocks every change can pass a negative-only test suite. Check the configured branch decision separately from the parser: a correct local decision does not prove GitHub requires it.

Verify the current head before merge

Run this checklist after the final push:

  • Record the repository, base SHA, and current head SHA
  • List every required check and its expected GitHub App source
  • Confirm each required result is attached to the current head
  • Distinguish successful, skipped, neutral, failed, cancelled, and timed-out conclusions
  • For every AI reviewer, verify provider identity, covered SHA, declared scope, execution state, and published output
  • Mark missing or stale review evidence UNKNOWN
  • Confirm all blocking findings have a disposition and named owner
  • Record who or which policy has merge authority
  • Re-run the checklist if the head changes

A screenshot of green checks is not enough. Retain the API response or audit artifact that binds names, sources, conclusions, and SHAs to the branch rule inspected at that time.

Not yet demonstrated: apart from the advisory check observed in part 7, the examples above are constructed. Before claiming that this gate works in a repository, retain a dated check graph, the effective branch rule, a completed review bound to its inputs, and permitted and rejected integration fixtures, including a push during review.

Part 5, Turn recurring review comments into tested controls, addresses a different failure: review feedback can be valid and timely yet recur because no durable control absorbs it.


YSNK

(You should now know)

  • A comment, workflow run, check, and required check are different objects
  • Usable review evidence needs provider, SHA, scope, execution state, and publication
  • A review of an earlier revision remains historical evidence after a new push
  • Advisory failure should remain visible without masquerading as review success
  • Every merge decision must be recomputed for the current head