fix(ci): stop the revert oracle scoring an all-skipped baseline as pass #1199
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
| # This Source Code Form is subject to the terms of the Mozilla Public | |
| # License, v. 2.0. If a copy of the MPL was not distributed with this | |
| # file, You can obtain one at https://mozilla.org/MPL/2.0/. | |
| name: Review posted | |
| # NO `paths:` FILTER AND NO `branches:` FILTER, for the reason | |
| # pr-review-signal.yml documents: a presence gate that can itself be filtered out | |
| # is the same defect it exists to catch (#3305). | |
| # | |
| # `synchronize` is load-bearing here rather than incidental: the gate adjudicates | |
| # a verdict for ONE head, so a new commit must re-open the question. Without it a | |
| # review of commit 1 would hold a PR green through commit 20. | |
| on: | |
| pull_request: | |
| types: [opened, synchronize, reopened, ready_for_review] | |
| permissions: | |
| contents: read | |
| # Both comment surfaces plus the review list, AND the stand-down label. | |
| # | |
| # `pull-requests: write` IS REQUIRED FOR THE LABEL, and this was established by | |
| # measurement after two wrong answers. The original grant was `issues: write`, | |
| # on the reasoning that a label is an issue resource; every label call has | |
| # failed since the day it was written. Switching from `gh pr edit` (GraphQL) to | |
| # the REST issue-labels endpoints did not help either -- same 403, measured on | |
| # PR #3589: | |
| # | |
| # gh: Resource not accessible by integration (HTTP 403) | |
| # | |
| # A reviewer had inferred from GitHub's `x-accepted-github-permissions: | |
| # issues=write; pull_requests=write` that the semicolon meant OR, so | |
| # `issues: write` would suffice. The live 403 says otherwise. Labelling a PULL | |
| # REQUEST needs `pull-requests: write`, whatever the resource is called. | |
| # | |
| # THIS IS A WIDER GRANT THAN THE COMMENT IT REPLACES WANTED, and that is stated | |
| # rather than glossed: the token can now comment on, close and retarget this | |
| # PR, not merely label it. Two things make it acceptable. The surface already | |
| # exists -- `claude-review.yml` holds `pull-requests: write` and also runs from | |
| # the PR's checkout, so nothing new is reachable that was not before. And the | |
| # alternative is not a smaller grant, it is a stand-down that never happens: | |
| # CodeRabbit reviewing every PR forever while three green steps say otherwise. | |
| # | |
| # The repository default is `read`, and a workflow CAN elevate past it -- proved | |
| # by `claude-review.yml` posting the marker on #3589 under exactly that setting. | |
| # So the default was never the cap here, and raising it repo-wide is not needed. | |
| pull-requests: write | |
| # For `gh label create`, which is repository-level rather than PR-level. | |
| issues: write | |
| concurrency: | |
| group: review-posted-${{ github.event.pull_request.number }} | |
| cancel-in-progress: true | |
| jobs: | |
| review-posted: | |
| name: Review posted | |
| runs-on: ubuntu-latest | |
| # ABOVE the gate's poll budget (REVIEW_POSTED_POLL_SECONDS in | |
| # scripts/review-lane-budget.mjs), deliberately, so a run that exhausts | |
| # the budget still gets to PRINT its verdict rather than being killed | |
| # mid-wait with no output and no `covered` value. The sibling gate states the | |
| # same rule at pr-review-signal.yml (a job cap comfortably over its own poll | |
| # budget; that gate asserts the pairing against its own YAML rather than | |
| # restating it, because this comment was stale about both numbers once). An | |
| # earlier version of this file had 5 minutes against a 600s budget, which | |
| # would have killed the job on every PR before it could say anything. | |
| timeout-minutes: 30 | |
| steps: | |
| # THE GATE RUNS FROM THE PR; ITS AUTHORITY COMES FROM THE BASE BRANCH. | |
| # | |
| # An earlier version pinned the whole checkout to `pull_request.base.sha` to | |
| # stop a contributor editing the rules that judge them. That closed the | |
| # forgery vector and broke two things: this very PR could not run, because | |
| # the gate does not exist on base yet, and any future PR IMPROVING the gate | |
| # would be tested against the old copy and never exercise its own change. | |
| # | |
| # Split instead. The workspace is the PR, so the harness tests the gate the | |
| # PR actually ships. The CONFIG -- which names who may post a marker, and is | |
| # the thing worth forging -- is read from the base branch below. | |
| # | |
| # RESIDUAL RISK, STATED: the gate's CODE still comes from the PR, so an edit | |
| # to MARKER_RE or to `evaluate` is not neutralised by this split. That edit | |
| # is visible in the diff and is what human review is for. It is a smaller | |
| # surface than the config (a one-word `expectedAuthors` addition is easy to | |
| # miss in a large diff; a change to the matching logic is not), and the | |
| # measured population here is 100% same-repo PRs over the last 80 merges. | |
| # If that changes, or if this gate goes enforcing on fork PRs, pin the code | |
| # to base as well and accept that gate changes take effect one merge later. | |
| - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 | |
| with: | |
| lfs: false | |
| persist-credentials: false | |
| - uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0 | |
| with: | |
| node-version: 22 | |
| # Runs BEFORE the gate and in this job rather than only in the scripts/ | |
| # glob catch-all: relying on a path-filtered job to test this gate would put | |
| # its harness behind a filter this workflow deliberately does not have. | |
| # | |
| # No `pnpm install`: the gate and its harness import node builtins only and | |
| # shell out to `gh`, preinstalled on runners. A gate that decides whether a | |
| # review happened must not be able to fail because a registry was slow. | |
| # | |
| # `review-provenance` is named here for the same reason: the gate's | |
| # "was this finding written at the head" predicate now lives in that | |
| # module (#3729), so leaving its tests to the scripts/ glob would put the | |
| # half that decides the verdict behind a filter this workflow refuses. | |
| # | |
| # `review-marker` is the third, and the same rule again (#3862): the | |
| # marker grammar, `MARKER_VERDICTS` and `certifiesDiff` moved there, and | |
| # `certifiesDiff` IS the `full` output this workflow turns into the | |
| # `llm-reviewed` label. Its tests are otherwise reachable only behind | |
| # test.yml's `scripts/**` path filter, so a PR touching neither would | |
| # decide the stand-down with an untested predicate. | |
| - name: Unit-test the gate itself | |
| run: >- | |
| node --test | |
| scripts/check-review-posted.test.mjs | |
| scripts/lib/review-provenance.test.mjs | |
| scripts/lib/review-marker.test.mjs | |
| # CLEARED FIRST, BEFORE THE GATE WAITS. This is a race, not a tidy-up. | |
| # | |
| # CodeRabbit's `auto_review` reads labels at PUSH time. If the label were | |
| # only reconciled after the poll, then on a new commit CodeRabbit would see | |
| # the label left over from the PREVIOUS head, stand down immediately, and | |
| # the gate would clear it minutes later -- after the decision it was meant | |
| # to influence. CodeRabbit does not re-review on an `unlabeled` event, so | |
| # that commit would ship read by neither reviewer. | |
| # | |
| # Clearing up front inverts the failure: the worst case is CodeRabbit | |
| # reviewing a PR the Claude lane also covers, which costs quota. The other | |
| # ordering costs a review. | |
| - name: Clear the stand-down label for this new head | |
| if: github.event.action == 'synchronize' || github.event.action == 'reopened' | |
| continue-on-error: true | |
| env: | |
| GH_TOKEN: ${{ github.token }} | |
| run: | | |
| # REST, not `gh pr edit`. `gh pr edit` goes through the GraphQL | |
| # `removeLabelsFromLabelable` mutation, which needs `pull-requests: | |
| # write`; this workflow holds `issues: write`, which is what the REST | |
| # issue-labels endpoints require and is the smaller grant. Measured on | |
| # PR #3587: every label call failed with "Resource not accessible by | |
| # integration (removeLabelsFromLabelable)" while the step reported | |
| # success, so the stand-down NEVER happened and CodeRabbit reviewed | |
| # every PR anyway. | |
| gh api "repos/${{ github.repository }}/issues/${{ github.event.pull_request.number }}/labels/llm-reviewed" \ | |
| --method DELETE --silent \ | |
| || echo "No label to clear (or no write access on a fork PR); CodeRabbit will review." | |
| # The config is the thing worth forging: it names who may post a marker. | |
| # Taken from the BASE branch so a PR cannot add its own author and pass. | |
| # When base has no copy -- true only for the PR that introduces this gate -- | |
| # that is said out loud and the PR's own config is used, because the | |
| # alternative is a job that can never run on the change that adds it. | |
| - name: Take the reviewer allowlist from the base branch | |
| id: cfg | |
| env: | |
| GH_TOKEN: ${{ github.token }} | |
| run: | | |
| set -euo pipefail | |
| # Written OUTSIDE the checkout: a file dropped into scripts/ would be | |
| # picked up by this repo's own source-shape gates and could be committed | |
| # by accident. | |
| out="$RUNNER_TEMP/review-posted.base.json" | |
| if gh api "repos/${{ github.repository }}/contents/scripts/review-posted.config.json?ref=${{ github.event.pull_request.base.sha }}" \ | |
| --jq '.content' 2>/dev/null | base64 -d > "$out" 2>/dev/null && [ -s "$out" ]; then | |
| echo "config=$out" >> "$GITHUB_OUTPUT" | |
| echo "Using the base branch's allowlist." | |
| else | |
| echo "config=scripts/review-posted.config.json" >> "$GITHUB_OUTPUT" | |
| echo "::notice::No allowlist on the base branch yet (bootstrap). Using this PR's copy;" | |
| echo "::notice::from the next PR onward the base copy is authoritative." | |
| fi | |
| - name: Check a review was actually posted for this head | |
| id: gate | |
| env: | |
| GH_TOKEN: ${{ github.token }} | |
| GITHUB_REPOSITORY: ${{ github.repository }} | |
| run: | | |
| poll_seconds="$(node scripts/review-lane-budget.mjs --poll-seconds)" | |
| node scripts/check-review-posted.mjs \ | |
| --pr "${{ github.event.pull_request.number }}" \ | |
| --sha "${{ github.event.pull_request.head.sha }}" \ | |
| --config "${{ steps.cfg.outputs.config }}" \ | |
| --timeout-seconds "$poll_seconds" | |
| # ADDS ONLY. The clearing half runs up front, above. | |
| # | |
| # `continue-on-error` because both failure modes here land on the SUCCESS | |
| # path and neither means the review is bad: a fork PR's GITHUB_TOKEN is | |
| # read-only whatever `permissions:` says, and the label may not exist in the | |
| # repository yet. Failing the job for either would turn a passing gate red. | |
| # The cost of the label not being applied is that CodeRabbit reviews a PR | |
| # that was already covered -- quota, not correctness. | |
| # | |
| # Reads `steps.gate.outputs.full`, never the step's exit code: in advisory | |
| # mode a failing verdict still exits 0, so the exit code would mark an | |
| # unreviewed PR as reviewed. | |
| # | |
| # `full`, NOT `covered`. `covered` answers "does a verdict exist for this | |
| # head" and is claude-review.yml's dedup key; `full` answers "was the WHOLE | |
| # diff reviewed", which is the only thing that entitles another reviewer to | |
| # stand down. They part company on a degraded review (#3679): the lane | |
| # reviewed the files that fit, so the head is covered and must not be | |
| # re-reviewed, while the omitted files were read by nobody and CodeRabbit | |
| # must still look at the PR. | |
| - name: Mark covered so CodeRabbit can stand down | |
| if: always() && steps.gate.outputs.full == 'true' | |
| continue-on-error: true | |
| env: | |
| GH_TOKEN: ${{ github.token }} | |
| run: | | |
| gh label create llm-reviewed --repo "${{ github.repository }}" \ | |
| --color 0e8a16 --description "A review was verified as posted for this PR's head." \ | |
| 2>/dev/null || true | |
| gh api "repos/${{ github.repository }}/issues/${{ github.event.pull_request.number }}/labels" \ | |
| --method POST -f "labels[]=llm-reviewed" --silent || true | |
| # READ IT BACK, because this step is `continue-on-error` and therefore | |
| # cannot report its own failure. That is how the permission bug above | |
| # survived: three green steps and no label on the PR. The stand-down is | |
| # a CLAIM about another system's behaviour, so it is checked, not | |
| # assumed -- the same rule the marker itself is held to. | |
| # Read into a VARIABLE, not a pipeline into `grep -q`. That pipeline is | |
| # correct today only because this step inherits `bash -e` WITHOUT | |
| # pipefail: with pipefail on, `grep -q` exiting early can SIGPIPE `gh` | |
| # and flip the whole read-back to permanently reporting "not on the | |
| # PR". A correctness that depends on an unstated shell option is one | |
| # `shell: bash` away from being wrong, silently, in the check whose | |
| # entire job is to not be silently wrong. `per_page=100` because the | |
| # default 30 would make a heavily-labelled PR warn while the label is | |
| # actually there. | |
| have=$(gh api "repos/${{ github.repository }}/issues/${{ github.event.pull_request.number }}/labels?per_page=100" \ | |
| --jq '.[].name' 2>/dev/null || true) | |
| if printf '%s\n' "$have" | grep -qx 'llm-reviewed'; then | |
| echo "✅ llm-reviewed is ON the PR: CodeRabbit will stand down for this head." | |
| else | |
| echo "⚠️ THE LABEL IS NOT ON THE PR, so CodeRabbit will review this PR as well." | |
| echo " Not a failure of the review -- the marker is verified either way -- but the" | |
| echo " quota saving did not happen. Most likely cause: the token lacks label write" | |
| echo " (fork PR), or the repository's default workflow permissions were tightened." | |
| fi | |
| # The refusal paths write `full=false` too, so a stale label is cleared even | |
| # when the gate could not read its own inputs. Without this a PR carrying | |
| # the label from an earlier head would keep it through any number of | |
| # refused runs. A head that degrades from full to partial clears it here | |
| # for the same reason. | |
| - name: Clear the stand-down label when this head is not fully reviewed | |
| if: always() && steps.gate.outputs.full == 'false' | |
| continue-on-error: true | |
| env: | |
| GH_TOKEN: ${{ github.token }} | |
| run: | | |
| gh api "repos/${{ github.repository }}/issues/${{ github.event.pull_request.number }}/labels/llm-reviewed" \ | |
| --method DELETE --silent \ | |
| || echo "Nothing to clear." |