diff --git a/.github/workflows/pr-risk.yml b/.github/workflows/pr-risk.yml index e540c7d..bfd2b2a 100644 --- a/.github/workflows/pr-risk.yml +++ b/.github/workflows/pr-risk.yml @@ -26,6 +26,50 @@ name: PR Risk Grade (reusable) # out, then labels what it has. Pair the caller with a per-PR cancel-in-progress concurrency # group so a new push supersedes a waiting run instead of stacking behind it. # +# ON-DEMAND GRADING (`pr_number` / `pr_numbers`): supply a PR number and that PR is graded with +# no `pull_request` event involved — which is how a repo enrolling mid-stream grades the open +# queue it already has, and how a PR is re-graded after a `.github/risk.json` change or a +# `risk-dispute`. Absent, the workflow reads the event exactly as before. Three things differ on +# the by-number path, all of them deliberate: +# * BOT-AUTHORED PRs ARE GRADED. The `github.actor != 'dependabot[bot]'` clause callers put in +# their `if:` is a TOKEN guard, not a policy one: a bot-triggered `pull_request` run gets a +# read-only GITHUB_TOKEN, so the label write — this workflow's entire product — would 403. +# On a dispatch the actor is the human who pressed the button and the token is writable, so +# the reason evaporates. Excluding them would also bias any backfill corpus badly; bots +# author a substantial minority of merged PRs. The same is true of the fork clause, and for +# the same reason — fork RISK is unaffected either way, because `external` is derived from +# the API's `isCrossRepository`, never from the actor, and forks grade R3 with no exceptions. +# * THE BASE REF IS RE-READ FROM THE API, per target, because there is no event payload to take +# it from. It is load-bearing rather than cosmetic: it selects which branch's +# `.github/risk.json` judges the PR, live PRs are commonly stacked on feature branches rather +# than the default branch, and an empty ref is NOT an error to the contents API — it silently +# resolves to the default branch. So an unresolvable base ref FAILS that target instead of +# grading it against rules nobody read. +# * A LOW `wait_for_checks_minutes` IS RIGHT HERE, and is not the same trade-off as on the event +# path. The wait exists to outlast the rest of the rollup while our own check sits in it; a +# dispatched run's check is attached to the dispatched ref, not to the PR's head commit, so +# it is not in that rollup at all and a settled PR reads its true state (`SUCCESS`, nothing +# pending) on the first poll. It is still not FREE: `0` breaks out after a single read, ahead +# of both the not-yet-registered grace window and the "require a settled reading to repeat" +# confirmation, so a target someone pushed to minutes ago lands the honest R2 floor. `1` costs +# one 15s backoff and a second read per PR and keeps the confirmation. Prefer `1`, not `0`. +# +# Batch (`pr_numbers`) grades one target at a time and ONE UNREADABLE PR NEVER ABANDONS THE REST: +# each target's outcome is recorded, the whole list is attempted, and the run then reports whether +# any target failed. There is deliberately no `all_open: true` — an explicit list is bounded, +# reviewable and re-runnable, and the list is capped rather than silently truncated. A dispatch +# that reaches this workflow with no target at all FAILS LOUDLY instead of exiting 0: a button +# that silently grades nothing is worse than no button. +# +# A BATCH CANNOT SERIALIZE PER-PR, and no concurrency key can make it: one run covers N pull +# requests and a run belongs to exactly one group. The label sync is a read-current / +# delete-stale / add-target sequence, so a batch that overlaps a `pull_request` run for one of ITS +# numbers can interleave with it and leave that PR briefly carrying two `risk:*` labels — the next +# grade of that PR re-syncs it to one, and the label gates nothing in the meantime. Two mitigations +# and one limit: dispatch a backfill when the queue is quiet; prefer `pr_number` when you want the +# per-PR concurrency group to serialize a re-grade against event runs; and a DELETE of a label +# another run already removed is tolerated rather than painted red (see apply-risk-label.sh). +# # SECRETS: none. This workflow declares no `secrets:` inputs and callers pass none — the only # credential in the job is the automatic `GITHUB_TOKEN` (`github.token`), used for the PR read # and the one label write. There is no `secrets: inherit` to add and nothing to rotate. @@ -48,13 +92,43 @@ name: PR Risk Grade (reusable) # on: # pull_request: # types: [opened, synchronize, reopened, ready_for_review] +# workflow_dispatch: +# inputs: +# pr_number: +# description: Grade ONE pr by number (enrollment backfill, or a manual re-grade). +# required: false +# pr_numbers: +# description: Grade several, comma-separated (12,15,20). Wins over pr_number. +# required: false # concurrency: -# group: pr-risk-${{ github.event.pull_request.number }} +# # THE KEY MUST CARRY THE RESOLVED TARGET, not just the event PR. On a workflow_dispatch +# # `github.event.pull_request.number` is empty, so an event-only key collapses to ONE +# # constant group for every dispatch — and with cancel-in-progress that means each +# # dispatched PR cancels the one before it, which is precisely the shape a backfill has. +# # `inputs` is empty on a `pull_request` run, so this one expression is correct on both. +# # A `pr_numbers` LIST keys its own group, which serializes identical batches but cannot +# # serialize a batch against a `pull_request` run for one of its members — see "A BATCH +# # CANNOT SERIALIZE PER-PR" above for what that costs and how to avoid it. +# group: ${{ github.workflow }}-${{ inputs.pr_numbers || inputs.pr_number || github.event.pull_request.number }} # cancel-in-progress: true # permissions: # contents: read # jobs: # pr-risk: +# # SCOPE EVERY GUARD CLAUSE TO THE EVENT IT DESCRIBES. Both clauses below are TOKEN +# # guards: a fork's and a bot's `pull_request` run gets a read-only GITHUB_TOKEN, so the +# # label write would 403 and the check would go red on every dependency bump. Neither is +# # a risk judgement — forks are graded R3 from the API's own fork flag, never from the +# # actor. On a workflow_dispatch there is no `github.event.pull_request`, so an +# # unscoped `head.repo.full_name == github.repository` is FALSE and the job SKIPS +# # SILENTLY: the dispatch button appears to do nothing, no run, no error, no annotation. +# # Keeping the clauses behind the event test is what makes the button work at all — and +# # on a dispatch the token is writable and the actor is a human, so there is nothing left +# # for either clause to protect. +# if: >- +# github.event_name != 'pull_request' || +# (github.actor != 'dependabot[bot]' && +# github.event.pull_request.head.repo.full_name == github.repository) # permissions: # contents: read # issues: write # create the risk:* labels repo-side on first use @@ -68,6 +142,20 @@ name: PR Risk Grade (reusable) # uses: Comfy-Org/github-workflows/.github/workflows/pr-risk.yml@ # v1 # with: # workflows_ref: +# # Both are empty on a `pull_request` run (`inputs` is empty there), which is exactly +# # the no-input event path — so ONE caller shape serves both event and dispatch and +# # there is nothing to keep in sync between two jobs. +# pr_number: ${{ inputs.pr_number }} +# pr_numbers: ${{ inputs.pr_numbers }} +# # For a backfill, dispatch with this lowered — see ON-DEMAND GRADING above for why a +# # low wait is sound on the by-number path and why `0` still is not the right value. +# # wait_for_checks_minutes: 1 +# +# A SKIPPED CALLER JOB IS INVISIBLE FROM HERE. This workflow cannot detect, warn about or recover +# from a caller whose `if:` excluded it — no run is created, so nothing of ours executes. The +# block above is the only lever, which is why the `if:` is spelled out rather than left to the +# enroller. What this workflow CAN do, and does, is refuse to be a silent no-op once it is +# actually reached: a run with no resolvable target fails with a message naming both inputs. # # GRANT ALL SIX OR THE RUN NEVER STARTS. A reusable workflow can only NARROW the caller's # token, never elevate it, so a caller whose block is short of what the `grade` job declares is @@ -80,6 +168,32 @@ name: PR Risk Grade (reusable) on: workflow_call: inputs: + pr_number: + description: >- + Grade ONE pull request by number instead of the event's. Leave it EMPTY on a + `pull_request` run: with no number supplied the target, the base ref and every emitted + label are exactly what they were before this input existed. Supplying it is what makes + grading possible without a `pull_request` event — the enrollment backfill of an + already-open queue, and the manual re-grade after a risk-map change or a + `risk-dispute`. Bot-authored and fork PRs ARE graded on this path (see the header). + Typed `string` rather than `number` because `workflow_dispatch` inputs arrive as + strings, and because an empty string is what lets the fall-through to the event's own + number stay a single expression. + type: string + required: false + default: '' + pr_numbers: + description: >- + Grade SEVERAL pull requests by number (comma-separated, e.g. `12,15,20`). Takes + precedence over `pr_number` when both are set. Targets are graded one at a time, and + one unreadable PR is REPORTED without abandoning the rest — the whole list is + attempted before the run decides its own outcome. Pair a long list with a low + `wait_for_checks_minutes`: the per-target waits are additive and the run stops starting + new targets once the job's budget is spent, reporting the un-attempted ones by number + rather than being cancelled mid-label. There is deliberately no `all_open: true`. + type: string + required: false + default: '' fleet_logins: description: >- GitHub logins whose PRs are supervised-agent output (comma-separated). @@ -107,12 +221,14 @@ on: default: '' wait_for_checks_minutes: description: >- - How long to wait for the REST of the check rollup to settle before + How long to wait, PER TARGET, for the REST of the check rollup to settle before labeling (the grading run itself is excluded from the rollup it reads). 0 labels immediately — expect R2 floors from still-pending checks. CLAMPED to what a 30-minute job can actually spend waiting (25), so an over-large value degrades to a shorter wait instead of a job - cancelled mid-sleep with the label never applied. + cancelled mid-sleep with the label never applied. On the by-number path a dispatched + run's own check is not in the graded PR's rollup at all, so `1` is usually enough + there; see ON-DEMAND GRADING in the header for why `0` still is not. type: number required: false default: 10 @@ -174,8 +290,17 @@ jobs: statuses: read # the same rollup's legacy commit-status contexts env: REPO: ${{ github.repository }} - PR_NUMBER: ${{ github.event.pull_request.number }} - BASE_REF: ${{ github.event.pull_request.base.ref }} + # The target list. `pr_numbers` wins over `pr_number`, and with neither supplied this is + # the event's own PR — so the event path resolves to the same single number it always did. + PR_NUMBERS: ${{ inputs.pr_numbers || inputs.pr_number || github.event.pull_request.number }} + # The event PR's base ref, passed through ONLY when no number was supplied. When one WAS, + # the target may be a different PR than the event's (or there may be no event at all), and + # the event's base ref would then point the override read at the wrong branch — so the + # by-number path re-reads it from the API per target instead. The expression reads oddly + # because GitHub's `||` yields the first TRUTHY operand: the `&& ... || ''` tail is how a + # conditional passthrough is spelled, and it yields '' both when a number was supplied and + # when there is no pull_request payload to read. + BASE_REF: ${{ (inputs.pr_number || inputs.pr_numbers) == '' && github.event.pull_request.base.ref || '' }} GH_TOKEN: ${{ github.token }} steps: - name: Load pr-risk tool @@ -188,175 +313,130 @@ jobs: ref: ${{ inputs.workflows_ref }} path: _pr_risk_tool persist-credentials: false - - - name: Fetch per-repo overrides from the base ref - id: overrides + - name: Grade the target PR(s) and sync the label + id: grade + # The per-target sequence — resolve the base ref, fetch that ref's override files, poll + # the grader until the rest of the rollup settles, sync the one label — lives in + # scripts/pr-risk/grade-targets.sh rather than inline here, so the event path and the + # by-number path cannot drift into two copies of it, and so it is unit-testable at all + # (tests/test_grade_targets.sh drives every branch with a stubbed `gh`). It grades ONE + # target or fifty through the same code; a batch's per-target failures are recorded and + # the remaining targets still graded. env: MAP_PATH: ${{ inputs.repo_map_path }} RB_PATH: ${{ inputs.repo_runbooks_path }} - run: | - set -uo pipefail - # Read from the BASE ref, so the PR being graded cannot edit the rules - # that judge it. Absent (a genuine 404) falls back to the shipped - # defaults; present-but-invalid is left for the grader's structural - # validation to reject loudly — a repo that commits a corrupt map must - # see red, not silent generic grading. - # - # ONLY A 404 MEANS ABSENT. Treating any non-zero exit as "no override" - # meant a 403 rate-limit, a 5xx or a network blip silently graded the - # PR against the generic default map instead of the repo's sharpened - # one — a LOWER tier computed from an input nobody read, which is the - # confident-answer-from-an-unread-source failure the unknown contract - # forbids everywhere else. So the status code is captured and anything - # that is not 200-or-404 fails the run. - errf="$(mktemp)" - fetch_override() { # -> prints the outfile, or nothing when absent - local p="$1" out="$2" - if gh api "repos/${REPO}/contents/${p}?ref=${BASE_REF}" \ - -H "Accept: application/vnd.github.raw" > "$out" 2>"$errf"; then - echo "using ${p} from ${BASE_REF}" >&2 - printf '%s' "$out" - elif grep -q '(HTTP 404)' "$errf"; then - rm -f "$out" # the redirect already created it empty; nothing must read it - echo "no ${p} on ${BASE_REF} — using the generic default" >&2 - else - echo "::error::could not read ${p} from ${BASE_REF}: $(tr '\n' ' ' < "$errf"). NOT falling back to the generic default: that would grade this PR against rules nobody read." >&2 - return 1 - fi - } - map="$(fetch_override "$MAP_PATH" repo-risk-map.json)" || exit 1 - rb="$(fetch_override "$RB_PATH" repo-risk-runbooks.json)" || exit 1 - rm -f "$errf" - echo "map=$map" >> "$GITHUB_OUTPUT" - echo "runbooks=$rb" >> "$GITHUB_OUTPUT" - - - name: Grade (waits for the rest of the rollup to settle) - id: grade - env: FLEET_LOGINS: ${{ inputs.fleet_logins }} BOT_LOGINS: ${{ inputs.bot_logins }} + LABEL_MAP: ${{ inputs.label_map }} SELF_CONTEXT: ${{ github.workflow }} SELF_RUN_ID: ${{ github.run_id }} WAIT_MINUTES: ${{ inputs.wait_for_checks_minutes }} - MAP_OVERRIDE: ${{ steps.overrides.outputs.map }} - RB_OVERRIDE: ${{ steps.overrides.outputs.runbooks }} JOB_TIMEOUT_MINUTES: 30 # keep in sync with timeout-minutes on the job + TOOL_DIR: _pr_risk_tool/scripts/pr-risk run: | set -uo pipefail - TOOL="_pr_risk_tool/scripts/pr-risk/grade-pr-risk.sh" - ARGS=( --repo "$REPO" --pr "$PR_NUMBER" - --self-context "$SELF_CONTEXT" --self-run-id "$SELF_RUN_ID" ) - [ -n "$FLEET_LOGINS" ] && ARGS+=( --fleet-logins "$FLEET_LOGINS" ) - [ -n "$BOT_LOGINS" ] && ARGS+=( --bot-logins "$BOT_LOGINS" ) - [ -n "$MAP_OVERRIDE" ] && ARGS+=( --map "$MAP_OVERRIDE" ) - [ -n "$RB_OVERRIDE" ] && ARGS+=( --runbooks "$RB_OVERRIDE" ) - - # CLAMP THE WAIT TO THE JOB'S OWN BUDGET rather than trusting the input - # description. A caller who passes 40 against a 30-minute timeout gets a - # job cancelled mid-sleep: the label step never runs, the PR keeps - # whatever label the previous push left, and the check goes red for a - # reason nobody would guess. 5 minutes of headroom is left for the - # label + summary steps. - max_wait=$(( JOB_TIMEOUT_MINUTES - 5 )) - wait_minutes="${WAIT_MINUTES:-10}" - [ "$wait_minutes" -ge 0 ] 2>/dev/null || wait_minutes=10 - if [ "$wait_minutes" -gt "$max_wait" ]; then - echo "::warning::wait_for_checks_minutes=${wait_minutes} exceeds what a ${JOB_TIMEOUT_MINUTES}-minute job can spend waiting — clamped to ${max_wait}" - wait_minutes="$max_wait" - fi - deadline=$(( $(date +%s) + 60 * wait_minutes )) - empty_deadline=$(( $(date +%s) + 120 )) # grace for checks to REGISTER at all - waited=0 - # Poll backoff: 15s doubling to 120s. The fixed 30s poll idled a runner - # up to the full wait on every `synchronize` event, per PR, across the - # fleet, for a grade that is one API call — increasing backoff cuts both - # runner minutes and API calls without changing the settle behaviour. - delay=15 - settled_once=0 - # rc=3 (the PR could not be read) covers rate limits, secondary rate - # limits and transient 5xx as well as a genuinely missing PR. Labeling - # `ungraded` on the first blip turns a momentary hiccup into a durable - # verdict, so it is retried with backoff. This gets its OWN budget, not - # the settle deadline: a transient API failure has nothing to do with - # how long the caller wants to wait for CI, and sharing the deadline - # would leave `wait_for_checks_minutes: 0` with no retry at all. - unreadable_tries=0 - max_unreadable_tries=4 - read_deadline=$(( $(date +%s) + 150 )) - read_delay=10 - while :; do - rc=0 - bash "$TOOL" "${ARGS[@]}" > record.json || rc=$? - case "$rc" in - 0|1) ;; # graded (1 = graded but unknown — still labeled) - 3) unreadable_tries=$(( unreadable_tries + 1 )) - if [ "$unreadable_tries" -lt "$max_unreadable_tries" ] && [ "$(date +%s)" -lt "$read_deadline" ]; then - echo "PR read failed (attempt ${unreadable_tries}/${max_unreadable_tries}) — retrying in ${read_delay}s" - sleep "$read_delay"; waited=$(( waited + read_delay )) - read_delay=$(( read_delay * 2 )); [ "$read_delay" -le 60 ] || read_delay=60 - continue - fi - echo "PR unreadable via the API after ${unreadable_tries} attempt(s) — labeling ungraded" - printf '{}' > record.json - break ;; - *) echo "grader failed (rc=$rc)"; exit 1 ;; # setup/usage bug: fail loud - esac - pending="$(jq -r '.checks_pending_excl_self // false' record.json)" - state="$(jq -r '.checks_state // "none"' record.json)" - now="$(date +%s)" - [ "$now" -lt "$deadline" ] || break - if [ "$pending" = "true" ]; then - settled_once=0 - elif [ "$state" = "none" ] && [ "$now" -lt "$empty_deadline" ]; then - # An EMPTY rollup is checks that have not registered yet, not CI - # that finished — exiting the poll here would apply a label that a - # check appearing a second later never revises. Bounded by a SHORT - # grace window rather than the whole budget, so a repo with - # genuinely no CI is not made to wait 25 minutes on every PR to - # arrive at the same honest R2. - settled_once=0 - elif [ "$settled_once" -eq 0 ]; then - # First settled reading. Require it to REPEAT before labeling: one - # transiently-quiet poll is not a settled rollup. - settled_once=1 - else - break - fi - sleep "$delay"; waited=$(( waited + delay )) - delay=$(( delay * 2 )); [ "$delay" -le 120 ] || delay=120 - done - tier="$(jq -r '.risk.tier // "unknown"' record.json)" - echo "tier=$tier" >> "$GITHUB_OUTPUT" - echo "waited=$waited" >> "$GITHUB_OUTPUT" - echo "graded tier: $tier (waited ${waited}s for the rollup)" - - - name: Apply risk label - env: - TIER: ${{ steps.grade.outputs.tier }} - LABEL_MAP: ${{ inputs.label_map }} - run: | - set -uo pipefail - bash _pr_risk_tool/scripts/pr-risk/apply-risk-label.sh + bash _pr_risk_tool/scripts/pr-risk/grade-targets.sh - name: Step summary if: always() env: TIER: ${{ steps.grade.outputs.tier }} WAITED: ${{ steps.grade.outputs.waited }} + TARGETS: ${{ steps.grade.outputs.targets }} run: | set -uo pipefail + RESULTS=pr-risk-results.jsonl + # WHETHER THE FILE EXISTS AT ALL IS ITSELF EVIDENCE. The grade step truncates it once the + # target list parses, so ABSENT means no target was ever resolved (a usage error, or a + # step killed that early), while PRESENT-BUT-EMPTY means targets were resolved and the + # step then stopped before recording the first outcome. Those are different failures and + # get different wording. + HAD_RESULTS=1 + [ -f "$RESULTS" ] || HAD_RESULTS=0 + [ -f "$RESULTS" ] || : > "$RESULTS" + # COUNT THE RECORDED ROWS RATHER THAN TRUSTING THE STEP OUTPUTS. Every output above is + # written by the grade step's LAST lines, so a step killed by the job timeout or dying + # mid-batch emits none of them — and branching on an empty `targets` chose the + # single-target rendering, which prints row ONE and silently discards every other target + # already recorded. The cancelled-mid-batch run is exactly the case those records exist + # for. The per-status counts are derived here too, so the heading can never disagree with + # the table under it. + ROWS="$(jq -s 'length' "$RESULTS" 2>/dev/null || echo 0)" + COUNTS="$(jq -sr '[(map(select(.status == "graded")) | length), + (map(select(.status == "ungraded")) | length), + (map(select(.status == "failed")) | length), + (map(select(.status == "skipped")) | length)] | @tsv' \ + "$RESULTS" 2>/dev/null || printf '0\t0\t0\t0')" + IFS=$'\t' read -r N_GRADED N_UNGRADED N_FAILED N_SKIPPED <<<"$COUNTS" { - echo "## PR risk grade: ${TIER:-unknown} (advisory)" - echo - if [ -s record.json ] && jq -e '.risk' record.json >/dev/null 2>&1; then - jq -r ' - "Grade = worst(path_floor, provenance, reversibility) — \(.risk.reason)\n", - "| axis | tier | reason |", "|---|---|---|", - (.risk.axes | to_entries[] | "| \(.key) | \(.value.tier // "unknown") | \(.value.reason) |"), - "\nMap `\(.risk.map_version)` · registry `\(.risk.registry_version)` · waited \($waited)s for the rollup · checks `\(.checks_state // "none")`" - ' --arg waited "${WAITED:-0}" record.json + if [ "$ROWS" -eq 0 ]; then + # No target was even attempted — a usage error (an empty or malformed target list), + # which is a different failure from an unreadable PR and must not borrow its + # wording: no PR was named, so none "could not be read". + echo "## PR risk grade: none (advisory)" + echo + if [ "$HAD_RESULTS" = 0 ]; then + echo "No pull request was graded: the run resolved no target — either none was supplied, or the grade step was cancelled before it got one. On a \`workflow_dispatch\`, supply \`pr_number\` or \`pr_numbers\` — see the step log for the exact reason." + else + echo "No pull request was graded: targets WERE resolved, but the grade step recorded no outcome for any of them — it was cancelled or died before the first target finished. The dispatched numbers are in the step log; none of them was labeled." + fi + elif [ "$ROWS" -eq 1 ] && [ "${TARGETS:-1}" = 1 ]; then + # ONE target: the per-axis table, unchanged — this is what the event path renders. + one="$(jq -sc '.[0] // {}' "$RESULTS" 2>/dev/null || echo '{}')" + rec="$(jq -r '.record // ""' <<<"$one")" + st="$(jq -r '.status // ""' <<<"$one")" + note="$(jq -r '.note // ""' <<<"$one")" + # The HEADING tier falls back to the recorded row. `tier` is a step OUTPUT, so it is + # empty on a cancelled step — and heading a row that recorded R1 as "grade: unknown" + # contradicts the table printed directly under it. + head_tier="${TIER:-}" + [ -n "$head_tier" ] || head_tier="$(jq -r '.tier // "unknown"' <<<"$one")" + if [ "$st" = failed ]; then + # A FAILED TARGET NEVER RENDERS A GRADE TABLE, even when it HAS a record. A label + # write that 403s leaves the PR carrying the previous push's grade, and heading + # that outcome "PR risk grade: R1" with the full axis table under it reads as "R1 + # was applied" — the one outcome where the label on the PR is not what the summary + # shows. The note says which input or write failed instead; reporting it as "could + # not be read via the API" would name the wrong cause. + echo "## PR risk grade: NOT APPLIED (advisory)" + echo + echo "Nothing was labeled: ${note:-see the step log for the reason}" + elif [ -n "$rec" ] && [ -s "$rec" ] && jq -e '.risk' "$rec" >/dev/null 2>&1; then + echo "## PR risk grade: ${head_tier:-unknown} (advisory)" + echo + jq -r ' + "Grade = worst(path_floor, provenance, reversibility) — \(.risk.reason)\n", + "| axis | tier | reason |", "|---|---|---|", + (.risk.axes | to_entries[] | "| \(.key) | \(.value.tier // "unknown") | \(.value.reason) |"), + "\nMap `\(.risk.map_version)` · registry `\(.risk.registry_version)` · waited \($waited)s for the rollup · checks `\(.checks_state // "none")`" + ' --arg waited "${WAITED:-0}" "$rec" + else + echo "## PR risk grade: ${head_tier:-unknown} (advisory)" + echo + echo "The PR could not be read via the API; nothing was graded (labeled ungraded — this is NOT a low-risk verdict)." + fi else - echo "The PR could not be read via the API; nothing was graded (labeled ungraded — this is NOT a low-risk verdict)." + # A BATCH has no single tier and no room for N axis tables, so it reports one row + # per target. `not attempted` is listed as its own outcome rather than folded into + # the failures: those PRs are un-graded, not mis-graded, and re-dispatching exactly + # those numbers is the fix. + echo "## PR risk grades: ${N_GRADED} graded, ${N_UNGRADED} ungraded, ${N_FAILED} failed, ${N_SKIPPED} not attempted (advisory)" + echo + # A ROW COUNT SHORT OF THE TARGET COUNT MEANS THE STEP DID NOT FINISH. Say so: the + # un-recorded numbers were never graded, and silence there reads as "all covered". + case "${TARGETS:-}" in + ''|*[!0-9]*) + echo "> The grade step never reported its own summary — it was cancelled or timed out. The rows below are what it had recorded when it stopped; anything else in the dispatched list was NOT graded." + echo ;; + *) + if [ "$TARGETS" -gt "$ROWS" ]; then + echo "> Only ${ROWS} of ${TARGETS} targets were recorded — the grade step did not run to completion. The rest were NOT graded; re-dispatch those numbers." + echo + fi ;; + esac + echo "| pr | tier | label | base ref | waited | note |" + echo "|---|---|---|---|---|---|" + jq -r '"| #\(.pr) | \(.tier // "—") | \(.label // "—") | `\(.base_ref // "—")` | \(.waited)s | \(.note // "") |"' "$RESULTS" fi echo echo "This label routes nothing and gates nothing. Disagree with the grade? Add the \`risk-dispute\` label and say why in a comment — the grader never touches that label." diff --git a/.github/workflows/test-pr-risk.yml b/.github/workflows/test-pr-risk.yml index 872b8a2..1f38a36 100644 --- a/.github/workflows/test-pr-risk.yml +++ b/.github/workflows/test-pr-risk.yml @@ -21,11 +21,15 @@ on: paths: - 'scripts/pr-risk/**' - '.github/workflows/test-pr-risk.yml' + # pr-risk.yml passes the scripts their whole input contract through env, so a change + # to it can break the suites without touching a file under scripts/pr-risk/. + - '.github/workflows/pr-risk.yml' push: branches: [main] paths: - 'scripts/pr-risk/**' - '.github/workflows/test-pr-risk.yml' + - '.github/workflows/pr-risk.yml' permissions: contents: read @@ -44,7 +48,7 @@ jobs: persist-credentials: false - name: shellcheck - run: shellcheck -x grade-pr-risk.sh apply-risk-label.sh tests/test_grade_pr_risk.sh tests/test_apply_risk_label.sh + run: shellcheck -x grade-pr-risk.sh apply-risk-label.sh grade-targets.sh tests/test_grade_pr_risk.sh tests/test_apply_risk_label.sh tests/test_grade_targets.sh - name: default map + registry parse and validate # The shipped defaults must pass the grader's own structural validation: @@ -58,3 +62,9 @@ jobs: - name: label suite run: bash tests/test_apply_risk_label.sh + + - name: targets suite + # The orchestration layer behind pr-risk.yml: base-ref resolution, the per-repo override + # contract, the settle poll, per-target failure isolation. Hermetic — `gh` is stubbed and + # every call it receives is logged, so the suite asserts on which requests were made. + run: bash tests/test_grade_targets.sh diff --git a/scripts/pr-risk/README.md b/scripts/pr-risk/README.md index 97386e1..32a11b5 100644 --- a/scripts/pr-risk/README.md +++ b/scripts/pr-risk/README.md @@ -67,6 +67,84 @@ Two CI-specific mechanics worth knowing: workflow cascade. Later phases that WANT label-triggered routing switch to an app token deliberately. +## Grading on demand (`pr_number` / `pr_numbers`) + +Every grade above is triggered by a `pull_request` event. Two things need a grade +with no event: a repo that **enrolls mid-stream** and wants the open queue it +already has labeled, and a **manual re-grade** after a `.github/risk.json` change +or on a PR carrying `risk-dispute`. Both are a `workflow_dispatch` on the +consumer's caller, forwarding a number: + +```yaml +on: + workflow_dispatch: + inputs: + pr_number: { required: false } # one PR + pr_numbers: { required: false } # 12,15,20 — wins over pr_number +``` + +With neither supplied the workflow reads the event exactly as before. The grading +logic is unchanged either way — it was always an API read keyed on a PR number, +never a read of the event payload. What the number-supplied path changes is only +which PR is read, plus three consequences worth knowing: + +- **Bot-authored and fork PRs are graded here.** The `github.actor != 'dependabot[bot]'` + and `head.repo.full_name == github.repository` clauses in the caller's `if:` + are **token** guards — a bot's or a fork's `pull_request` run gets a read-only + `GITHUB_TOKEN`, so the label write would 403 and the check would go red on + every dependency bump. On a dispatch the token is writable and the actor is a + human, so neither applies. Fork **risk** is untouched: `external` comes from + the API's own fork flag, never from the actor, and still grades R3. +- **Both clauses must be scoped to the event, or the dispatch is a silent + no-op.** There is no `github.event.pull_request` on a `workflow_dispatch`, so + an unscoped fork clause is false and the job skips with no run, no error and no + annotation. The same applies to the concurrency group: an event-only key + collapses to one constant group, and with `cancel-in-progress` a batch has each + dispatch cancel the one before it. The workflow header carries a copy-paste + block that is correct on both event shapes — use it rather than reconstructing + one, and note that a caller-side skip is undetectable from inside the reusable. +- **A low `wait_for_checks_minutes` is right for a backfill, but `0` is not.** + The wait exists to outlast the rest of the rollup while the grading run's own + check sits in it. A dispatched run's check attaches to the dispatched ref, not + to the PR's head commit, so it is not in that rollup at all and a settled PR + reads its true state on the first poll. `0` still breaks out after a single + read, ahead of the "require a settled reading to repeat" confirmation, so a + target someone pushed to minutes ago lands the honest R2 floor. `1` costs one + 15s backoff per PR and keeps the confirmation. + +A batch grades one target at a time and **one unreadable PR is reported without +abandoning the rest** — the whole list is attempted, then the run reports whether +any target failed. The list is explicit and capped (50) rather than an +`all_open: true` that would be unbounded, and a target the job's time budget +cannot reach is reported by number as *not attempted* rather than started and cut +off. The base ref is re-read **per target** and an unresolvable one fails that +target: the ref selects which branch's `.github/risk.json` judges the PR, live +PRs are commonly stacked on feature branches rather than the default branch, and +an empty `?ref=` is not an error to the contents API — it silently resolves to +the default branch. That is also why the ref is percent-encoded into the request +rather than interpolated raw: `#`, `&` and `+` are all legal in a branch name, +and a raw `#` truncates the URL into exactly that empty-ref read. A 404 for the +**ref** ("no commit found for the ref" — a deleted or renamed base branch) is +distinguished from a 404 for the **file** and fails the target instead of +falling back to the generic map. + +Two operational caveats for a backfill: + +- **A batch cannot serialize per-PR.** One run covers N pull requests and a run + belongs to exactly one concurrency group, so a batch overlapping a + `pull_request` run for one of its own numbers can interleave with it — the + label sync is a read-current / delete-stale / add-target sequence, so that PR + may briefly carry two `risk:*` labels (the next grade re-syncs it, and the + label gates nothing meanwhile). Dispatch when the queue is quiet, and use + `pr_number` when you want the per-PR group to serialize against event runs. A + DELETE of a label a concurrent run already removed is treated as removed, not + as a failure. +- **The pre-grader reads retry.** Rate limits are global, not per-PR, so the + base-ref and override reads — the first hop for every target — retry a + transient failure with backoff, as the grader already does. Without it one + secondary-rate-limit burst mid-backfill failed every remaining target at once. + A definitive answer (404, 401, 422) is never retried. + ## Per-repo overrides (read from the base ref) The shipped map and registry are deliberately generic. A consumer repo sharpens @@ -116,6 +194,13 @@ Labels are created on first use, color-coded green → red (gray for ungraded). its connection capped the list at 100, which put exactly the PRs a risk grade helps most in the ungraded lane. A read that comes back short of `changedFiles` is still `unknown`. +- `grade-targets.sh` — the orchestration layer, extracted from `pr-risk.yml`'s + inline job body so the event path and the by-number path cannot drift into two + copies of it. Per target: resolve the base ref, fetch that ref's override + files, poll the grader until the rest of the rollup settles, sync the one + label. Grades one target or a list through the same code, records each + outcome, and never lets one bad target abandon the rest. It computes nothing + about risk. - `apply-risk-label.sh` — the one write. Owns exactly the five mapped labels: removes stale ones, applies the computed one, touches nothing else. - `risk-map.v0.json` / `runbook-registry.v0.json` — the generic defaults. diff --git a/scripts/pr-risk/apply-risk-label.sh b/scripts/pr-risk/apply-risk-label.sh index e63d966..03f2c82 100755 --- a/scripts/pr-risk/apply-risk-label.sh +++ b/scripts/pr-risk/apply-risk-label.sh @@ -120,9 +120,20 @@ for l in "${OWNED[@]}"; do # push's grade for changed code. That is why the message has to name the resulting state and # the cause: the red check is the only signal, and a reader must not read the stale label as # a current verdict. - ghq api -X DELETE "repos/$REPO/issues/$PR_NUMBER/labels/$(enc "$l")" >/dev/null \ - || fail "could not remove stale label '$l' from $REPO#$PR_NUMBER: $(gherr) — '$TARGET' was NOT applied, so the PR still carries the STALE grade '$l'" - log "removed stale '$l'" + # + # A 404 IS NOT A FAILURE HERE. It means the label is already off the PR — the state this loop + # is trying to reach. The read above is a snapshot, so anything that removes the label between + # that read and this DELETE (a human, or a concurrent grading run of the same PR, which a + # batch dispatch can overlap with an event run) made it 404. Failing on that painted a red + # check and skipped the target label for a PR that was in the desired state. + if ! ghq api -X DELETE "repos/$REPO/issues/$PR_NUMBER/labels/$(enc "$l")" >/dev/null; then + case "$(gherr)" in + *"(HTTP 404)"*) log "stale '$l' was already gone (404) — treating it as removed" ;; + *) fail "could not remove stale label '$l' from $REPO#$PR_NUMBER: $(gherr) — '$TARGET' was NOT applied, so the PR still carries the STALE grade '$l'" ;; + esac + else + log "removed stale '$l'" + fi fi done diff --git a/scripts/pr-risk/grade-targets.sh b/scripts/pr-risk/grade-targets.sh new file mode 100755 index 0000000..5ddf5e9 --- /dev/null +++ b/scripts/pr-risk/grade-targets.sh @@ -0,0 +1,524 @@ +#!/usr/bin/env bash +# grade-targets.sh — grade and label N pull requests, one target at a time. The orchestration +# layer of the reusable pr-risk.yml workflow: it sequences the four per-PR steps (resolve the +# base ref, fetch that ref's override files, poll the grader until the rest of the check rollup +# settles, sync the one label) and computes nothing about risk itself. The tier comes from +# grade-pr-risk.sh; the label write is apply-risk-label.sh's. +# +# EXTRACTED from pr-risk.yml's inline job body, unchanged in behaviour, for two reasons: +# * ONE code path now serves all three ways a grade can be asked for — the `pull_request` +# event, a `workflow_dispatch` naming one PR, and a dispatched comma-separated batch. The +# alternative was a second copy of the settle-poll and the override fetch for the by-number +# path, and this repo already knows what two copies of one grader cost (grade-pr-risk.sh's +# own header records that lesson). +# * It is TESTABLE. As inline YAML the poll loop, the 404-vs-error override contract and the +# per-target failure isolation had no unit coverage at all; tests/test_grade_targets.sh now +# drives every branch of them with a stubbed `gh`. +# +# ONE UNREADABLE TARGET NEVER ABANDONS THE REST. Each target is attempted independently and its +# outcome recorded; the run's exit code reports whether ANY target failed, but only after every +# target has had its turn. A 30-PR backfill that hits one deleted PR still labels the other 29. +# +# Inputs (env): +# REPO owner/name of the repo holding the PRs (required) +# PR_NUMBERS target PR numbers, comma/space/newline separated (required) +# BASE_REF base ref for the SINGLE target, when the caller already has it from +# the event payload. Honoured ONLY when there is exactly one target; +# empty means "resolve it from the API per target". +# MAP_PATH / RB_PATH per-repo override paths, read from each target's BASE ref +# TOOL_DIR directory holding grade-pr-risk.sh + apply-risk-label.sh +# (default: beside this script) +# FLEET_LOGINS BOT_LOGINS SELF_CONTEXT SELF_RUN_ID passed through to the grader +# WAIT_MINUTES per-target settle wait (default 10) +# JOB_TIMEOUT_MINUTES the calling job's timeout-minutes (default 30) — the whole run is +# budgeted against it, not just each target. Two deadlines come out of +# it: the WAIT budget (how long targets may sleep for CI) and the JOB +# deadline (after which a new target is not STARTED at all). They are +# not the same thing — see main(). +# LABEL_MAP passed through to apply-risk-label.sh +# MAX_TARGETS refuse a list longer than this (default 50) +# RESULTS per-target JSONL outcome file (default pr-risk-results.jsonl) +# DRY_RUN 1 = grade and report, write no label +# GH_TOKEN token for gh +# GITHUB_OUTPUT when set, tier / waited / targets / graded / ungraded / failed / +# skipped are appended +# +# Exit: 0 = every target ended with a label in sync (a `risk:ungraded` label IS in sync — an +# unreadable PR is a reported verdict, not a broken run, exactly as on the event path). +# 1 = at least one target could not be graded or labeled, after all of them were tried. +# 2 = usage/setup error: nothing was attempted. +# +# Deliberately bash (shebang), not zsh — CI runners and the test suite both exercise bash. + +set -uo pipefail + +SELF_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +TOOL_DIR="${TOOL_DIR:-$SELF_DIR}" +GRADER="$TOOL_DIR/grade-pr-risk.sh" +LABELER="$TOOL_DIR/apply-risk-label.sh" + +REPO="${REPO:-}" +PR_NUMBERS="${PR_NUMBERS:-}" +BASE_REF="${BASE_REF:-}" +MAP_PATH="${MAP_PATH:-.github/risk.json}" +RB_PATH="${RB_PATH:-.github/risk-runbooks.json}" +FLEET_LOGINS="${FLEET_LOGINS:-}" +BOT_LOGINS="${BOT_LOGINS:-}" +SELF_CONTEXT="${SELF_CONTEXT:-}" +SELF_RUN_ID="${SELF_RUN_ID:-}" +WAIT_MINUTES="${WAIT_MINUTES:-10}" +JOB_TIMEOUT_MINUTES="${JOB_TIMEOUT_MINUTES:-30}" +LABEL_MAP="${LABEL_MAP:-}" +MAX_TARGETS="${MAX_TARGETS:-50}" +RESULTS="${RESULTS:-pr-risk-results.jsonl}" +DRY_RUN="${DRY_RUN:-0}" +# The retry/backoff constants are env-overridable ONLY so the suite can exercise the +# unreadable-PR, read-retry and settle-repeat branches without sleeping through the production +# backoff. CI passes none of them, so the values below are what runs in production. +MAX_UNREADABLE_TRIES="${MAX_UNREADABLE_TRIES:-4}" +READ_RETRY_BUDGET_SECONDS="${READ_RETRY_BUDGET_SECONDS:-150}" +POLL_DELAY_SECONDS="${POLL_DELAY_SECONDS:-15}" +READ_RETRY_TRIES="${READ_RETRY_TRIES:-3}" +READ_RETRY_DELAY_SECONDS="${READ_RETRY_DELAY_SECONDS:-10}" + +# Set by the per-target helpers, read by their callers. +G_TIER="" +G_WAITED=0 +TARGET_COUNT=1 +OVERALL_DEADLINE=0 +JOB_DEADLINE=0 + +log() { printf '[grade-targets] %s\n' "$*" >&2; } +die() { printf '[grade-targets] ERROR %s\n' "$*" >&2; exit 2; } + +# SCRATCH FILES AND THE EXIT TRAP ARE CREATED LAZILY, on first use, and the trap is installed only +# by a DIRECT invocation. At file scope they were side effects of merely SOURCING this file, and +# the `trap ... EXIT` replaced the sourcing shell's own EXIT trap — so a suite that sources these +# helpers to drive them directly silently lost its `rm -rf "$SANDBOX"` cleanup and leaked both the +# sandbox and these temp files. The footer's "sourceable without side effects" claim is only true +# with this deferred. +GT_DIRECT=0 +[ "${BASH_SOURCE[0]}" = "${0}" ] && GT_DIRECT=1 +ERRF="" +LABELF="" +OUTF="" +init_scratch() { + [ -z "$ERRF" ] || return 0 + ERRF="$(mktemp "${TMPDIR:-/tmp}/grade-targets-err.XXXXXX")" || die "mktemp failed" + LABELF="$(mktemp "${TMPDIR:-/tmp}/grade-targets-label.XXXXXX")" || die "mktemp failed" + OUTF="$(mktemp "${TMPDIR:-/tmp}/grade-targets-out.XXXXXX")" || die "mktemp failed" + [ "$GT_DIRECT" = 1 ] && trap 'rm -f "$ERRF" "$LABELF" "$OUTF"' EXIT + return 0 +} +gherr() { + [ -n "$ERRF" ] && [ -f "$ERRF" ] || return 0 + tr '\n' ' ' < "$ERRF" | sed 's/[[:space:]]*$//' +} + +# ---- URL building ------------------------------------------------------------------------------ +# EVERY INTERPOLATED VALUE BELOW IS A URL COMPONENT, so it is percent-encoded like one. Git branch +# names legally contain `#`, `&`, `+` and `%`, and consumer-supplied override paths can too: raw, +# a PR based on `fix/#123-thing` had its request truncated at the `#`, which arrives at the +# contents endpoint as an EMPTY `?ref=` — and an empty ref is not an error there, it silently +# resolves to the repository DEFAULT branch. That is precisely the "graded against rules nobody +# read" failure resolve_base_ref exists to prevent, reached by a different door (`&` splits off a +# bogus query param; `+` decodes to a space and 404s into the generic-default fallback). This is +# the same reason apply-risk-label.sh encodes label names before putting them in a path. +enc() { jq -rn --arg s "$1" '$s | @uri'; } +# A path keeps its separators — `/` is structural here, not data — but each SEGMENT is encoded. +enc_path() { jq -rn --arg s "$1" '$s | split("/") | map(@uri) | join("/")'; } + +# ---- transient failures on the pre-grader reads ------------------------------------------------ +# WHY THESE READS RETRY. Each target's base-ref read and its two override reads happen BEFORE the +# grader, which already retries this same failure class (rate limit, secondary rate limit, +# transient 5xx) four times with backoff, precisely so a blip does not become a durable verdict. +# Rate limits are GLOBAL rather than per-PR, so on a 50-PR backfill one secondary-rate-limit burst +# hit every remaining target at its very first hop and failed them wholesale — the inverse of the +# "one unreadable PR never abandons the rest" guarantee this file's header promises. Retrying here +# is what stops the batch's most-repeated read from being its least resilient one. +# +# A DEFINITIVE ANSWER IS NOT RETRIED. 404 (the path is absent, or no such PR), 401, 410 and 422 do +# not change on a second ask, and fetch_override needs the 404 verdict PROMPTLY to fall back to the +# shipped defaults. 403 is ambiguous — GitHub returns it both for a missing scope and for a +# secondary rate limit — so it is retried only when the message reads like a rate limit. +retryable_err() { # gh's stderr in $ERRF -> rc 0 when another attempt could plausibly differ + local msg; msg="$(gherr)" + case "$msg" in + *"rate limit"*|*"Rate limit"*|*"secondary rate"*|*"abuse detection"*) return 0 ;; + *"(HTTP 5"*|*"(HTTP 429)"*) return 0 ;; # server side / explicit throttle + *"(HTTP "*) return 1 ;; # any other status is an answer, not a blip + *) return 0 ;; # no status at all: DNS, TLS, timeout, gh itself + esac +} + +retry_read() { # -> rc 0, else gh's rc with its stderr left in $ERRF + init_scratch + local out="$1"; shift + local tries="$READ_RETRY_TRIES" attempt=1 delay="$READ_RETRY_DELAY_SECONDS" rc + while :; do + rc=0 + gh api "$@" > "$out" 2>"$ERRF" || rc=$? + [ "$rc" -eq 0 ] && return 0 + retryable_err || return "$rc" + [ "$attempt" -lt "$tries" ] || return "$rc" + # A retry may never spend the time a LATER target needs: past the job's own deadline the + # remaining targets are better reported un-attempted by number than started and cut off. + [ "$JOB_DEADLINE" -eq 0 ] || [ "$(( $(date +%s) + delay ))" -lt "$JOB_DEADLINE" ] || return "$rc" + log "read failed (attempt ${attempt}/${tries}) — retrying in ${delay}s: $(gherr)" + sleep "$delay" + attempt=$(( attempt + 1 )) + delay=$(( delay * 2 )); [ "$delay" -le 60 ] || delay=60 + done +} + +# ---- targets --------------------------------------------------------------------------------- +# A PR number is a positive integer with NO leading zeros. `007` is rejected rather than +# normalised: GitHub would resolve it to 7, so tolerating it would let `7,007` present as two +# targets for one PR and label it twice, the second write racing the first. +parse_targets() { # -> one validated, de-duplicated number per line + local raw="$1" norm n seen=" " nums=() fields=() + norm="$(tr ',\n\t' ' ' <<<"$raw")" + read -ra fields <<<"$norm" + for n in "${fields[@]}"; do + [ -n "$n" ] || continue + [[ "$n" =~ ^[1-9][0-9]*$ ]] \ + || die "bad PR number '$n' in PR_NUMBERS — want positive integers, comma-separated" + case "$seen" in *" $n "*) log "target $n listed twice — grading it once"; continue ;; esac + seen="$seen$n " + nums+=("$n") + done + [ "${#nums[@]}" -gt 0 ] \ + || die "no PR numbers to grade. On the event path PR_NUMBERS comes from the pull_request payload; on a workflow_dispatch the caller must supply pr_number or pr_numbers. Refusing to exit 0 on an empty target list — a dispatch that silently grades nothing is worse than no dispatch." + [ "${#nums[@]}" -le "$MAX_TARGETS" ] \ + || die "${#nums[@]} targets exceeds MAX_TARGETS=$MAX_TARGETS — split the backfill into batches. A list this long cannot finish inside the job's timeout, and a run cancelled mid-batch leaves an arbitrary prefix labeled with no record of where it stopped." + printf '%s\n' "${nums[@]}" +} + +# ---- the base ref ---------------------------------------------------------------------------- +# WHY AN UNRESOLVED REF IS FATAL RATHER THAN DEFAULTED. The ref is interpolated into the override +# read below as `contents/${p}?ref=${base}`, and an EMPTY ref is not an error to that endpoint — +# GitHub resolves it to the repository's DEFAULT branch. So a base ref we failed to read would +# silently fetch some other branch's .github/risk.json (or fall back to the generic default when +# the PR's real base carries an override the default branch does not) and grade the PR against +# rules nobody read. That is the same failure the non-404 guard in fetch_override exists to +# prevent, arriving by a different door. Stacked PRs make it concrete: base refs on live PRs in +# the pilot repo include feature branches, not just `main`. +resolve_base_ref() { # -> ref on stdout, rc 1 (reason already annotated on stderr) + init_scratch + local num="$1" ref + if ! retry_read "$OUTF" "repos/${REPO}/pulls/${num}" --jq '.base.ref'; then + echo "::error::could not read the base ref of ${REPO}#${num}: $(gherr). NOT grading it against the default branch's rules." >&2 + return 1 + fi + ref="$(tr -d '\n' < "$OUTF")" + case "$ref" in + ""|null) + echo "::error::the base ref of ${REPO}#${num} read back empty — refusing to fall through to the repository default branch, which would grade this PR against another branch's rules." >&2 + return 1 ;; + esac + printf '%s' "$ref" +} + +# ---- the per-repo overrides ------------------------------------------------------------------- +# Read from the target's BASE ref, so the PR being graded cannot edit the rules that judge it. +# Absent (a genuine 404) falls back to the shipped defaults; present-but-invalid is left for the +# grader's structural validation to reject loudly — a repo that commits a corrupt map must see +# red, not silent generic grading. +# +# ONLY A 404 MEANS ABSENT. Treating any non-zero exit as "no override" meant a 403 rate-limit, a +# 5xx or a network blip silently graded the PR against the generic default map instead of the +# repo's sharpened one — a LOWER tier computed from an input nobody read, which is the +# confident-answer-from-an-unread-source failure the unknown contract forbids everywhere else. So +# the status code is captured and anything that is not 200-or-404 fails the target. +# +# A 404 FROM THIS ENDPOINT IS TWO DIFFERENT ANSWERS. "the path is not in that tree" is the benign +# one this fallback is for; "no commit found for the ref" is NOT — it means the base branch was +# deleted or renamed (reachable on a by-number re-grade of an old PR), and treating it as "no +# override" grades the PR confidently against rules nobody read, the same failure the non-404 guard +# was written to stop. GitHub distinguishes them in the message body, so this does too. +fetch_override() { # -> prints the outfile, or nothing when absent + init_scratch + local p="$1" out="$2" base="$3" + if retry_read "$out" "repos/${REPO}/contents/$(enc_path "$p")?ref=$(enc "$base")" \ + -H "Accept: application/vnd.github.raw"; then + echo "using ${p} from ${base}" >&2 + printf '%s' "$out" + elif grep -qi 'no commit found for the ref' "$ERRF"; then + rm -f "$out" + echo "::error::the ref '${base}' does not resolve in ${REPO} (${p} was requested from it): $(gherr). NOT falling back to the generic default: a 404 for the REF is not a 404 for the FILE, and grading against the default branch's rules is exactly what re-reading the base ref exists to prevent." >&2 + return 1 + elif grep -q '(HTTP 404)' "$ERRF"; then + rm -f "$out" # the redirect already created it empty; nothing must read it + echo "no ${p} on ${base} — using the generic default" >&2 + else + echo "::error::could not read ${p} from ${base}: $(gherr). NOT falling back to the generic default: that would grade this PR against rules nobody read." >&2 + return 1 + fi +} + +# ---- grade one target, waiting for the rest of the rollup to settle --------------------------- +# Sets G_TIER and G_WAITED. rc 0 = there is a record to label (including the deliberate `unknown` +# one), rc 1 = the grader itself failed and nothing may be labeled. +# +# CHECKS SETTLE BEFORE THE LABEL DOES: the reversibility axis asks "did tests covering these +# lines actually run", and at event time the rest of the rollup is usually still running. The +# grading job excludes its own RUN from the rollup it reads, so what it waits out is the OTHER +# checks. On a workflow_dispatch the grading run's own check is attached to the dispatched ref, +# not to the PR's head commit, so there is nothing of ours in that rollup to wait out and a +# settled PR reads its true state on the first poll — which is what makes a LOW wait the right +# setting for a backfill, and why a low wait there does not reproduce the R2 floors that +# `wait_for_checks_minutes: 0` produces on the event path. +settle_grade() { # + local num="$1" record="$2" map_override="$3" rb_override="$4" deadline="$5" + local args=( --repo "$REPO" --pr "$num" ) + [ -z "$SELF_CONTEXT" ] || args+=( --self-context "$SELF_CONTEXT" ) + [ -z "$SELF_RUN_ID" ] || args+=( --self-run-id "$SELF_RUN_ID" ) + [ -z "$FLEET_LOGINS" ] || args+=( --fleet-logins "$FLEET_LOGINS" ) + [ -z "$BOT_LOGINS" ] || args+=( --bot-logins "$BOT_LOGINS" ) + [ -z "$map_override" ] || args+=( --map "$map_override" ) + [ -z "$rb_override" ] || args+=( --runbooks "$rb_override" ) + + local empty_deadline pending state now delay nap settled_once rc + # Grace for checks to REGISTER at all. + empty_deadline=$(( $(date +%s) + 120 )) + G_WAITED=0 + # Poll backoff: 15s doubling to 120s. The fixed 30s poll idled a runner up to the full wait on + # every `synchronize` event, per PR, across the fleet, for a grade that is one API call — + # increasing backoff cuts both runner minutes and API calls without changing settle behaviour. + delay="$POLL_DELAY_SECONDS" + settled_once=0 + # rc=3 (the PR could not be read) covers rate limits, secondary rate limits and transient 5xx + # as well as a genuinely missing PR. Labeling `ungraded` on the first blip turns a momentary + # hiccup into a durable verdict, so it is retried with backoff. This gets its OWN budget, not + # the settle deadline: a transient API failure has nothing to do with how long the caller wants + # to wait for CI, and sharing the deadline would leave `wait_for_checks_minutes: 0` with no + # retry at all. It IS capped by the JOB's deadline — not by the wait budget, which a batch may + # legitimately outlive while still grading — so one dead PR cannot get the run cancelled. + local unreadable_tries=0 max_unreadable_tries="$MAX_UNREADABLE_TRIES" read_deadline read_delay=10 + read_deadline=$(( $(date +%s) + READ_RETRY_BUDGET_SECONDS )) + [ "$JOB_DEADLINE" -eq 0 ] || [ "$read_deadline" -le "$JOB_DEADLINE" ] || read_deadline="$JOB_DEADLINE" + while :; do + rc=0 + bash "$GRADER" "${args[@]}" > "$record" || rc=$? + case "$rc" in + 0|1) ;; # graded (1 = graded but unknown — still labeled) + 3) unreadable_tries=$(( unreadable_tries + 1 )) + if [ "$unreadable_tries" -lt "$max_unreadable_tries" ] && [ "$(date +%s)" -lt "$read_deadline" ]; then + log "PR read failed (attempt ${unreadable_tries}/${max_unreadable_tries}) — retrying in ${read_delay}s" + sleep "$read_delay"; G_WAITED=$(( G_WAITED + read_delay )) + read_delay=$(( read_delay * 2 )); [ "$read_delay" -le 60 ] || read_delay=60 + continue + fi + log "PR unreadable via the API after ${unreadable_tries} attempt(s) — labeling ungraded" + printf '{}' > "$record" + break ;; + *) log "grader failed for ${REPO}#${num} (rc=$rc)"; G_TIER=""; return 1 ;; # setup/usage bug + esac + pending="$(jq -r '.checks_pending_excl_self // false' "$record")" + state="$(jq -r '.checks_state // "none"' "$record")" + now="$(date +%s)" + [ "$now" -lt "$deadline" ] || break + if [ "$pending" = "true" ]; then + settled_once=0 + elif [ "$state" = "none" ] && [ "$now" -lt "$empty_deadline" ]; then + # An EMPTY rollup is checks that have not registered yet, not CI that finished — exiting + # the poll here would apply a label that a check appearing a second later never revises. + # Bounded by a SHORT grace window rather than the whole budget, so a repo with genuinely no + # CI is not made to wait 25 minutes on every PR to arrive at the same honest R2. + settled_once=0 + elif [ "$settled_once" -eq 0 ]; then + # First settled reading. Require it to REPEAT before labeling: one transiently-quiet poll + # is not a settled rollup. + settled_once=1 + else + break + fi + # CLAMP THE SLEEP TO WHAT REMAINS OF THE DEADLINE. The loop tests `now < deadline` and then + # slept a full 15/30/60/120s, so a continuously-pending rollup overran the caller's wait by up + # to 105s per target — and in a batch that overrun is charged to the LATER targets' budget, + # pushing them into the un-attempted lane for time this target had no right to spend. The + # backoff itself keeps doubling; only the nap is shortened. + now="$(date +%s)" + nap="$delay" + [ "$(( now + nap ))" -le "$deadline" ] || nap=$(( deadline - now )) + [ "$nap" -gt 0 ] || break + sleep "$nap"; G_WAITED=$(( G_WAITED + nap )) + delay=$(( delay * 2 )); [ "$delay" -le 120 ] || delay=120 + done + G_TIER="$(jq -r '.risk.tier // "unknown"' "$record" 2>/dev/null)" + # AN EMPTY TIER IS THE UNKNOWN LANE, NEVER A GRADED ONE. A record that is empty or not JSON at + # all makes jq print nothing and exit 0, and apply-risk-label.sh maps an empty TIER to + # `risk:ungraded` — so leaving G_TIER empty labeled the PR ungraded while this file recorded + # `status: graded` with a null tier and the batch counters credited it as graded. + [ -n "$G_TIER" ] || G_TIER=unknown + return 0 +} + +# ---- one target, end to end ------------------------------------------------------------------- +record_result() { #