Repository navigation
Review gate: "reviewed, nothing found" only on a Copilot review of the current head - #6371
Merged
Merged
Conversation
…e current head (#4730) Policy review-evidence-current-head. With no reviewer thread on the pull request, condition 1 of Automatic review answered now requires a completed Copilot review object on the CURRENT head. A clean review of an earlier head, the internal reviewer's clean review and the 'Reviewer unavailable' degradation run no longer release it; review-waived stays the escape. The new state is named (RED - NO REVIEW OF THIS HEAD) and is not waited for. Refs #4730 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Historical refusals can mask the new verdict, and existing policy documentation still contradicts the change.
2 open findings
What changed in this PR
Tightens review evidence so clean reviews count only for the current head.
Changes:
- Adds current-head Copilot review validation.
- Retires degradation-based merge-gate acceptance.
- Documents and tests the revised policy.
| File | Description |
|---|---|
.github/scripts/check-review-answered.py |
Implements and tests current-head review evidence. |
ReviewFindingsAnswered.md |
Documents the revised gate behavior. |
PolicyNotProse.md |
Registers the new policy. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
408
to
412
| stale_reported = stale_clean | ||
| # Only when the state actually STANDS: a waived refusal is not an unreviewable pull | ||
| # request, it is a reviewed-enough one, and saying otherwise would re-create the | ||
| # confusion in the other direction. | ||
| refused += [first_line(r.get("body")) for k, r in kinds if k == "refused"] |
Contributor
Author
There was a problem hiding this comment.
Good catch. This is fixed in cb5ab4e: once a review has landed and the only problem is that it was not of this head (stale_clean), historical refusals are no longer collected into refused. The reader is therefore told RED — NO REVIEW OF THIS HEAD and not "unreviewable". A new self-test case pins the mixed history, a refusal followed by a clean review of an earlier head. It asserts the NO-REVIEW-OF-THIS-HEAD headline and the re-request remedy, and that none of the UNREVIEWABLE strings appear.
| | `review-then-suites` | RETIRED — superseded by `suites-parallel-with-review`. Was: every pull request in the fleet moved REVIEW FIRST, then the suites, then the arm: the stage gate (`node-repo-stage-gate.yml`, default `review-before-suites: true`, with its `stage-advance.yml` listener) holds the expensive suites until the head's automatic review landed and every finding is answered; the suites then test a fresh merge with the current main (`suites-test-fresh-merge`); and auto-merge is armed only when the review is answered AND every required status check of the base is green on the head (`check-review-answered.py` arm gate condition 4, ported by MeshWeaver.Plugins `PrArming`). Replaces `suites-parallel-with-review` (suites beside the review), which it reverted the same day; `review-before-suites: false` remains a per-caller opt-out that never changes the merge or the arm. Manual: [Staged Pull Request Pipeline](../StagedPullRequestPipeline). | retired | 2026-10-05 → 2026-10-07 | maintainer | | ||
| | `suites-parallel-with-review` | The expensive suites run BESIDE the automatic review: the stage gate (`node-repo-stage-gate.yml`, default `review-before-suites: false`) lets them start as soon as stage 0 is green, on a fresh merge with the current main (`suites-test-fresh-merge`). Merging and arming are untouched — auto-merge is armed only when the review is answered AND every required status check of the base is green on the head (`check-review-answered.py` arm gate condition 4, ported by MeshWeaver.Plugins `PrArming`). Back in force after `review-then-suites` (review first, then the suites) was retired — the reviewer takes one review at a time, so holding the suites behind it left runners idle. `review-before-suites: true` (with the `stage-advance.yml` listener) remains a per-caller opt-in. Manual: [Staged Pull Request Pipeline](../StagedPullRequestPipeline). | in force | 2026-10-07 | maintainer | | ||
| | `suites-test-fresh-merge` | Every suite job of a pull request tests the head merged onto the base branch's CURRENT tip, resolved when the suites start and reported in the run — never GitHub's `refs/pull/N/merge` as it stood at the push, and never a stale merge on a re-run. The merge is computed in the job (no push, no branch update, no new head or review); a head that conflicts with the current tip is red. Manual: [Fresh Merge Under Test](../FreshMergeUnderTest). | in force | 2026-10-05 | maintainer | | ||
| | `review-evidence-current-head` | "Reviewed, nothing found" is ONLY a completed Copilot review object on the CURRENT head with zero findings. `Automatic review answered` (`check-review-answered.py` `evaluate`, via `copilot_review_on`) accepts no timeout, no `Reviewer unavailable` degradation run, no internal-reviewer review and no clean review of an earlier head as that evidence; a pull request WITH findings is still judged by its answered threads (`review-once-per-pull-request`). The `review-waived` label applied by a maintainer stays the escape. A generated-only App pull request stays *not owed* (provenance, not a claim of review). Manual: [Review Findings Answered](../ReviewFindingsAnswered) → *Reviewed, nothing found*. | in force | 2026-10-09 | maintainer | |
Contributor
Author
There was a problem hiding this comment.
Agreed. This is fixed in cb5ab4e and e65557e. The review-once-per-pull-request register row now names its narrowing: with no reviewer thread, Automatic review answered accepts only a Copilot review of the current head. The manual's One review per pull request section at lines 178 and 183 now says the same and links to Reviewed, nothing found. The stage gate and the arm gate keep the any-head acceptance, as before.
…iew) Refs #4730 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Contributor
Test Results 24 files 24 suites 49m 58s ⏱️ Results for commit e65557e. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refs #4730
The decision being implemented
Recorded as policy
review-evidence-current-headinDoc/Architecture/PolicyNotProse.What changes in
Automatic review answered(check-review-answered.pyevaluate)PENDING, not a refusal) whosecommit_idis the current head (copilot_review_on). These are not that evidence any more:Reviewer unavailabledegradation check run, which released condition 1 with nobody having reviewed the pull request.review-once-per-pull-request).RED — NO REVIEW OF THIS HEAD, the step summary, and To go green. The remedy is a maintainer re-requesting the Copilot review, orreview-waived. It is a structured field (Verdict.stale_clean), and--wait-for-reviewdoes not wait for it, because no wait delivers a review the ruleset (review_on_push: false) will not request.Consequence for reviewers to weigh
The rulesets review once per pull request, so a clean pull request that is pushed again after its review reads no review of this head until a maintainer re-requests Copilot or applies the waiver. The push can be a fix or a merge of
main. Pull requests with findings are unaffected.Verification
python3 .github/scripts/check-review-answered.py --self-test: all cases green. There are 10 newEVIDENCEcases (positive and negative pairs), plus 2 generated-only cases. The degradation-release cases are now negative controls.Docs
ReviewFindingsAnswered.mdhas a new section, Reviewed, nothing found — the only evidence that counts. The degradation section is marked retired for the merge gate, and the rule, the table and the summary are updated.🤖 Generated with Claude Code