Skip to content

Automatic review answered cannot tell "reviewed, nothing found" from "never reviewed" — a quota refusal has stopped every core merge since 11:39Z #4730

Closed

Description

@rbuergi

Automatic review answered cannot distinguish "Copilot reviewed and found nothing" from "Copilot never ran", and today the second state has held every core merge since 11:39Z.

Measured

Six pull requests are green on the required test context and blocked on this gate alone:

PR Consolidate test results Automatic review answered
#4719, #4720, #4721, #4723, #4727, #4728 success failure

copilot-pull-request-reviewer returns failure with an empty output.title and output.summary — the quota refusal. Measured conclusions today, all failure:

#4719  11:38:59 → 11:39:36
#4720  11:43:17 → 11:43:54
#4725  15:36:24 → 15:37:07
#4728  15:48:35 → 15:49:20

Still refusing 4 h 10 min after the first one, so this is not a blip that clears on its own within a working session.

The gate's own log is correct and explicit: "review … is a refusal: Copilot was unable to review … reached their quota limit", threads opened by the automatic reviewer: 0, answered by a person: 0, and the remedy is "a maintainer re-requests the review, or applies the review-waived label — never an agent, and never automatically."

The gate is behaving correctly and failing in the safe direction. #4575 built it to close #4299, where three PRs merged past valid findings; that reasoning still holds and nothing here argues for weakening it. The only core PR that landed today is #4717, whose review completed before the quota ran out.

The gap

0 unanswered threads is produced by two states that are opposites:

  1. Copilot reviewed and raised nothing — the PR is as reviewed as it will ever be.
  2. Copilot never reviewed — the PR is entirely unreviewed.

The check-run conclusion distinguishes them, but the remedy does not: both end at a human, and in state 2 there is nothing for an agent or a person to answer — re-requesting only works once quota exists again. So an exhausted quota converts a review gate into an unconditional stop on every open PR, with no signal that says how long or what to do besides wait.

What would close this

Not a waiver path for agents, and not a raised bound. The two asks are:

  • Name the state. When the reviewer refuses for quota/availability, say so in the gate's summary and in the PR, distinctly from "findings unanswered" — so a reader sees "unreviewable right now" rather than "someone must answer findings" that do not exist.
  • Decide the policy for a structurally unavailable reviewer. Options worth weighing: hold as today (accepting that core stops until quota returns); auto-retry on a schedule once quota resets and let the existing gate re-evaluate; or a maintainer-set, time-boxed review-waived that is recorded on the PR. This is a policy call, not an agent's to take — the point of this issue is that it is currently taken by default and invisibly.

Cost today

Core merges stopped from 11:39Z. Six PRs, all green on tests, all with auto-merge armed, waiting on a human who has to notice. Plugins is unaffected (different reviewer path).

Filed from a session that hit this while draining the PR backlog; I did not re-request, waive, or use --admin.

Activity

  1. rbuergi commented on Sep 18, 2026

    @rbuergi
    ContributorAuthor

    Update: the refusal names a monthly quota — and it cleared ~5 hours later

    The verbatim refusal, from core PR #4719, workflow "Running Copilot Code Review", run 35340664292, job 105585581365 (copilot-pull-request-reviewer), read over REST at ~11:45Z:

    Error successfully reported to sweagentd session.
    Error successfully reported to sweagentd backend.
    Dt [SessionModelError]: You have exceeded your monthly quota (Request ID: B426:27CEB7:AA7A683:BC91137:6AAD22F3)
      errorType: 'quota',
      [cause]: [Error: You have exceeded your monthly quota (…)]
    ##[error]Process completed with exit code 1.
    

    So "monthly" is the service's own word, not an inference. What is not established: whose quota it is (org seat pool vs. the reviewer's own plan) and when the month rolls. Stating only what the log says.

    It cleared, which makes the reporting gap sharper rather than moot

    Measured 16:46Z — every copilot-pull-request-reviewer run on the six blocked PRs succeeded within four minutes of each other:

    #4725  success  16:32:29        #4721  success  16:33:04
    #4719  success  16:32:56        #4723  success  16:34:09
    #4720  success  16:35:20        #4728  success  16:35:59
    

    after refusing continuously from 11:38:59Z to 15:49:20Z. Roughly a five-hour outage, self-resolving, with no signal at any point saying whether it would last minutes or to the end of the month.

    And the aftermath is the actual cost. Two of the six — #4721 and #4725 — were still red with zero findings, because their Automatic review answered runs (12:49Z and 15:36Z) predate their Copilot success. The gate had correctly recorded "no review has landed" and nothing re-evaluated it once one did. Their red was stale and a re-run cleared it. The other four were genuinely red on real unanswered findings.

    So a reader at 16:45 saw six identically-failure gates with three different causes behind them — quota refusal, stale verdict, genuine findings — and the only way to tell them apart was to compare each gate run's timestamp against its Copilot run's. That is the concrete shape of what this issue asks for.

    What this adds to the ask

    • check-review-answered.py already has the case, named "quota refusal is not a review (Fix teardown SIGSEGV (#613): cascade hosted-hub creation freeze through the subtree #645 body) expected=red: has not landed" — so the refusal is recognised internally and simply has nowhere to surface. The state is known; it is not reported.
    • A stale red needs to re-evaluate itself. A gate whose verdict was "no review has landed" should not stay authoritative after one lands. Re-running by hand works and is legitimate, but it requires a human to notice — which is the same dependency this issue is about.
    • Distinguishing "unreviewable right now" from "findings await your reply" in the gate's summary would have made all six readable at a glance, with no timestamp archaeology.

    Quota log quoted from a peer session that read it directly; the conclusion timestamps above are my own measurement.

  2. rbuergi commented on Sep 18, 2026

    @rbuergi
    ContributorAuthor

    The decisive piece of context: this gate was shipped deliberately non-required

    Measured just now:

    ruleset 2128472  "main pr protection"  active
      required: Consolidate test results
      required: Automatic review answered
      updated_at = 2026-09-17T21:15:18+02:00
    

    AGENTS.md independently records the flip: the second context "was advisory as recently as 2026-09-11", and the stale singular in that file later cost a session ~45 minutes, with three PRs sitting green, armed and silently outside the merge queue.

    The gate landed yesterday as #4575 (closing #4299, option B) after a measured run of PRs merging past unanswered findings — the worst being #4310, which merged with 14 unanswered and stopped Plugins publication for six hours. Per the session that built it, it shipped non-required on purpose; it was made a required context overnight, by someone else.

    That reframes this issue. The gate is doing exactly what it was designed to do — red until a review lands, by design, precisely so an outage cannot read as "reviewed." That behaviour is correct and should not be weakened. What changed is that it became load-bearing while keeping a design tuned for advisory use: as an advisory check, "red until a review lands" costs a reader a glance; as a required context, the same behaviour converts any reviewer outage into a hard fleet-wide stop on every open PR, with no signal saying whether to wait minutes or a month.

    So the ask here is not "make the gate more permissive". It is:

    1. Name the state — a quota/availability refusal should read differently from "findings await your reply". check-review-answered.py already recognises the refusal (its self-test case "quota refusal is not a review (Fix teardown SIGSEGV (#613): cascade hosted-hub creation freeze through the subtree #645 body) expected=red: has not landed"); it simply has nowhere to surface it.
    2. Let a stale verdict re-evaluate itself. Today fix(#3542): the node-content editor binds by JSON key, so an admin's update policy reaches the record #4721 and feat(i18n): the harness-disconnect status lines, in both languages #4725 sat red with zero findings purely because their gate ran at 12:49Z and 15:36Z, before their reviews landed at ~16:33Z. A verdict of "no review has landed" should not stay authoritative once one does.
    3. Revisit on Review is advisory on core main: three PRs merged today with valid findings standing because checks outran the reviewer — the third (#4291) left three security-relevant findings on main until #4297 #4299 whether required was the intended promotion, now that its cost under an outage is measured: ~4h50m, six PRs, three different causes wearing one identical red.

    Ruleset state and the 16:32–16:36Z conclusions are my own measurement; the #4575/#4299 history and the "shipped non-required" statement come from the session that built the gate.

  3. rbuergi commented on Sep 18, 2026

    @rbuergi
    ContributorAuthor

    Measured evidence for ask 2 ("let a stale verdict re-evaluate itself"), from driving #4725 out of exactly this state — because the obvious implementation of ask 2 has a trap that cost me three attempts.

    Re-running the check is NOT sufficient, and fails identically each time

    #4725 sat red with a landed, clean review (### 🟢 Approval recommended, 16:32Z) against a gate run from 15:36Z. I re-ran the Review answered pull_request run three times. Every attempt failed byte-identically, naming the same two review ids:

    review 5249627587 … is a refusal: "…reached their quota limit."
    review 5250166596 … is an unrecognised body: "### 🟢 Approval recommended"
    

    The reason is not staleness of the verdict — it is that a re-run executes the script from the PULL REQUEST's tree, not from main. #4725 branched before #4738, so every attempt ran the pre-fix matcher. What actually cleared it was git merge origin/main into the branch; the very next run went green with no other change.

    Verified before pushing, against the real body rather than the fixture:

    classify_review_body(#4725's actual review) = landed
    

    Why that matters for the design

    If ask 2 is implemented as "re-run the check when a review lands", it inherits this: the re-run uses the branch's copy of check-review-answered.py. For any PR that predates a fix to the matcher — precisely the population stuck during an outage — the automatic re-evaluation would loop on the same verdict and look like the fix not working.

    Two shapes that do not have the problem:

    • Re-evaluate without re-running the workflow — publish a fresh check-run conclusion from a lane that runs main's copy (the pull_request_review event already fires on the right repository state), rather than re-running the PR's own job.
    • Or have the gate report which copy of itself it ran (a version/sha line), so "re-ran and got the same answer" is distinguishable from "re-ran an old copy". That is the same "name the state" principle as ask 1, applied to the gate's own code.

    On ask 1, a small corroboration

    The two states really are indistinguishable in the wild: today's six blocked PRs included ones blocked by the quota refusal, ones blocked by a landed-but-unrecognised body, and #4696 earlier blocked by a stale check-run kind (protection reads the pull_request run; my replies produced only pull_request_review_comment runs). Three different causes, one identical red, and no way to tell them apart without reading the job log each time.

  4. rbuergi commented on Sep 21, 2026

    @rbuergi
    ContributorAuthor

    Triage 2026-09-21: the acute half has cleared; the structural half is what keeps this open

    Blocked on: a design decision about what evidence "reviewed, nothing found" should require — because every way of distinguishing it from "never reviewed" trades safety for liveness, and the gate is currently on the safe side deliberately.

    The quota refusal is over — measured today

    On a pull request opened this evening (#5149, head d0ecc46e51), both check-runs are success:

    Running Copilot Code Review   completed/success
    Review answered              completed/success
    

    So the 4 h 10 min refusal window this issue documents (11:38Z → 15:49Z on 2026-09-18) is not live, and core merges are not held. That is the half that made this urgent, and it cleared without anything being changed — which is also the point: nothing in the repository can tell that it cleared, or would tell anyone when it recurs.

    What has not changed

    The gate still cannot distinguish the two states, and this issue's own reading of that is right: it is behaving correctly and failing in the safe direction. copilot-pull-request-reviewer returning failure with an empty output.title/output.summary is indistinguishable from any other failure without inspecting the reviewer's own output, and treating an unreviewed PR as reviewed is the one error this gate exists to prevent.

    The documented remedy — "a maintainer re-requests the review, or applies the review-waived label — never an agent, and never automatically" — is the right shape and is deliberately not automatable. An agent applying review-waived on a quota refusal would convert a safe failure into a silent bypass, which is exactly the trap in every other "half a change lands and nothing is red" finding in this backlog.

    What would actually close it, and why each is a decision

    1. Detect the refusal and say so in the gate's own conclusion — same red, different sentence, plus a pointer to the remedy. This is the cheapest and it changes nothing about who may unblock a PR. It is also the least valuable on its own: the log already says "review … is a refusal: Copilot was unable to review … reached their quota limit", so a reader who opens the log is already told. The gap is that a reader who only sees failure on the PR page is not.
    2. Alert on a sustained refusal — the thing nobody had on 2026-09-18, when six PRs sat blocked for four hours before anyone noticed. Needs a decision about where such an alert goes and what the threshold is.
    3. Let a sustained refusal auto-waive — the only option that restores liveness, and the one that must not be taken lightly: it makes an unreviewed PR mergeable on a timer.

    🚨 A related trap worth carrying on this issue, because it will bite whoever works on it: the review check's log can read GREEN while the pull request stays BLOCKED, and REST cannot see the difference. Answering a thread fires both a pull_request_review and a pull_request_review_comment event, and branch protection reads only the first kind — so the newest check-run can read success while the rollup cites a real failure from a suite nobody reads. Anyone adding a third state to this gate needs to know that the gate's own verdict is not what protection consumes.

    enhancement, no severity — correct: the gate is not broken, and this is a request for a distinction it cannot currently make.

  5. rbuergi commented on Sep 22, 2026

    @rbuergi
    ContributorAuthor

    Triage 2026-09-22 — ASK 1 IS ALREADY LANDED. One policy decision remains, and it is the whole issue now.

    Read the shipped predicate rather than the thread. This issue asks for two things and the first is
    done
    , which changes what it is handed back for.

    ✅ "Name the state" — implemented, in all three surfaces, with self-test cover

    check-review-answered.py distinguishes a refusal from unanswered findings and says so:

    surface what a refusal now reads as
    run headline (render) RED — UNREVIEWABLE (the reviewer REFUSED to review this pull request)
    step summary (summary_markdown → $GITHUB_STEP_SUMMARY) ❌ red — **unreviewable right now**: the reviewer refused to review this pull request
    remedy (guidance) "nothing on this pull request can answer this … There are no findings to reply to and no commit that changes the answer; pushing, re-running this check and replying to threads all leave it exactly here."

    And it is held there by the self-test rather than by prose: the case(...) harness gained says /
    never_says arguments specifically for this — its own comment reads "#4730 was entirely about
    those two strings being wrong while the verdict was right, so a case that checks only reasons cannot
    see it"
    — with cases "a refusal TELLS the reader it is unreviewable, not that the review is coming",
    "a waived refusal is not reported as unreviewable", and "a 2026-09-18-shaped body that is a refusal
    is still refused"
    . render() is deliberately inside the asserted text so the headline cannot be
    deleted with every case still green.

    The thread's ask 2 (a stale verdict re-evaluating itself) is also substantially addressed, by
    --wait-for-review 15: while a landed review is the ONLY thing missing, the step re-reads for up to
    15 minutes before answering red. That closes the #4721/#4725 shape — a gate that ran at 12:49Z and
    15:36Z against a review that landed at ~16:33Z — without re-running anything from the branch's own
    tree, which is the trap that comment identified (three re-runs of #4725 failed byte-identically because
    a re-run executes the PR's copy of the script).

    🚨 What is left is ONE decision, and it is deliberately not automatable

    The exact act: decide the policy for a structurally unavailable reviewer. Three options, from this
    issue's own text:

    1. hold as today — core stops until quota returns, accepting that a reviewer outage is a fleet-wide
      merge stop (measured: 4 h 10 min on 2026-09-18, six PRs green on tests, all blocked);
    2. auto-retry on a schedule once quota resets, letting the existing gate re-evaluate;
    3. a maintainer-set, time-boxed review-waived, recorded on the PR.

    🚨 One correction for whoever picks it up

    This issue is labelled enhancement, and its title — "a quota refusal has stopped every core merge
    since 11:39Z"
    — reads as an incident. It is not one now. The gate is behaving correctly and failing in
    the safe direction; nothing here argues for weakening it, and the standing recommendation on the thread
    that option 1 "is the cheapest … also the least valuable on its own" refers to naming the state,
    which has since landed. So the remaining decision is about liveness policy under a structurally
    unavailable reviewer
    , not about the gate's correctness.

    What I did NOT establish

    • Whose quota it is (org seat pool vs the reviewer's own plan) or when the month rolls — the service's
      word is "monthly" and that is all the log says.
    • Whether option 2 is implementable without a credential that can re-request a review; I did not look.
    • I did not re-request a review, apply review-waived, or use --admin anywhere.
  6. rbuergi commented on Oct 9, 2026

    @rbuergi
    ContributorAuthor

    Left open. Ask 1 (name the refusal state) is landed. Decision needed: what evidence counts as "reviewed, nothing found" (for example a completed Copilot review object with zero findings on the current head) versus "never reviewed". Each option trades safety for liveness. The review-waived label is the current escape. Effort: about half a day once decided.

  7. rbuergi commented on Oct 9, 2026

    @rbuergi
    ContributorAuthor

    The maintainer decided this on 2026-10-09:

    "reviewed, nothing found" = ONLY a completed Copilot review object on the CURRENT head with zero findings. No timeout-based acceptance. review-waived label stays the escape.

    The implementation is in #6371, as policy review-evidence-current-head. When a pull request has no reviewer thread, Automatic review answered now accepts only a Copilot review of the current head. These no longer count as that evidence:

    • a clean review of an earlier head,
    • the internal reviewer's clean review,
    • the Reviewer unavailable degradation run.

    The new state is labelled RED — NO REVIEW OF THIS HEAD, and the gate does not wait for it to clear. A pull request with findings is still judged by whether its threads are answered. This issue closes once #6371 is merged and the gate is running from main.

  8. rbuergi commented on Oct 9, 2026

    @rbuergi
    ContributorAuthor

    Closed: the decision is implemented and merged to main as #6371 (policy review-evidence-current-head). Automatic review answered now reads .github/scripts/check-review-answered.py from main, which holds the change in 12 places. When a pull request has no reviewer thread, only a completed Copilot review of the current head counts as "reviewed, nothing found". Elapsed time, the degradation run, the internal reviewer and a clean review of an earlier head no longer count. review-waived is still the escape. The new state is labelled RED — NO REVIEW OF THIS HEAD. The self-test runs on every pull request through dotnet-test.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    area:ciCore subsystem: CI lanes, gates and runnersenhancementNew feature or requestfeature:review-answered-gateFeature: review answered gate

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions