From 415c6d7fdaf8b01d2057e75bf9d596ba49a13730 Mon Sep 17 00:00:00 2001 From: huang47 <157390+huang47@users.noreply.github.com> Date: Mon, 31 Aug 2026 21:36:19 -0700 Subject: [PATCH 1/5] feat(pr-risk): use canonical named tiers --- scripts/pr-risk/grade-pr-risk.sh | 135 ++++++++------ scripts/pr-risk/grade-targets.sh | 6 +- scripts/pr-risk/risk-map.v0.json | 57 +++--- scripts/pr-risk/runbook-registry.v0.json | 4 +- scripts/pr-risk/tests/test_grade_pr_risk.sh | 195 +++++++++++--------- scripts/pr-risk/tests/test_grade_targets.sh | 30 +-- 6 files changed, 235 insertions(+), 192 deletions(-) diff --git a/scripts/pr-risk/grade-pr-risk.sh b/scripts/pr-risk/grade-pr-risk.sh index fe2d0a0c..cbfaf0dd 100755 --- a/scripts/pr-risk/grade-pr-risk.sh +++ b/scripts/pr-risk/grade-pr-risk.sh @@ -1,7 +1,7 @@ #!/usr/bin/env bash # grade-pr-risk.sh — deterministic PR risk grader for CI (the reusable pr-risk.yml workflow). # -# Grades ONE pull request into a risk tier R0 (safest) .. R3 (riskiest), or refuses with +# Grades ONE pull request into a risk tier low (safest) .. xhigh (riskiest), or refuses with # `unknown` when an input could not be read. The tier is advisory: this script only COMPUTES; # the workflow around it leaves the label. Nothing here gates, blocks, comments, or merges. # @@ -30,7 +30,7 @@ # is the WORST tier over every rule any changed path matches. # AXIS 2 — PROVENANCE. runbook / agent-supervised / human / external. A PR whose identity # matches a runbook but whose DIFF SHAPE does not is not a runbook, and falls back to its -# underlying class. `external` (any fork, or a first-time HUMAN contributor) is R3 on +# underlying class. `external` (any fork, or a first-time HUMAN contributor) is xhigh on # provenance alone; a non-fork bot is a runbook candidate, not an outsider, because every # GitHub App authors with `author_association: NONE`. # AXIS 3 — REVERSIBILITY. Single clean revert? Mutates persistent state or deletes data? @@ -101,24 +101,48 @@ USAGE } # ---- the map --------------------------------------------------------------------------------- -# An unreadable map is fatal — grading against one would grade every PR R0. VALID JSON IS NOT +# An unreadable map is fatal — grading against one would grade every PR low. VALID JSON IS NOT # ENOUGH: `{}` parses, so a syntactically-valid but STRUCTURALLY EMPTY map used to sail through -# upstream — `default_tier` fell back to R0, no path rule matched anything, and every PR graded -# R0. So the shape is checked too, and every tier STRING in the file is checked against the tier +# upstream — `default_tier` fell back to low, no path rule matched anything, and every PR graded +# low. So the shape is checked too, and every tier STRING in the file is checked against the tier # enum here rather than being tolerated downstream: `tier_rank` cannot rank a tier it does not # know, and a fail-safe rank is a worse answer than a refusal at load time. read_map() { # -> JSON on stdout, rc 0; rc 1 + reason on stderr - local f="$1" kind="${2:-map}" raw shape + local f="$1" kind="${2:-map}" raw shape legacy normalize [ -f "$f" ] || { echo "$kind $f not found" >&2; return 1; } raw="$(cat "$f" 2>/dev/null)" || { echo "cannot read $f" >&2; return 1; } jq -e . >/dev/null 2>&1 <<<"$raw" || { echo "$kind is not valid JSON" >&2; return 1; } if [ "$kind" = map ]; then + legacy="$(jq -r '[.. | strings | select(IN("R0","R1","R2","R3"))] | unique | join(", ")' <<<"$raw")" + if [ -n "$legacy" ]; then + warn "risk map uses deprecated tier aliases ($legacy); use low, medium, high, xhigh" + fi + # Normalize once at the trust boundary. Everything downstream sees only the canonical names, + # while existing consumer maps keep working until they are migrated. + normalize=' + def canonical: + if . == "R0" then "low" elif . == "R1" then "medium" + elif . == "R2" then "high" elif . == "R3" then "xhigh" else . end; + if has("tiers") and (.tiers | type) == "array" then .tiers |= map(canonical) else . end + | if has("default_tier") then .default_tier |= canonical else . end + | if (.path_rules | type) == "array" + then .path_rules |= map(if has("tier") then .tier |= canonical else . end) else . end + | if (.provenance_tiers | type) == "object" + then .provenance_tiers |= with_entries( + if (.key | startswith("_")) then . else .value |= canonical end) else . end + | if (.reversibility | type) == "object" + then .reversibility |= ( + if has("no_green_checks_tier") then .no_green_checks_tier |= canonical else . end + | if has("no_test_touched_tier") then .no_test_touched_tier |= canonical else . end + | if has("clean_tier") then .clean_tier |= canonical else . end) + else . end' + raw="$(jq -c "$normalize" <<<"$raw")" || { echo "could not normalize $kind" >&2; return 1; } # shellcheck disable=SC2016 # jq program: $vars belong to jq shape=' - def known: ["R0","R1","R2","R3"]; + def known: ["low","medium","high","xhigh"]; if type != "object" then "not a JSON object" elif (.path_rules | type) != "array" then "path_rules is missing or not an array" - elif (.path_rules | length) == 0 then "path_rules is EMPTY — an empty rule set would grade every PR R0" + elif (.path_rules | length) == 0 then "path_rules is EMPTY — an empty rule set would grade every PR low" elif ([.path_rules[] | select((.class | type) != "string" or (.paths | type) != "array" or (.paths | length) == 0)] | length) > 0 then "a path rule is missing a class or a non-empty paths list" elif ([.path_rules[] | .tier | select(IN(known[]) | not)] | length) > 0 @@ -129,14 +153,14 @@ read_map() { # -> JSON on stdout, rc 0; rc 1 + reaso # EVERY provenance class must be MAPPED, not just well-typed. Checking only the values let # a map that OMITS `external` pass, and the lookup then fell back to a tier of its own # choosing — silently retiring the "external (a fork, or a first-time human contributor) is - # R3, no exceptions" invariant that the default map comment and the README promise. A class + # xhigh, no exceptions" invariant that the default map comment and the README promise. A class # nobody mapped is a routing decision nobody made, so it is refused at load time. # (No apostrophes in here: the whole shape program is a single-quoted shell string.) elif ((["runbook","agent-supervised","human","external"] - [.provenance_tiers | keys[]]) | length) > 0 then "provenance_tiers is missing a class: \(["runbook","agent-supervised","human","external"] - [.provenance_tiers | keys[]]) — an unmapped class would be graded off a tier nobody chose" elif ((.reversibility // {}) | has("test_path_patterns")) and (((.reversibility // {}).test_path_patterns | type) != "array") then "reversibility.test_path_patterns is present but not an array" - elif (.default_tier // "R0") as $d | ($d | IN(known[])) | not then "default_tier is outside \(known)" + elif (.default_tier // "low") as $d | ($d | IN(known[])) | not then "default_tier is outside \(known)" elif [(.reversibility // {}) | .no_green_checks_tier, .no_test_touched_tier, .clean_tier | select(. != null and (IN(known[]) | not))] | length > 0 then "a reversibility tier is outside \(known)" else empty end' @@ -194,27 +218,27 @@ cat <<'JQ' def matches_any($globs): . as $p | any($globs[]?; . as $g | ($p | test($g | glob2re))); # An UNRECOGNIZED tier ranks as the RISKIEST, never the safest. read_map already refuses a # map carrying one, so this is defence in depth — but the direction matters: defaulting to - # R0 would let a typo'd or future tier silently DOWNGRADE a PR's grade, which inverts the + # low would let a typo'd or future tier silently DOWNGRADE a PR's grade, which inverts the # "unknown is never safe" contract in the one place it decides routing. - def tier_rank: {"R0":0,"R1":1,"R2":2,"R3":3}[.] // 3; + def tier_rank: {"low":0,"medium":1,"high":2,"xhigh":3}[.] // 3; def worst($a; $b): if ($a | tier_rank) >= ($b | tier_rank) then $a else $b end; $map as $M | $rb as $RB | ($fleet | logins) as $fleetl | ($bots | logins) as $botl - | ($M.default_tier // "R0") as $DEF + | ($M.default_tier // "low") as $DEF | . as $r | (.changed_paths) as $paths | ([$paths[]? | .path]) as $plist # EVERY path the diff touches, DESTINATION *and* ORIGIN. A RENAMED file is recorded under its # destination only, so matching `.path` alone let a rename out of a sensitive directory escape # the floor entirely: move `.github/workflows/deploy.yml` or an `auth/` file to an innocuous - # name and the R3 rule that guards it never matches. The origin path is part of what the PR + # name and the xhigh rule that guards it never matches. The origin path is part of what the PR # did, so it is graded too. | ([$paths[]? | .path, (.previous_path // empty)] | unique) as $pall # ---- AXIS 1: PATH FLOOR --------------------------------------------------------------- - # WORST over every rule any changed path matches. An R0 rule (docs, tests) can never cancel - # an R3 rule (migrations) in the same PR — that is why this is a max, not a last-match-wins. + # WORST over every rule any changed path matches. A low rule (docs, tests) can never cancel + # an xhigh rule (migrations) in the same PR — that is why this is a max, not a last-match-wins. | (if $r.changed_paths_status != "ok" or $paths == null then {tier:null, status:"unknown", reason:("changed-path list is " + ($r.changed_paths_status // "absent") + " — a PR whose files we cannot read is exactly the PR that might touch auth"), @@ -223,7 +247,7 @@ cat <<'JQ' ([$M.path_rules[]? | . as $rule | select($pall | any(. as $p | $p | matches_any($rule.paths)))]) as $hit # PER-FILE FLOORS, for REPORTING ONLY. The same rules, matched one file at a time, so the # publish surfaces can say WHICH files put the floor where it is ("94% of this diff is - # R0/R1; the 6% that makes it R3 is these two files") instead of printing an opaque tier. + # low/medium; the 6% that makes it xhigh is these two files") instead of printing an opaque tier. # This CANNOT move the grade: `worst` over these per-file floors is by construction the # same value as `worst` over every matched rule, because each file's floor already starts # at $DEF and every matched rule is some file's rule. tests/test_grade_pr_risk.sh pins @@ -273,7 +297,7 @@ cat <<'JQ' # # NO STATUS TWIN HERE, deliberately, and it is the only provenance input without one. The # twins exist where the un-collected default would ANSWER — reading an absent `is_fork` as - # "not a fork" retires `external => R3`. This default runs the other way: absent means "not + # "not a fork" retires `external => xhigh`. This default runs the other way: absent means "not # known to be a bot", which sends the author BACK through the association test and grades it # the stricter way, exactly as a pre-schema-4 record graded before. Nothing is retired, so # there is nothing to refuse to answer. A schema-3 corpus row therefore still grades an App @@ -288,19 +312,19 @@ cat <<'JQ' | ($r.labels_status // "ok") as $lbst # `external` is decided from is_fork + author_association, and those arrive with a STATUS # twin — whether they were actually read has to be asked before they are believed. Reading - # an un-collected `is_fork` as "not a fork" would make `external => R3` — the one provenance + # an un-collected `is_fork` as "not a fork" would make `external => xhigh` — the one provenance # class never routed unattended — silently unreachable. Unread is `unknown`, and `unknown` # refuses to grade the axis. | ($r.provenance_status // (if ($r | has("is_fork")) then "ok" else "absent" end)) as $pvst # ORDER IS THE RULE HERE, AND IT IS NOT REARRANGEABLE. The fork test runs FIRST and stays # unconditional: a fork PR is `external` no matter who authored it, so a bot on a fork can - # never escape R3 by presenting a bot login. Only AFTER that does the author-association half + # never escape xhigh by presenting a bot login. Only AFTER that does the author-association half # get narrowed to authors the shared resolver does NOT read as a bot. # # WHY THE NARROWING. A GitHub App is never an org member, so EVERY PR opened by a repo-owned # App arrives with `author_association: NONE` — and testing that string before the login - # classification pinned every such PR `external` => R3 regardless of what it changed (measured - # on one consumer: 14 of 19 R3 grades in a 24-PR sample came from this, not from the diff). + # classification pinned every such PR `external` => xhigh regardless of what it changed (measured + # on one consumer: 14 of 19 xhigh grades in a 24-PR sample came from this, not from the diff). # It was also unfixable from the consumer side, because both levers sit further down: the # `bot_logins` input feeds `classify_login`, and `.github/risk-runbooks.json` is read by the # shape assertion below. A non-fork bot now falls through to `runbook-candidate`, where the @@ -323,9 +347,9 @@ cat <<'JQ' # assertion below resolves `$rbk != null` to `runbook` ahead of `$base_class` either way. # What it fixes is the UNREGISTERED bot, which either half would classify `agent-supervised` # — contradicting this axis's stated promise (and the map's `_why`) that a non-fork bot with - # no asserting entry falls back to `human`. The default map tiers both R1, so nothing moves + # no asserting entry falls back to `human`. The default map tiers both medium, so nothing moves # there; the two classes exist so a CONSUMER map can tier them apart, and a consumer that - # trusts its own supervised agents at R0 would otherwise hand R0 to any bot PR someone + # trusts its own supervised agents at low would otherwise hand low to any bot PR someone # labelled `agent-coded`. Trust in this axis is earned by the shape assertion, never by a label. | (if $pvst != "ok" or $lbst != "ok" then "unknown" elif ($r.is_fork // false) then "external" @@ -385,16 +409,16 @@ cat <<'JQ' | (if $prov == "unknown" or $author == null then {tier:null, status:"unknown", reason:(if $pvst != "ok" - then "fork / author-association were not collected (\($pvst)) — the `external` provenance class is un-decidable, and defaulting it to 'not a fork' would silently retire the external => R3 rule" + then "fork / author-association were not collected (\($pvst)) — the `external` provenance class is un-decidable, and defaulting it to 'not a fork' would silently retire the external => xhigh rule" elif $lbst != "ok" then "the PR label list is \($lbst) — `agent-coded` cannot be read off a truncated list, and reading it as absent would be a confident answer from a source nobody finished reading" else "PR author did not resolve to a GitHub account — provenance is unattributable" end), provenance:null} - # An UNMAPPED class falls back to the RISKIEST tier, never R1 — same direction as + # An UNMAPPED class falls back to the RISKIEST tier, never medium — same direction as # tier_rank. read_map now REQUIRES all four classes, so this is defence in depth; the - # direction is what matters, because defaulting to R1 is how a map that omitted `external` + # direction is what matters, because defaulting to medium is how a map that omitted `external` # used to grade a fork the same as a teammate. - else {tier: (($M.provenance_tiers // {})[$prov] // "R3"), status:"ok", + else {tier: (($M.provenance_tiers // {})[$prov] // "xhigh"), status:"ok", provenance: $prov, runbook: (if $rbk == null then null else $rbk.id end), runbook_lane: (if $rbk == null then null else $rbk.lane end), @@ -406,17 +430,17 @@ cat <<'JQ' # ---- AXIS 3: REVERSIBILITY ------------------------------------------------------------ # Four questions, answered deterministically and in worsening order: - # mutates persistent state / deletes data? -> R3 (reverting code does not restore state) - # deletes a file under a sensitive class? -> R3 (not a single clean revert) - # did tests covering these lines actually run? no green rollup -> R2; green but no test - # file touched -> R1; green and a test touched -> R0. + # mutates persistent state / deletes data? -> xhigh (reverting code does not restore state) + # deletes a file under a sensitive class? -> xhigh (not a single clean revert) + # did tests covering these lines actually run? no green rollup -> high; green but no test + # file touched -> medium; green and a test touched -> low. # `flag_gated` is RECORDED but never LOWERS a tier — an axis may only move riskier. So is # `files`, which names the changed paths that supplied the tier on the two rungs where it is # attributable to specific files at all (see the attribution block below). | ($M.reversibility // {}) as $RV # Was the check rollup READ? `checks_status: ok` with a null `checks_state` is GitHub # genuinely reporting no rollup for this head (a repo with no CI) and IS gradeable — the - # honest R2. A rollup that was never collected is not: reading it as "no green rollup" + # honest high. A rollup that was never collected is not: reading it as "no green rollup" # would be a confident answer computed from a source nobody read. | ($r.checks_status // (if ($r | has("checks_state")) then "ok" else "absent" end)) as $ckst | (if $r.changed_paths_status != "ok" or $paths == null @@ -433,7 +457,7 @@ cat <<'JQ' # Sensitive-class match over the DELETED paths ONLY. This was computed from $A1.classes — # the classes matched by ANY changed file — so a PR that merely MODIFIED an auth file # while deleting an unrelated README reported "deletes N file(s) under a sensitive class" - # and pinned reversibility R3. The two sets have to be the same set for the sentence the + # and pinned reversibility xhigh. The two sets have to be the same set for the sentence the # reason string prints to be true. # The delete-sensitive RULES, resolved once. The reason sentence, the $del_sensitive # test and the attribution below all read this same set, so the three can never @@ -453,7 +477,7 @@ cat <<'JQ' # the built-in regex when it does not. Hardcoding it meant a consumer could not fix it # with .github/risk.json — the one lever the workflow gives them — and the regex misses # `test_*.py`, `*_test.py`, `*Test.java` and `*_spec.rb`, so whole ecosystems could never - # reach clean_tier and sat at R1 forever. + # reach clean_tier and sat at medium forever. # DESTINATION paths only ($plist, not $pall): renaming `x_test.go` to `x.go` REMOVES a # test, and matching the origin path would read that as "a test file changed" and let it # reach clean_tier. The path floor uses $pall because widening it there can only ever @@ -468,15 +492,15 @@ cat <<'JQ' # `files` list that disagrees with the `reason` printed beside it is worse than no # list at all. `k` feeds nothing but the attribution; it never reaches `$d.t`. | (if ($irrev | length) > 0 - then {k:"irreversible-class", t:"R3", why:("touches " + ($irrev|join(", ")) + " — mutates persistent state or deletes data; reverting the code does not restore it")} + then {k:"irreversible-class", t:"xhigh", why:("touches " + ($irrev|join(", ")) + " — mutates persistent state or deletes data; reverting the code does not restore it")} elif (($deleted | length) > 0 and $del_sensitive) - then {k:"delete-sensitive", t:"R3", why:("removes " + ($del_sens_paths|length|tostring) + " file(s) under a sensitive class (" + ($del_classes|join(", ")) + ") — not a single clean revert")} + then {k:"delete-sensitive", t:"xhigh", why:("removes " + ($del_sens_paths|length|tostring) + " file(s) under a sensitive class (" + ($del_classes|join(", ")) + ") — not a single clean revert")} elif $checks == null or $checks != "SUCCESS" - then {k:"no-green-checks", t: ($RV.no_green_checks_tier // "R2"), + then {k:"no-green-checks", t: ($RV.no_green_checks_tier // "high"), why:("no GREEN check rollup (" + ($checks // "absent") + ") — cannot answer whether tests covering these lines actually ran")} elif ($touched_test | not) - then {k:"no-test-touched", t: ($RV.no_test_touched_tier // "R1"), why:"checks green but the diff touches no test file — nothing proves the suite covers THESE lines"} - else {k:"clean", t: ($RV.clean_tier // "R0"), why:"single clean revert, no persistent-state mutation, checks green, tests touched"} end) as $d + then {k:"no-test-touched", t: ($RV.no_test_touched_tier // "medium"), why:"checks green but the diff touches no test file — nothing proves the suite covers THESE lines"} + else {k:"clean", t: ($RV.clean_tier // "low"), why:"single clean revert, no persistent-state mutation, checks green, tests touched"} end) as $d # WHICH FILES SUPPLIED THIS TIER — REPORTING ONLY, exactly like the per-file path # floors above, and derived AFTER `$d` for the same reason: it is computed FROM the # decision and read nowhere else in the grader, so it cannot become a second input to @@ -486,8 +510,8 @@ cat <<'JQ' # The publisher (BE-7414) uses it to ask "if the top path-floor files were peeled off # this PR, would the reversibility reason go with them?" — so it is populated ONLY on # the two rungs that are attributable to specific files, and `null` on the other - # three. `null` is not "no files": R2 ("no green rollup") is a property of the head - # COMMIT and R1 ("no test touched") of the WHOLE change set, and neither is removable + # three. `null` is not "no files": high ("no green rollup") is a property of the head + # COMMIT and medium ("no test touched") of the WHOLE change set, and neither is removable # by dropping files — so `null` reads as "not attributable, keep suppressing", which # an empty array would not. # @@ -499,10 +523,10 @@ cat <<'JQ' # is decided first-match, but a PR can trip the irreversible-class rung AND remove a # file under a sensitive class. Naming only the migrations there would answer the # publisher's peel question "yes, the reversibility reason goes with these files" while - # the delete-sensitive rung silently held the axis at R3 — the conservative suppression + # the delete-sensitive rung silently held the axis at xhigh — the conservative suppression # the `null` rungs exist for, turned into a false positive. Unioning is what makes the # subset test sound: peeling every path in `files` clears EVERY attributable rung, so - # the axis provably lands below R3. tests/test_grade_pr_risk.sh pins that property + # the axis provably lands below xhigh. tests/test_grade_pr_risk.sh pins that property # directly (peel the attributed paths, re-grade, assert the tier moved) rather than # only the shape of the list. # @@ -533,7 +557,7 @@ cat <<'JQ' # a SAFE answer to the publisher's peel question rather than merely a true one. Peeling # clears both attributable rungs, so the remainder restarts the ladder at `no-green-checks` # — but those three lower rungs are MAP-CONFIGURABLE, and a consumer that sets - # `no_green_checks_tier: "R3"` lands the remainder right back on R3. Without this the + # `no_green_checks_tier: "xhigh"` lands the remainder right back on xhigh. Without this the # publisher reads "every attributed path is in the peel set" as "the tier drops" and # promises a reduction a legal map makes impossible. # @@ -550,8 +574,8 @@ cat <<'JQ' # against `!= "SUCCESS"` and is kept only so the two spellings stay byte-identical) — a # paraphrase here is a second copy of the rung that can drift out of step with it. | (if $rev_files == null then null - elif $checks == null or $checks != "SUCCESS" then ($RV.no_green_checks_tier // "R2") - else worst(($RV.no_test_touched_tier // "R1"); ($RV.clean_tier // "R0")) end) as $rev_residual + elif $checks == null or $checks != "SUCCESS" then ($RV.no_green_checks_tier // "high") + else worst(($RV.no_test_touched_tier // "medium"); ($RV.clean_tier // "low")) end) as $rev_residual | {tier:$d.t, status:"ok", reason:$d.why, files:$rev_files, residual_tier:$rev_residual, flag_gated:$flag, deleted_files:($deleted|length)} end) as $A3 @@ -566,7 +590,7 @@ cat <<'JQ' registry_version: ($RB.registry_version // "unknown"), graded_at: $now, tier: (if ($unk|length) > 0 then null - else (reduce [$A1.tier, $A2.tier, $A3.tier][] as $t ("R0"; worst(.; $t))) end), + else (reduce [$A1.tier, $A2.tier, $A3.tier][] as $t ("low"; worst(.; $t))) end), status: (if ($unk|length) > 0 then "unknown" else "ok" end), reason: (if ($unk|length) > 0 then "unknown: " + ([$unk[] | .reason] | join(" | ")) @@ -589,7 +613,7 @@ grade_stream() { # # THE GRADING JOB IS PART OF THE ROLLUP IT READS. When this script runs inside a workflow on # the PR it is grading, its own check run is in progress, so the raw statusCheckRollup.state -# can never be SUCCESS at grade time — every CI-time grade would floor at R2 and the tiers +# can never be SUCCESS at grade time — every CI-time grade would floor at high and the tiers # would be an artifact of the measurement. With --self-run-id (preferred) or --self-context # the rollup is therefore recomputed from the individual contexts, EXCLUDING our own check # runs: any remaining pending => PENDING, any remaining SUCCESS => SUCCESS, remaining contexts @@ -603,13 +627,13 @@ grade_stream() { # own PENDING; it must never be able to hide a red check. Matching on the workflow NAME # dropped every sibling job of a consumer that put the grading job inside its existing CI # workflow, so a FAILED test job vanished and the remaining green contexts aggregated to -# SUCCESS — reversibility then graded R0/R1 on a red PR. +# SUCCESS — reversibility then graded low/medium on a red PR. # * SUCCESS requires at least one context that actually CONCLUDED SUCCESS. A rollup of # nothing but SKIPPED / NEUTRAL / null establishes nothing about whether tests ran, which -# is the axis's whole question, so it aggregates to NEUTRAL and floors at R2. +# is the axis's whole question, so it aggregates to NEUTRAL and floors at high. # --self-run-id also makes the match EXACT (github.run_id), so a same-named workflow in the # consumer repo is no longer excluded. A caller that still embeds the grading job inside a -# multi-job workflow has all of that run's siblings excluded and lands on the honest R2 floor +# multi-job workflow has all of that run's siblings excluded and lands on the honest high floor # rather than a false green — enroll pr-risk as its OWN workflow to grade off a full rollup. fetch_pr_record() { # -> record JSON on stdout, rc 1 on an unreadable PR # Token note: the checkSuite -> workflowRun traversal below is an ACTIONS resource, so the @@ -623,7 +647,7 @@ fetch_pr_record() { # -> record JSON on stdout, rc 1 on an unreadab # and the half-record never reaches jq. That matters: if a nulled `workflowRun` DID reach jq, # `is_self`'s `(.checkSuite.workflowRun.databaseId // -1)` fallback would match nothing, # self-exclusion would silently stop working, our own in-progress run would read PENDING - # forever, and the PR would land a confident-looking R2 floor after burning the whole wait + # forever, and the PR would land a confident-looking high floor after burning the whole wait # budget. Rejecting the read outright is what keeps that from being reachable — so do NOT # "recover" partial data here by dropping the rc check. local repo="$1" num="$2" q qctx ctxsel resp files fstatus errf @@ -846,11 +870,12 @@ main() { # The REASON is captured alongside the value in ONE call each: re-running read_map just to # collect its stderr would read the file twice, and the two reads could disagree. An unusable - # map is fatal — grading against one would grade every PR R0. + # map is fatal — grading against one would grade every PR low. local map rb errf errf="$(mktemp "${TMPDIR:-/tmp}/grade-pr-risk-err.XXXXXX")" || die "mktemp failed" map="$(read_map "$RISK_MAP" map 2>"$errf")" \ - || die "risk map unusable ($RISK_MAP): $(tr '\n' ' ' < "$errf")— refusing to grade: an unusable map would grade every PR R0" + || die "risk map unusable ($RISK_MAP): $(tr '\n' ' ' < "$errf")— refusing to grade: an unusable map would grade every PR low" + [ ! -s "$errf" ] || { cat "$errf" >&2; : > "$errf"; } rb="$(read_map "$RUNBOOKS" runbooks 2>"$errf")" \ || die "runbook registry unusable ($RUNBOOKS): $(tr '\n' ' ' < "$errf")— refusing to grade" rm -f "$errf" diff --git a/scripts/pr-risk/grade-targets.sh b/scripts/pr-risk/grade-targets.sh index b3223f1a..96694b7b 100755 --- a/scripts/pr-risk/grade-targets.sh +++ b/scripts/pr-risk/grade-targets.sh @@ -286,7 +286,7 @@ fetch_override() { # -> prints the outfile, or nothi # 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 +# setting for a backfill, and why a low wait there does not reproduce the high 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" @@ -344,7 +344,7 @@ settle_grade() { # <paths-json> <paths-status> <checks> [head_ref] grade() { bash "$GRADER" --stdin 2>/dev/null; } echo "— phase 1: worst-wins on the path floor —" -# docs (R0 rule) + migration (R3 rule) in one PR: the R0 rule must not cancel the R3 one. +# docs (low rule) + migration (xhigh rule) in one PR: the low rule must not cancel the xhigh one. out="$(rec 1 dev 'fix: tweak' '[{"path":"README.md","additions":1,"deletions":0,"change_type":"MODIFIED"},{"path":"db/migrations/0001_x.sql","additions":9,"deletions":0,"change_type":"ADDED"}]' ok SUCCESS | grade)" -eq "docs cannot cancel migrations" R3 "$(jq -r '.risk.axes.path_floor.tier' <<<"$out")" -eq "overall is R3" R3 "$(jq -r '.risk.tier' <<<"$out")" +eq "docs cannot cancel migrations" xhigh "$(jq -r '.risk.axes.path_floor.tier' <<<"$out")" +eq "overall is xhigh" xhigh "$(jq -r '.risk.tier' <<<"$out")" echo "— phase 2: a runbook cannot buy its way past the path floor —" out="$(rec 2 'dependabot[bot]' 'chore(deps): bump x from 1 to 2' '[{"path":"go.mod","additions":1,"deletions":1,"change_type":"MODIFIED"},{"path":"go.sum","additions":2,"deletions":2,"change_type":"MODIFIED"}]' ok SUCCESS 'dependabot/go_modules/x-2' CONTRIBUTOR | grade)" eq "provenance is runbook" runbook "$(jq -r '.risk.axes.provenance.provenance' <<<"$out")" -eq "provenance proposes R0" R0 "$(jq -r '.risk.axes.provenance.tier' <<<"$out")" -eq "path floor still decides R3" R3 "$(jq -r '.risk.tier' <<<"$out")" +eq "provenance proposes low" low "$(jq -r '.risk.axes.provenance.tier' <<<"$out")" +eq "path floor still decides xhigh" xhigh "$(jq -r '.risk.tier' <<<"$out")" echo "— phase 3: provenance alone is never sufficient (shape assertion) —" # dependabot's identity, but the diff touches a path outside its permitted set. @@ -73,7 +73,7 @@ if [ "$sf" -ge 1 ]; then ok "shape failure recorded"; else bad "shape failure re echo "— phase 4: external is never overridden by a runbook match —" out="$(rec 4 'dependabot[bot]' 'chore(deps): bump x from 1 to 2' '[{"path":"go.mod","additions":1,"deletions":1,"change_type":"MODIFIED"}]' ok SUCCESS 'dependabot/go_modules/x-2' NONE true | grade)" eq "fork stays external" external "$(jq -r '.risk.axes.provenance.provenance' <<<"$out")" -eq "external grades R3" R3 "$(jq -r '.risk.tier' <<<"$out")" +eq "external grades xhigh" xhigh "$(jq -r '.risk.tier' <<<"$out")" echo "— phase 5: the unknown contract —" out="$(rec 5 dev 'mystery' null unknown SUCCESS | grade)" @@ -87,6 +87,23 @@ out="$(rec 6 dev 'docs: x' '[{"path":"README.md","additions":1,"deletions":0,"ch eq "map version stamped" v0-generic "$(jq -r '.risk.map_version' <<<"$out")" eq "registry version stamped" v0-generic "$(jq -r '.risk.registry_version' <<<"$out")" +echo "— phase 6b: legacy R0..R3 maps remain compatible but emit canonical tiers —" +LEGACY_MAP="$SANDBOX/legacy-map.json" +jq ' + def legacy: + if . == "low" then "R0" elif . == "medium" then "R1" + elif . == "high" then "R2" elif . == "xhigh" then "R3" else . end; + walk(if type == "string" then legacy else . end)' "$SELF_DIR/../risk-map.v0.json" > "$LEGACY_MAP" +out="$(rec 61 dev 'fix: legacy map' '[{"path":"db/migrations/0001_x.sql","additions":1,"deletions":0,"change_type":"ADDED"}]' ok SUCCESS \ + | bash "$GRADER" --stdin --map "$LEGACY_MAP" 2>"$SANDBOX/legacy-map.err")" +eq "a legacy map still grades" xhigh "$(jq -r '.risk.tier' <<<"$out")" +eq "legacy path tiers are normalized in the record" xhigh "$(jq -r '.risk.axes.path_floor.tier' <<<"$out")" +if grep -F 'deprecated tier aliases' "$SANDBOX/legacy-map.err" >/dev/null; then + ok "legacy map use emits a deprecation warning" +else + bad "legacy map use emits a deprecation warning" "$(cat "$SANDBOX/legacy-map.err")" +fi + echo "— phase 7: a structurally empty map is refused outright —" echo "— per-file path floors are REPORTING ONLY: worst(files) == the floor itself —" # publish-risk-surfaces.sh renders one row per file from risk.axes.path_floor.files. Those @@ -95,14 +112,14 @@ echo "— per-file path floors are REPORTING ONLY: worst(files) == the floor its # with the tier printed above it. # ONE line: --stdin is JSONL, and a pretty-printed record is dropped by the per-line reader. inv="$(grade <<<'{"changed_paths_status":"ok","author":"someone","is_fork":false,"labels":[],"checks_status":"ok","checks_state":"SUCCESS","provenance_status":"ok","changed_paths":[{"path":".github/workflows/a.yml","change_type":"MODIFIED"},{"path":"docs/a.md","change_type":"MODIFIED"},{"path":"src/plain.go","change_type":"MODIFIED"}]}')" -eq "the floor is R3 (the ci rule)" "R3" "$(jq -r '.risk.axes.path_floor.tier' <<<"$inv")" -eq "worst over the per-file floors equals it" "R3" \ - "$(jq -r '[.risk.axes.path_floor.files[].tier] | map({"R0":0,"R1":1,"R2":2,"R3":3}[.]) | max - | ["R0","R1","R2","R3"][.]' <<<"$inv")" +eq "the floor is xhigh (the ci rule)" "xhigh" "$(jq -r '.risk.axes.path_floor.tier' <<<"$inv")" +eq "worst over the per-file floors equals it" "xhigh" \ + "$(jq -r '[.risk.axes.path_floor.files[].tier] | map({"low":0,"medium":1,"high":2,"xhigh":3}[.]) | max + | ["low","medium","high","xhigh"][.]' <<<"$inv")" eq "every changed file gets exactly one row" 3 "$(jq '.risk.axes.path_floor.files | length' <<<"$inv")" -eq "an unmapped path falls to the map default, not to the floor" "R0" \ +eq "an unmapped path falls to the map default, not to the floor" "low" \ "$(jq -r '.risk.axes.path_floor.files[] | select(.path == "src/plain.go") | .tier' <<<"$inv")" -eq "the docs rule keeps its own R0 row under an R3 floor" "R0" \ +eq "the docs rule keeps its own low row under an xhigh floor" "low" \ "$(jq -r '.risk.axes.path_floor.files[] | select(.path == "docs/a.md") | .tier' <<<"$inv")" printf '{}' > "$SANDBOX/empty-map.json" @@ -112,10 +129,10 @@ eq "empty map exits 2" 2 "$?" echo "— phase 8: reversibility floors and rungs —" out="$(rec 8 dev 'docs: x' '[{"path":"README.md","additions":1,"deletions":0,"change_type":"MODIFIED"}]' ok PENDING | grade)" -eq "pending checks floor reversibility at R2" R2 "$(jq -r '.risk.axes.reversibility.tier' <<<"$out")" +eq "pending checks floor reversibility at high" high "$(jq -r '.risk.axes.reversibility.tier' <<<"$out")" out="$(rec 9 dev 'test: cover x' '[{"path":"pkg/x_test.go","additions":9,"deletions":0,"change_type":"ADDED"}]' ok SUCCESS | grade)" -eq "green + test touched grades reversibility R0" R0 "$(jq -r '.risk.axes.reversibility.tier' <<<"$out")" -eq "human provenance keeps overall at R1" R1 "$(jq -r '.risk.tier' <<<"$out")" +eq "green + test touched grades reversibility low" low "$(jq -r '.risk.axes.reversibility.tier' <<<"$out")" +eq "human provenance keeps overall at medium" medium "$(jq -r '.risk.tier' <<<"$out")" echo "— phase 9: --pr with a stubbed gh: the self-excluding rollup —" # Fixture: our own workflow's check run is IN_PROGRESS (it always is, mid-run); one other @@ -171,7 +188,7 @@ eq "self-excluded rollup reads SUCCESS (by name)" SUCCESS "$(jq -r '.checks_stat eq "the record carries the graded head sha" \ "c0ffee1234567890abcdef1234567890abcdef12" "$(jq -r '.head_sha' <<<"$out")" eq "nothing else pending" false "$(jq -r '.checks_pending_excl_self' <<<"$out")" -eq "live docs PR grades R1" R1 "$(jq -r '.risk.tier' <<<"$out")" +eq "live docs PR grades medium" medium "$(jq -r '.risk.tier' <<<"$out")" # --self-run-id is the EXACT selector: same result, but keyed on github.run_id, so a # same-named workflow elsewhere in the consumer repo is no longer swept out of the rollup. out="$(graded_pr --self-run-id 999)" @@ -194,19 +211,19 @@ echo "— phase 10: self-exclusion can never HIDE a red check —" # The failing check belongs to OUR OWN run (the case a consumer creates by putting the grading # job inside its existing CI workflow — every sibling job then shares our run id). Excluding it # from the rollup would aggregate the remaining green contexts to SUCCESS and grade a RED PR -# R0/R1, so the FAILURE scan deliberately covers self too. +# low/medium, so the FAILURE scan deliberately covers self too. jq '.data.repository.pullRequest.commits.nodes[0].commit.statusCheckRollup.contexts.nodes[1] = {"__typename":"CheckRun","name":"unit tests","status":"COMPLETED","conclusion":"FAILURE", "checkSuite":{"workflowRun":{"databaseId":999,"workflow":{"name":"CI - PR Risk Grade"}}}}' \ "$SANDBOX/fixture.json" > "$SANDBOX/f2.json" && cp "$SANDBOX/f2.json" "$SANDBOX/fixture.json" out="$(graded_pr --self-run-id 999)" eq "a failing check in our own run still reads FAILURE" FAILURE "$(jq -r '.checks_state' <<<"$out")" -eq "and reversibility cannot go below R2" R2 "$(jq -r '.risk.axes.reversibility.tier' <<<"$out")" +eq "and reversibility cannot go below high" high "$(jq -r '.risk.axes.reversibility.tier' <<<"$out")" echo "— phase 11: a rollup of nothing but SKIPPED is not a green rollup —" # SKIPPED / NEUTRAL / null conclusions establish NOTHING about whether tests covering these # lines ran, which is the reversibility axis's entire question — so they must not aggregate to -# SUCCESS and let the axis drop to R0/R1. +# SUCCESS and let the axis drop to low/medium. jq '.data.repository.pullRequest.commits.nodes[0].commit.statusCheckRollup.contexts.nodes = [{"__typename":"CheckRun","name":"Grade PR risk","status":"IN_PROGRESS","conclusion":null, "checkSuite":{"workflowRun":{"databaseId":999,"workflow":{"name":"CI - PR Risk Grade"}}}}, @@ -215,7 +232,7 @@ jq '.data.repository.pullRequest.commits.nodes[0].commit.statusCheckRollup.conte "$SANDBOX/fixture.json" > "$SANDBOX/f2.json" && cp "$SANDBOX/f2.json" "$SANDBOX/fixture.json" out="$(graded_pr --self-run-id 999)" eq "all-SKIPPED does not aggregate to SUCCESS" NEUTRAL "$(jq -r '.checks_state' <<<"$out")" -eq "no green rollup floors reversibility at R2" R2 "$(jq -r '.risk.axes.reversibility.tier' <<<"$out")" +eq "no green rollup floors reversibility at high" high "$(jq -r '.risk.axes.reversibility.tier' <<<"$out")" echo "— phase 12: a SHORT changed-file read is unknown, never a floor from the files that fit —" # GraphQL says 5 changed files, the file endpoint returned 1. The old `files(first:100)` path @@ -229,31 +246,31 @@ eq "and the overall grade refuses" null "$(jq -r '.risk.tier' <<<"$out")" echo "— phase 13: a RENAME cannot walk a file out of its guarded directory —" # The origin path is graded too: `git mv src/auth/x.go misc/x.go` used to be recorded under the -# destination ALONE, so the R3 `auth` rule never matched and the move escaped the floor. +# destination ALONE, so the xhigh `auth` rule never matched and the move escaped the floor. out="$(rec 13 dev 'refactor: move things' '[{"path":"misc/x.go","previous_path":"src/auth/x.go","additions":1,"deletions":1,"change_type":"RENAMED"}]' ok SUCCESS | grade)" -eq "the origin path still hits the auth floor" R3 "$(jq -r '.risk.axes.path_floor.tier' <<<"$out")" -eq "renaming a file out of a sensitive class is not a clean revert" R3 "$(jq -r '.risk.axes.reversibility.tier' <<<"$out")" +eq "the origin path still hits the auth floor" xhigh "$(jq -r '.risk.axes.path_floor.tier' <<<"$out")" +eq "renaming a file out of a sensitive class is not a clean revert" xhigh "$(jq -r '.risk.axes.reversibility.tier' <<<"$out")" echo "— phase 14: 'deletes a sensitive file' means the DELETED files, not any changed file —" # MODIFIES an auth file and DELETES an unrelated README. The sensitive-class match used to run # over every changed file, so this reported "deletes N file(s) under a sensitive class" and -# pinned reversibility R3 — a true tier from a false sentence. +# pinned reversibility xhigh — a true tier from a false sentence. out="$(rec 14 dev 'chore: tidy' '[{"path":"src/auth/x.go","additions":2,"deletions":1,"change_type":"MODIFIED"},{"path":"README.md","additions":0,"deletions":9,"change_type":"DELETED"}]' ok SUCCESS | grade)" -eq "deleting a doc is not deleting an auth file" R1 "$(jq -r '.risk.axes.reversibility.tier' <<<"$out")" -eq "the auth path floor still decides the grade" R3 "$(jq -r '.risk.tier' <<<"$out")" -# ...and a genuinely deleted auth file still pins R3. +eq "deleting a doc is not deleting an auth file" medium "$(jq -r '.risk.axes.reversibility.tier' <<<"$out")" +eq "the auth path floor still decides the grade" xhigh "$(jq -r '.risk.tier' <<<"$out")" +# ...and a genuinely deleted auth file still pins xhigh. out="$(rec 15 dev 'chore: drop it' '[{"path":"src/auth/x.go","additions":0,"deletions":9,"change_type":"DELETED"}]' ok SUCCESS | grade)" -eq "deleting an auth file is R3 on reversibility" R3 "$(jq -r '.risk.axes.reversibility.tier' <<<"$out")" +eq "deleting an auth file is xhigh on reversibility" xhigh "$(jq -r '.risk.axes.reversibility.tier' <<<"$out")" echo "— phase 15: the default map's globs match at DEPTH, not just at the repo root —" # `*` does not cross `/`, so an unprefixed glob compiles to a root-only match and the rule -# silently matches nothing in a real tree. The grader + its map are R3: a PR that edits the +# silently matches nothing in a real tree. The grader + its map are xhigh: a PR that edits the # judge must not be graded safest by that judge. out="$(rec 16 dev 'chore: retune' '[{"path":"scripts/pr-risk/risk-map.v0.json","additions":3,"deletions":1,"change_type":"MODIFIED"}]' ok SUCCESS | grade)" -eq "a nested risk map is R3" R3 "$(jq -r '.risk.axes.path_floor.tier' <<<"$out")" +eq "a nested risk map is xhigh" xhigh "$(jq -r '.risk.axes.path_floor.tier' <<<"$out")" out="$(rec 17 dev 'chore: retune' '[{"path":"scripts/pr-risk/grade-pr-risk.sh","additions":3,"deletions":1,"change_type":"MODIFIED"}]' ok SUCCESS | grade)" -eq "the grader itself is R3" R3 "$(jq -r '.risk.axes.path_floor.tier' <<<"$out")" -# A shell test OUTSIDE a tests/ directory must still match the R0 tests class. +eq "the grader itself is xhigh" xhigh "$(jq -r '.risk.axes.path_floor.tier' <<<"$out")" +# A shell test OUTSIDE a tests/ directory must still match the low tests class. out="$(rec 18 dev 'test: smoke' '[{"path":"hack/smoke-test.sh","additions":3,"deletions":0,"change_type":"ADDED"}]' ok SUCCESS | grade)" if jq -e '.risk.axes.path_floor.classes | index("tests")' >/dev/null <<<"$out"; then ok "a nested *-test.sh matches the tests class" @@ -261,15 +278,15 @@ else bad "a nested *-test.sh matches the tests class" "$(jq -c '.risk.axes.path_ echo "— phase 16: 'did a test file change?' comes from the MAP, so every ecosystem can answer —" # The built-in regex knows only the Go/TS shapes, so a Python or Java consumer could never -# reach clean_tier and sat at R1 forever. test_path_patterns in the map is the fix. +# reach clean_tier and sat at medium forever. test_path_patterns in the map is the fix. for p in pkg/test_foo.py pkg/foo_test.py app/FooTest.java spec/foo_spec.rb; do out="$(rec 19 dev 'test: cover it' "[{\"path\":\"$p\",\"additions\":9,\"deletions\":0,\"change_type\":\"ADDED\"}]" ok SUCCESS | grade)" - eq "$p counts as a touched test" R0 "$(jq -r '.risk.axes.reversibility.tier' <<<"$out")" + eq "$p counts as a touched test" low "$(jq -r '.risk.axes.reversibility.tier' <<<"$out")" done echo "— phase 17: a map that forgets a provenance class is refused, not guessed —" # `{}`-shaped omissions used to pass validation and then grade forks off a fallback tier -# nobody chose, silently retiring "external is R3, no exceptions". +# nobody chose, silently retiring "external is xhigh, no exceptions". jq 'del(.provenance_tiers.external)' "$SELF_DIR/../risk-map.v0.json" > "$SANDBOX/no-external.json" rec 20 dev 'docs: x' '[{"path":"README.md","additions":1,"deletions":0,"change_type":"MODIFIED"}]' ok SUCCESS \ | bash "$GRADER" --stdin --map "$SANDBOX/no-external.json" >/dev/null 2>&1 @@ -406,10 +423,10 @@ eq "and still grades" ok "$(jq -r '.risk.status' <<<"$out")" echo "— phase 21: a repo-owned App is a runbook candidate, not an outsider —" # A GitHub App is never an org member, so EVERY PR a repo-owned App opens arrives with # `author_association: NONE`. Testing that string BEFORE the login classification graded every -# such PR `external` => R3 regardless of its diff, and no consumer lever could reach it: the +# such PR `external` => xhigh regardless of its diff, and no consumer lever could reach it: the # `bot_logins` input and .github/risk-runbooks.json are both read further down. The fix narrows # the association half to non-bots; the FORK half stays unconditional, which is what keeps a bot -# on a fork from presenting a bot login to escape R3. +# on a fork from presenting a bot login to escape xhigh. cat > "$SANDBOX/app-runbooks.json" <<'FIX' {"registry_version":"v0-test","runbooks":[ {"id":"data-snapshot-refresh", @@ -422,31 +439,31 @@ FIX app_grade() { bash "$GRADER" --stdin --runbooks "$SANDBOX/app-runbooks.json" 2>/dev/null; } SNAP='[{"path":"data/skills.json","additions":40,"deletions":12,"change_type":"MODIFIED"}]' -# (1) non-fork App PR, author_association NONE, registry entry asserts -> runbook, R0 on the axis. +# (1) non-fork App PR, author_association NONE, registry entry asserts -> runbook, low on the axis. out="$(rec 22 'cloud-code-bot[bot]' 'chore: refresh skills snapshot' "$SNAP" ok SUCCESS bot/refresh NONE false | app_grade)" eq "a non-fork App PR is not external" runbook "$(jq -r '.risk.axes.provenance.provenance' <<<"$out")" eq "and names the runbook that asserted" data-snapshot-refresh "$(jq -r '.risk.axes.provenance.runbook' <<<"$out")" -eq "provenance proposes R0" R0 "$(jq -r '.risk.axes.provenance.tier' <<<"$out")" -# The other axes still decide: an unmapped data path floors R0 and green-but-no-test is R1, so -# the grade now RESPONDS to the diff instead of being pinned R3 by the author's association. -eq "the diff, not the association, decides the grade" R1 "$(jq -r '.risk.tier' <<<"$out")" +eq "provenance proposes low" low "$(jq -r '.risk.axes.provenance.tier' <<<"$out")" +# The other axes still decide: an unmapped data path floors low and green-but-no-test is medium, so +# the grade now RESPONDS to the diff instead of being pinned xhigh by the author's association. +eq "the diff, not the association, decides the grade" medium "$(jq -r '.risk.tier' <<<"$out")" -# (2) the SAME App PR from a FORK is still external R3. The fork test runs first and is +# (2) the SAME App PR from a FORK is still external xhigh. The fork test runs first and is # unconditional — reordering the two would open exactly this hole. out="$(rec 23 'cloud-code-bot[bot]' 'chore: refresh skills snapshot' "$SNAP" ok SUCCESS bot/refresh NONE true | app_grade)" eq "a bot on a fork is still external" external "$(jq -r '.risk.axes.provenance.provenance' <<<"$out")" -eq "and still grades R3" R3 "$(jq -r '.risk.tier' <<<"$out")" +eq "and still grades xhigh" xhigh "$(jq -r '.risk.tier' <<<"$out")" # (3) a non-fork HUMAN with author_association NONE is untouched: the first-time-contributor # guard is narrowed to non-bots, not removed. out="$(rec 24 'drive-by-human' 'feat: my first patch' "$SNAP" ok SUCCESS feature NONE false | app_grade)" eq "a first-time human contributor is still external" external "$(jq -r '.risk.axes.provenance.provenance' <<<"$out")" -eq "and still grades R3" R3 "$(jq -r '.risk.tier' <<<"$out")" +eq "and still grades xhigh" xhigh "$(jq -r '.risk.tier' <<<"$out")" # (4) identity alone buys NO trust: a bot with no asserting registry entry falls back to human. out="$(rec 25 'unregistered-bot[bot]' 'chore: something' "$SNAP" ok SUCCESS bot/x NONE false | app_grade)" eq "an unregistered bot grades human, never runbook" human "$(jq -r '.risk.axes.provenance.provenance' <<<"$out")" -eq "human provenance proposes R1, not R0" R1 "$(jq -r '.risk.axes.provenance.tier' <<<"$out")" +eq "human provenance proposes medium, not low" medium "$(jq -r '.risk.axes.provenance.tier' <<<"$out")" # ...and so does a REGISTERED bot whose diff shape does not assert (paths outside its set). out="$(rec 26 'cloud-code-bot[bot]' 'chore: refresh' '[{"path":"src/evil.go","additions":9,"deletions":0,"change_type":"MODIFIED"}]' ok SUCCESS bot/refresh NONE false | app_grade)" eq "a registered App failing its shape is human, not runbook" human "$(jq -r '.risk.axes.provenance.provenance' <<<"$out")" @@ -468,17 +485,17 @@ eq "and without --bot-logins it reads as a first-time human" external "$(jq -r ' # A REGISTERED producer was never at risk (the shape assertion resolves to `runbook` ahead of # the base class either way) — this pins that, then pins the case that WAS wrong: the # UNREGISTERED bot, which either half classified `agent-supervised`, contradicting the promise -# that a bot with no asserting entry falls back to `human`. The default map tiers both R1, so +# that a bot with no asserting entry falls back to `human`. The default map tiers both medium, so # no grade moves here; the classes exist so a CONSUMER map can tier them apart, and a consumer -# that trusts its supervised agents at R0 would otherwise hand R0 to any `agent-coded` bot PR. +# that trusts its supervised agents at low would otherwise hand low to any `agent-coded` bot PR. out="$(rec 29 'cloud-code-bot[bot]' 'chore: refresh skills snapshot' "$SNAP" ok SUCCESS bot/refresh NONE false \ | jq -c '.labels = ["agent-coded"]' | app_grade)" eq "a registered App is runbook with or without the label" runbook "$(jq -r '.risk.axes.provenance.provenance' <<<"$out")" -eq "and still earns R0 from the shape assertion" R0 "$(jq -r '.risk.axes.provenance.tier' <<<"$out")" +eq "and still earns low from the shape assertion" low "$(jq -r '.risk.axes.provenance.tier' <<<"$out")" out="$(rec 30 'unregistered-bot[bot]' 'chore: something' "$SNAP" ok SUCCESS bot/x NONE false \ | jq -c '.labels = ["agent-coded"]' | app_grade)" eq "an agent-coded unregistered bot is human, not agent-supervised" human "$(jq -r '.risk.axes.provenance.provenance' <<<"$out")" -eq "and human is R1, exactly what agent-supervised was" R1 "$(jq -r '.risk.axes.provenance.tier' <<<"$out")" +eq "and human is medium, exactly what agent-supervised was" medium "$(jq -r '.risk.axes.provenance.tier' <<<"$out")" # The same for `fleet_logins`. That collision exists only because `author_is_bot` no longer # comes from the login string: a Bot actor's GraphQL login arrives UNSUFFIXED, so an operator # who lists it in --fleet-logins makes `classify_login` say "fleet" while GitHub says Bot. The @@ -502,7 +519,7 @@ eq "an agent-coded human is still agent-supervised" agent-supervised "$(jq -r '. # (7) `author_is_bot` is tested `== true`, NOT for jq truthiness. It is the one field that can # switch the `external` guard off, and in jq the STRING "false" — what a foreign collector # writing JSON by hand emits — is truthy. Read loosely, that grades a first-time outsider -# `human` R1 on a field nobody set to true. +# `human` medium on a field nobody set to true. for junk in '"false"' '0' '""' 'null'; do out="$(rec 34 'drive-by-human' 'feat: my first patch' "$SNAP" ok SUCCESS feature NONE false \ | jq -c ".author_is_bot = $junk" | app_grade)" @@ -527,7 +544,7 @@ jq '.data.repository.pullRequest.author = {"login":"cloud-code-bot","__typename" | .data.repository.pullRequest.title = "data: refresh bundled skills snapshot (auto)" | .data.repository.pullRequest.headRefName = "bot/refresh-skills" # ...and the ROLLUP STATE with it. Without --self-run-id the grader reads `.state` directly, - # and phase 9 left it PENDING — which floored reversibility R2 and meant the SUCCESS CheckRun + # and phase 9 left it PENDING — which floored reversibility high and meant the SUCCESS CheckRun # below was never actually consulted, so the fixture implied coverage it did not have. | .data.repository.pullRequest.commits.nodes[0].commit.statusCheckRollup.state = "SUCCESS" | .data.repository.pullRequest.commits.nodes[0].commit.statusCheckRollup.contexts.nodes = @@ -564,11 +581,11 @@ eq "an App PR is no longer external on the live path" human "$(jq -r '.risk.axes out="$(bot_pr --runbooks "$SANDBOX/app-runbooks.json" --bot-logins '')" eq "an entry listing only the SUFFIXED form asserts against an unsuffixed Bot login" \ runbook "$(jq -r '.risk.axes.provenance.provenance' <<<"$out")" -eq "and the tier now responds to the diff" R0 "$(jq -r '.risk.axes.provenance.tier' <<<"$out")" +eq "and the tier now responds to the diff" low "$(jq -r '.risk.axes.provenance.tier' <<<"$out")" # The rollup really is read on this path: green, but no test file in the diff, so reversibility -# proposes R1 and that — not the author's account type — is what decides the grade. -eq "the green rollup is actually consulted" R1 "$(jq -r '.risk.axes.reversibility.tier' <<<"$out")" -eq "and the overall grade is R1, decided by the diff" R1 "$(jq -r '.risk.tier' <<<"$out")" +# proposes medium and that — not the author's account type — is what decides the grade. +eq "the green rollup is actually consulted" medium "$(jq -r '.risk.axes.reversibility.tier' <<<"$out")" +eq "and the overall grade is medium, decided by the diff" medium "$(jq -r '.risk.tier' <<<"$out")" # THE SYNTHESIS IS ONE-WAY AND GATED ON GITHUB'S ACTOR TYPE, which is the whole safety argument: # the suffix is only ever ADDED, and only for an author GitHub types `Bot`. A USER account that # happens to be named `cloud-code-bot` presents `cloud-code-bot` and nothing turns the entry's @@ -594,15 +611,15 @@ jq '.runbooks[0].identity.logins = ["cloud-code-bot[bot]","cloud-code-bot"]' \ jq '.data.repository.pullRequest.isCrossRepository = true' \ "$SANDBOX/botfixture.json" > "$SANDBOX/bf2.json" && cp "$SANDBOX/bf2.json" "$SANDBOX/botfixture.json" out="$(bot_pr --runbooks "$SANDBOX/app-runbooks-both.json")" -eq "a Bot author on a fork is still external R3" external "$(jq -r '.risk.axes.provenance.provenance' <<<"$out")" -eq "and the grade is R3" R3 "$(jq -r '.risk.tier' <<<"$out")" +eq "a Bot author on a fork is still external xhigh" external "$(jq -r '.risk.axes.provenance.provenance' <<<"$out")" +eq "and the grade is xhigh" xhigh "$(jq -r '.risk.tier' <<<"$out")" # A HUMAN author on the same live path keeps the first-time-contributor guard. jq '.data.repository.pullRequest.isCrossRepository = false | .data.repository.pullRequest.author = {"login":"drive-by","__typename":"User"}' \ "$SANDBOX/botfixture.json" > "$SANDBOX/bf2.json" && cp "$SANDBOX/bf2.json" "$SANDBOX/botfixture.json" out="$(bot_pr --runbooks "$SANDBOX/app-runbooks-both.json")" eq "a live first-time human is still external" external "$(jq -r '.risk.axes.provenance.provenance' <<<"$out")" -eq "and still grades R3" R3 "$(jq -r '.risk.tier' <<<"$out")" +eq "and still grades xhigh" xhigh "$(jq -r '.risk.tier' <<<"$out")" echo "— phase 23: reversibility says WHICH files supplied its tier — REPORTING ONLY —" # `axes.reversibility.files` exists so the publisher (BE-7414) can ask "if the top path-floor @@ -618,7 +635,7 @@ echo "— phase 23: reversibility says WHICH files supplied its tier — REPORTI subset_ok() { jq -e '((.risk.axes.reversibility.files // []) - [.risk.axes.path_floor.files[].path]) | length == 0' >/dev/null; } # peeled_tier — the PROPERTY `files` claims, not its shape: drop exactly the attributed paths, -# re-grade, and the axis must no longer be R3. Asserting the shape alone passes green on an +# re-grade, and the axis must no longer be xhigh. Asserting the shape alone passes green on an # attribution that names a real file but not ALL the files holding the tier up, which is the # only reading of the field the publisher actually makes. The per-file path-floor phase pins # its analogue (`worst(files) == floor`); this is the reversibility equivalent. @@ -632,11 +649,11 @@ peeled_tier() { # <pr> <paths-json> <graded-out> -> the reversibility tier after # (a) irreversible class: the migration supplied the tier; the docs file rode along. paths='[{"path":"db/migrations/0001_x.sql","additions":9,"deletions":0,"change_type":"ADDED"},{"path":"docs/a.md","additions":1,"deletions":0,"change_type":"MODIFIED"}]' out="$(rec 30 dev 'feat: schema' "$paths" ok SUCCESS | grade)" -eq "irreversible-class reversibility is still R3" R3 "$(jq -r '.risk.axes.reversibility.tier' <<<"$out")" -eq "and the overall tier is still R3" R3 "$(jq -r '.risk.tier' <<<"$out")" +eq "irreversible-class reversibility is still xhigh" xhigh "$(jq -r '.risk.axes.reversibility.tier' <<<"$out")" +eq "and the overall tier is still xhigh" xhigh "$(jq -r '.risk.tier' <<<"$out")" eq "only the migration is attributed" '["db/migrations/0001_x.sql"]' "$(jq -c '.risk.axes.reversibility.files' <<<"$out")" if subset_ok <<<"$out"; then ok "attributed paths are a subset of the path-floor rows"; else bad "attributed paths are a subset of the path-floor rows" "$(jq -c '.risk.axes.reversibility.files' <<<"$out")"; fi -eq "peeling the attributed paths drops the irreversible-class tier" R1 "$(peeled_tier 130 "$paths" "$out")" +eq "peeling the attributed paths drops the irreversible-class tier" medium "$(peeled_tier 130 "$paths" "$out")" # (b) delete-sensitive: attribution is BY ROW, never by intersecting a row's classes with the # sensitive list. `src/auth/y.go` is MODIFIED and carries class `auth` too — a class @@ -645,12 +662,12 @@ eq "peeling the attributed paths drops the irreversible-class tier" R1 "$(peeled # either (peeling it alone would leave the reason standing). paths='[{"path":"src/auth/x.go","additions":0,"deletions":9,"change_type":"DELETED"},{"path":"src/auth/y.go","additions":2,"deletions":1,"change_type":"MODIFIED"},{"path":"README.md","additions":0,"deletions":4,"change_type":"DELETED"}]' out="$(rec 31 dev 'chore: drop it' "$paths" ok SUCCESS | grade)" -eq "delete-sensitive reversibility is still R3" R3 "$(jq -r '.risk.axes.reversibility.tier' <<<"$out")" -eq "and the overall tier is still R3" R3 "$(jq -r '.risk.tier' <<<"$out")" +eq "delete-sensitive reversibility is still xhigh" xhigh "$(jq -r '.risk.axes.reversibility.tier' <<<"$out")" +eq "and the overall tier is still xhigh" xhigh "$(jq -r '.risk.tier' <<<"$out")" eq "only the DELETED sensitive file is attributed" '["src/auth/x.go"]' "$(jq -c '.risk.axes.reversibility.files' <<<"$out")" eq "the deleted_files count is untouched by the attribution" 2 "$(jq -r '.risk.axes.reversibility.deleted_files' <<<"$out")" if subset_ok <<<"$out"; then ok "delete attribution is a subset of the path-floor rows"; else bad "delete attribution is a subset of the path-floor rows" "$(jq -c '.risk.axes.reversibility.files' <<<"$out")"; fi -eq "peeling the attributed path drops the delete-sensitive tier" R1 "$(peeled_tier 131 "$paths" "$out")" +eq "peeling the attributed path drops the delete-sensitive tier" medium "$(peeled_tier 131 "$paths" "$out")" # ...and the SENTENCE agrees with the list beside it: one sensitive removal, class `auth` only. # The count used to be every removal and the classes every class any removal matched, so this # fixture printed "removes 2 file(s) under a sensitive class (auth, docs)" — with `docs` not @@ -664,38 +681,38 @@ eq "the reason counts only the SENSITIVE removals" \ # publisher can compare against. paths='[{"path":"misc/x.go","previous_path":"src/auth/x.go","additions":1,"deletions":1,"change_type":"RENAMED"}]' out="$(rec 32 dev 'refactor: move things' "$paths" ok SUCCESS | grade)" -eq "renaming out of a sensitive class is still R3" R3 "$(jq -r '.risk.axes.reversibility.tier' <<<"$out")" +eq "renaming out of a sensitive class is still xhigh" xhigh "$(jq -r '.risk.axes.reversibility.tier' <<<"$out")" eq "the renamed row is attributed under its DESTINATION" '["misc/x.go"]' "$(jq -c '.risk.axes.reversibility.files' <<<"$out")" if subset_ok <<<"$out"; then ok "rename attribution is a subset of the path-floor rows"; else bad "rename attribution is a subset of the path-floor rows" "$(jq -c '.risk.axes.reversibility.files' <<<"$out")"; fi -eq "peeling the renamed row drops the tier" R1 "$(peeled_tier 132 "$paths" "$out")" +eq "peeling the renamed row drops the tier" medium "$(peeled_tier 132 "$paths" "$out")" -# (c2) BOTH R3 RUNGS AT ONCE — the tier ladder is first-match, the attribution is NOT. This PR +# (c2) BOTH xhigh RUNGS AT ONCE — the tier ladder is first-match, the attribution is NOT. This PR # adds a migration (irreversible class) AND deletes an auth file (sensitive removal). Naming # only the migration would answer the publisher's peel question "yes, the reversibility reason -# goes with these files" while the delete-sensitive rung silently held the axis at R3 — a false +# goes with these files" while the delete-sensitive rung silently held the axis at xhigh — a false # positive out of the very suppression the `null` rungs exist for. The peel below is the assert -# that matters: strip everything `files` names and the axis must actually leave R3. +# that matters: strip everything `files` names and the axis must actually leave xhigh. paths='[{"path":"db/migrations/0002_y.sql","additions":9,"deletions":0,"change_type":"ADDED"},{"path":"src/auth/x.go","additions":0,"deletions":9,"change_type":"DELETED"},{"path":"docs/a.md","additions":1,"deletions":0,"change_type":"MODIFIED"}]' out="$(rec 38 dev 'feat: schema + drop auth' "$paths" ok SUCCESS | grade)" -eq "both-rungs reversibility is still R3" R3 "$(jq -r '.risk.axes.reversibility.tier' <<<"$out")" +eq "both-rungs reversibility is still xhigh" xhigh "$(jq -r '.risk.axes.reversibility.tier' <<<"$out")" eq "the reason is still the FIRST rung's" \ "touches migrations — mutates persistent state or deletes data; reverting the code does not restore it" \ "$(jq -r '.risk.axes.reversibility.reason' <<<"$out")" eq "but BOTH rungs' files are attributed" \ '["db/migrations/0002_y.sql","src/auth/x.go"]' "$(jq -c '.risk.axes.reversibility.files' <<<"$out")" if subset_ok <<<"$out"; then ok "the union is a subset of the path-floor rows"; else bad "the union is a subset of the path-floor rows" "$(jq -c '.risk.axes.reversibility.files' <<<"$out")"; fi -eq "peeling the union clears EVERY attributable rung" R1 "$(peeled_tier 138 "$paths" "$out")" +eq "peeling the union clears EVERY attributable rung" medium "$(peeled_tier 138 "$paths" "$out")" # (d)/(e)/(f) the three non-attributable rungs. "No green rollup" is a property of the head # COMMIT and "no test touched" of the WHOLE change set; neither is removable by dropping files. out="$(rec 33 dev 'docs: x' '[{"path":"README.md","additions":1,"deletions":0,"change_type":"MODIFIED"}]' ok PENDING | grade)" -eq "no green rollup is still R2" R2 "$(jq -r '.risk.axes.reversibility.tier' <<<"$out")" +eq "no green rollup is still high" high "$(jq -r '.risk.axes.reversibility.tier' <<<"$out")" eq "and attributes nothing" null "$(jq -r '.risk.axes.reversibility.files' <<<"$out")" out="$(rec 34 dev 'feat: x' '[{"path":"src/x.go","additions":9,"deletions":0,"change_type":"MODIFIED"}]' ok SUCCESS | grade)" -eq "no test touched is still R1" R1 "$(jq -r '.risk.axes.reversibility.tier' <<<"$out")" +eq "no test touched is still medium" medium "$(jq -r '.risk.axes.reversibility.tier' <<<"$out")" eq "and attributes nothing" null "$(jq -r '.risk.axes.reversibility.files' <<<"$out")" out="$(rec 35 dev 'test: cover x' '[{"path":"pkg/x_test.go","additions":9,"deletions":0,"change_type":"ADDED"}]' ok SUCCESS | grade)" -eq "a clean revert is still R0" R0 "$(jq -r '.risk.axes.reversibility.tier' <<<"$out")" +eq "a clean revert is still low" low "$(jq -r '.risk.axes.reversibility.tier' <<<"$out")" eq "and attributes nothing" null "$(jq -r '.risk.axes.reversibility.files' <<<"$out")" # An unknown path axis takes reversibility unknown BEFORE the attribution runs, so the field is @@ -721,23 +738,23 @@ out="$(rec 39 dev 'feat: schema' "$paths" ok SUCCESS | grade)" # peeling and re-grading. Asserting them equal is what stops the bound drifting into a guess. eq "residual_tier equals the tier a real peel produces (green rollup)" \ "$(peeled_tier 139 "$paths" "$out")" "$(jq -r '.risk.axes.reversibility.residual_tier' <<<"$out")" -eq "…which on the repo map's defaults is R1" R1 "$(jq -r '.risk.axes.reversibility.residual_tier' <<<"$out")" +eq "…which on the repo map's defaults is medium" medium "$(jq -r '.risk.axes.reversibility.residual_tier' <<<"$out")" # NOT GREEN: rung 3 tests the HEAD COMMIT's rollup, which no peel can change, so the remainder -# lands exactly on no_green_checks_tier — R2 here, still below the R3 headline. +# lands exactly on no_green_checks_tier — high here, still below the xhigh headline. out="$(rec 40 dev 'feat: schema' "$paths" ok PENDING | grade)" -eq "a non-green rollup pins the residual at no_green_checks_tier" R2 \ +eq "a non-green rollup pins the residual at no_green_checks_tier" high \ "$(jq -r '.risk.axes.reversibility.residual_tier' <<<"$out")" -# THE CONSUMER OVERRIDE the field exists to catch: raise no_green_checks_tier to R3 and the -# remainder lands back on R3, so peeling the migration buys the PR nothing. The attribution is +# THE CONSUMER OVERRIDE the field exists to catch: raise no_green_checks_tier to xhigh and the +# remainder lands back on xhigh, so peeling the migration buys the PR nothing. The attribution is # unchanged — only the bound moves, and it is the bound the publisher gates on. ovmap="$SANDBOX/map-no-green-r3.json" -jq '.reversibility.no_green_checks_tier = "R3"' "$SELF_DIR/../risk-map.v0.json" > "$ovmap" +jq '.reversibility.no_green_checks_tier = "xhigh"' "$SELF_DIR/../risk-map.v0.json" > "$ovmap" out="$(rec 41 dev 'feat: schema' "$paths" ok PENDING | bash "$GRADER" --stdin --map "$ovmap" 2>/dev/null)" -eq "…and an override that raises that rung to R3 is reported as R3" R3 \ +eq "…and an override that raises that rung to xhigh is reported as xhigh" xhigh \ "$(jq -r '.risk.axes.reversibility.residual_tier' <<<"$out")" eq "…while the attribution itself is unchanged" '["db/migrations/0001_x.sql"]' \ "$(jq -c '.risk.axes.reversibility.files' <<<"$out")" -eq "…and the tier the override cannot touch is still R3 from the irreversible class" R3 \ +eq "…and the tier the override cannot touch is still xhigh from the irreversible class" xhigh \ "$(jq -r '.risk.axes.reversibility.tier' <<<"$out")" # The three non-attributable rungs answer no peel question at all, so they carry no bound either. out="$(rec 42 dev 'feat: x' '[{"path":"src/x.go","additions":9,"deletions":0,"change_type":"MODIFIED"}]' ok SUCCESS | grade)" diff --git a/scripts/pr-risk/tests/test_grade_targets.sh b/scripts/pr-risk/tests/test_grade_targets.sh index c9a59485..d5d03a9e 100755 --- a/scripts/pr-risk/tests/test_grade_targets.sh +++ b/scripts/pr-risk/tests/test_grade_targets.sh @@ -15,11 +15,11 @@ # which is what an empty `?ref=` silently resolves to. # * ONE BAD TARGET NEVER ABANDONS THE REST. A batch reports the bad one and grades the others. # * `ungraded` IS NOT A FAILED RUN, on either path — an unreadable PR is a reported verdict. -# * FORKS AND BOTS. Forks still grade R3 from the API-derived fork flag; bot-authored PRs DO +# * FORKS AND BOTS. Forks still grade xhigh from the API-derived fork flag; bot-authored PRs DO # grade on the by-number path (the actor guard that skips them is a token guard on the event # path only). # * SELF-EXCLUSION SURVIVES A DISPATCH: a run id that is not in the PR's rollup excludes -# nothing, and a settled rollup therefore reads its true SUCCESS instead of an R2 floor. +# nothing, and a settled rollup therefore reads its true SUCCESS instead of a high floor. # # bash tests/test_grade_targets.sh # exit 0 = all green # @@ -173,9 +173,9 @@ run_targets PR_NUMBERS=42 BASE_REF=main eq "the run succeeds" 0 "$RC" eq "one result recorded" 1 "$(results | jq -s length)" eq "status is graded" graded "$(res '.status')" -eq "the docs PR grades R1" R1 "$(res '.tier')" +eq "the docs PR grades medium" medium "$(res '.tier')" eq "the event's base ref is the one used" main "$(res '.base_ref')" -eq "the label is the mapped R1 label" risk:R1 "$(res '.label')" +eq "the label is the mapped medium label" risk:medium "$(res '.label')" # The BC proof: the event path spends no API call re-reading a base ref it was handed. `pulls/42` # appears only as the grader's own `pulls/42/files` read. no_text "no base-ref PR read is issued" "pulls/42 " "$(calls | grep -v files || true)" @@ -193,7 +193,7 @@ has_text "the base ref is read once" "repos/test/repo/pulls/42 " "$(calls | grep has_text "the risk map is read from THAT ref" "contents/.github/risk.json?ref=release%2Fv2" "$(calls)" has_text "and so is the runbook registry" "contents/.github/risk-runbooks.json?ref=release%2Fv2" "$(calls)" no_text "never from an empty ref" "?ref=" "$(calls | grep 'contents/' | grep -F '?ref=' | grep -v 'ref=release%2Fv2' || true)" -eq "the tier is unchanged by the path taken" R1 "$(res '.tier')" +eq "the tier is unchanged by the path taken" medium "$(res '.tier')" echo "— phase 3: a 404 override falls back to the shipped default and still grades —" eq "contents_mode is 404 for this phase" 404 "$(cat "$STUB_DIR/contents_mode")" @@ -241,7 +241,7 @@ eq "all three targets are recorded" 3 "$(results | jq -s length)" eq "the first target graded" graded "$(res '.status' 0)" eq "the unreadable one failed" failed "$(res '.status' 1)" eq "the LAST target still graded" graded "$(res '.status' 2)" -eq "and it carries a real tier" R1 "$(res '.tier' 2)" +eq "and it carries a real tier" medium "$(res '.tier' 2)" has_text "the batch summary counts both outcomes" "2 graded, 0 ungraded, 1 failed" "$OUT" rm -f "$STUB_DIR/fail_numbers" @@ -263,14 +263,14 @@ touch "$STUB_DIR/label_fail" run_targets PR_NUMBERS=42 BASE_REF=main DRY_RUN=0 eq "the run fails" 1 "$RC" eq "the target is recorded failed" failed "$(res '.status')" -eq "the tier it computed is kept" R1 "$(res '.tier')" +eq "the tier it computed is kept" medium "$(res '.tier')" has_text "and the note says the label did not land" "label write FAILED" "$(results)" rm -f "$STUB_DIR/label_fail" -echo "— phase 10: forks still grade R3 from the API-derived fork flag, on the by-number path —" +echo "— phase 10: forks still grade xhigh from the API-derived fork flag, on the by-number path —" write_fixture dev NONE true run_targets PR_NUMBERS=42 -eq "a fork grades R3" R3 "$(res '.tier')" +eq "a fork grades xhigh" xhigh "$(res '.tier')" eq "provenance is external" external \ "$(jq -r '.risk.axes.provenance.provenance' "$WORK/record-42.json" 2>/dev/null)" write_fixture @@ -282,24 +282,24 @@ echo "— phase 11: a BOT-authored PR grades on the by-number path —" write_fixture 'dependabot[bot]' CONTRIBUTOR false run_targets PR_NUMBERS=42 eq "the bot's PR is graded, not skipped" graded "$(res '.status')" -eq "and it carries a real tier" R1 "$(res '.tier')" +eq "and it carries a real tier" medium "$(res '.tier')" write_fixture echo "— phase 12: self-exclusion survives a dispatch — a foreign run id excludes nothing —" # A dispatched run's check is attached to the dispatched ref, not the PR's head, so it is absent -# from the rollup entirely. The rollup must then read its true settled state rather than the R2 +# from the rollup entirely. The rollup must then read its true settled state rather than the high # floor a still-pending self check produces on the event path. run_targets PR_NUMBERS=42 SELF_RUN_ID=777777 eq "the settled rollup reads SUCCESS" SUCCESS \ "$(jq -r '.checks_state' "$WORK/record-42.json" 2>/dev/null)" eq "nothing reads as pending" false \ "$(jq -r '.checks_pending_excl_self' "$WORK/record-42.json" 2>/dev/null)" -# R1 = "checks green but no test file touched" (the fixture's only file is a README), which is -# the point: the axis ANSWERED the question. An R2 here would mean "no green rollup" — the floor +# medium = "checks green but no test file touched" (the fixture's only file is a README), which is +# the point: the axis ANSWERED the question. A high here would mean "no green rollup" — the floor # the event path pays when its own check is still in flight, and the one a dispatch must not pay. -eq "so reversibility is not floored at R2" R1 \ +eq "so reversibility is not floored at high" medium \ "$(jq -r '.risk.axes.reversibility.tier' "$WORK/record-42.json" 2>/dev/null)" -eq "and the grade is a real tier" R1 "$(res '.tier')" +eq "and the grade is a real tier" medium "$(res '.tier')" echo "— phase 13: the target list is validated, and a dispatch with no target FAILS LOUDLY —" run_targets PR_NUMBERS="" From 815331057ebf2302b90e276364845bd8f7cc0f6f Mon Sep 17 00:00:00 2001 From: huang47 <157390+huang47@users.noreply.github.com> Date: Mon, 31 Aug 2026 21:36:19 -0700 Subject: [PATCH 2/5] feat(pr-risk): support named risk labels --- .github/workflows/pr-risk.yml | 34 +++--- scripts/pr-risk/apply-risk-label.sh | 67 ++++++++--- .../pr-risk/tests/test_apply_risk_label.sh | 112 +++++++++++------- 3 files changed, 134 insertions(+), 79 deletions(-) diff --git a/.github/workflows/pr-risk.yml b/.github/workflows/pr-risk.yml index d62bb4d7..55b509a0 100644 --- a/.github/workflows/pr-risk.yml +++ b/.github/workflows/pr-risk.yml @@ -1,11 +1,13 @@ name: PR Risk Grade (reusable) # Reusable ADVISORY PR risk grader — the shadow-check rung of the PR risk-grading ladder. -# Grades every PR event into a tier R0 (safest) .. R3 (riskiest) and syncs ONE label -# (`risk:R0` .. `risk:R3`, or `risk:ungraded` when an input was unreadable). That label is +# Grades every PR event into a tier low (safest) .. xhigh (riskiest) and syncs ONE label +# (`risk:low` .. `risk:xhigh`, or `risk:ungraded` when an input was unreadable). That label is # the entire product: nothing is gated, nothing is blocked, nothing merges, no comment is # posted. Humans look at the label and agree or disagree; disagreement is recorded by adding # the `risk-dispute` label (which this workflow never touches) plus a comment saying why. +# Existing maps and label_map inputs may still use R0..R3 as deprecated aliases; the grader +# normalizes them to low..xhigh before grading, and all new output uses the canonical names. # # grade = worst(path_floor, provenance, reversibility) — three deterministic axes; the worst # tier wins, so no axis can move a PR into a safer lane than another axis put it. No LLM, no @@ -75,7 +77,7 @@ name: PR Risk Grade (reusable) # read-only GITHUB_TOKEN that the caller's `permissions:` block cannot elevate, so the label # write — this workflow's entire product — would 403. On a dispatch the token is writable, so # the reason evaporates. 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 +# from the API's `isCrossRepository`, never from the actor, and forks grade xhigh with no # exceptions. DEPENDABOT-TRIGGERED RUNS need no such hatch and are graded on BOTH paths: the # caller pattern below deliberately carries no `github.actor != 'dependabot[bot]'` clause, # because the caller's `permissions:` block elevates Dependabot's read-only token and this @@ -97,7 +99,7 @@ name: PR Risk Grade (reusable) # 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 +# confirmation, so a target someone pushed to minutes ago lands the honest high 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: @@ -114,8 +116,9 @@ name: PR Risk Grade (reusable) # last-writer-wins with still EXACTLY ONE `risk:*` label — possibly the staler tier, which gates # nothing meanwhile and which the next grade re-syncs (on the LAST push there is no next grade, so # re-dispatch on `pr_number` if a final grade looks wrong). It can no longer leave two contradictory -# labels on a PR under one `label_map`; remapping `label_map` orphans the old names, which is a -# one-time repo-side cleanup. What the shape does cost is a narrower residual: the PUT is built from +# labels on a PR under one `label_map`; remapping `label_map` orphans custom old names, which is a +# one-time repo-side cleanup (the known former defaults risk:R0..risk:R3 are retired automatically). +# What the shape does cost is a narrower residual: the PUT is built from # a snapshot read, so a NON-owned label added by someone else in the read→PUT window is dropped # (`risk-dispute` included) and one removed in it is resurrected. That window opens only on a run # that actually changes the grade and is roughly one API round-trip — about three on the first @@ -139,7 +142,7 @@ name: PR Risk Grade (reusable) # ENROLL THIS AS ITS OWN WORKFLOW, not as one job inside an existing CI workflow. The grading # job is part of the check rollup it reads, so it excludes its own RUN from that rollup; a job # sharing a run with the rest of CI therefore excludes its siblings too and lands on the honest -# R2 floor instead of grading off a full rollup. (It can never grade a red PR green either way +# high floor instead of grading off a full rollup. (It can never grade a red PR green either way # — a FAILING check is never excluded.) # # Caller pattern (consumer repo, .github/workflows/ci-pr-risk.yml): @@ -175,7 +178,7 @@ name: PR Risk Grade (reusable) # # guard, not a risk judgement: a fork's `pull_request` run gets a read-only GITHUB_TOKEN, # # the `permissions:` block below CANNOT elevate it (the Dependabot carve-out below does # # not extend to forks), so the label write would 403 and the check would go red. Fork -# # RISK is untouched either way — forks grade R3 from the API's own fork flag, never from +# # RISK is untouched either way — forks grade xhigh from the API's own fork flag, never from # # the actor — so a fork PR is simply ungraded-by-absence here; grade it by dispatching on # # `pr_number`, or enroll with `pull_request_target` instead, which is safe by construction # # for this workflow (see the fork paragraph above). Keep the clause BEHIND the event test: @@ -319,7 +322,7 @@ on: bot with no runbook registry entry grades as human — identity alone never buys trust. LOAD-BEARING, not a hint: a listed login skips the first-time-contributor test, moving a non-fork NONE PR from `external` - (R3) to `human` (R1), and nothing validates that the login really is a + (xhigh) to `human` (medium), and nothing validates that the login really is a machine account. List only accounts you control; prune retired ones. type: string required: false @@ -328,9 +331,10 @@ on: description: >- Rename the five grader-owned labels, as `tier=label` pairs (comma-separated). Default: - `R0=risk:R0,R1=risk:R1,R2=risk:R2,R3=risk:R3,unknown=risk:ungraded`. - Tier KEYS are fixed; only the label text is yours. Missing labels are - created on first use, color-coded green through red. + `low=risk:low,medium=risk:medium,high=risk:high,xhigh=risk:xhigh,unknown=risk:ungraded`. + Canonical tier keys are low, medium, high, xhigh, and unknown. + Deprecated R0..R3 keys remain accepted. Missing labels are created on + first use, color-coded green through red. type: string required: false default: '' @@ -338,7 +342,7 @@ on: description: >- 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 + reads). 0 labels immediately — expect high 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. On the by-number path a dispatched @@ -1052,14 +1056,14 @@ jobs: 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" + # empty on a cancelled step — and heading a row that recorded medium 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 + # that outcome "PR risk grade: medium" with the full axis table under it reads as "medium # 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. diff --git a/scripts/pr-risk/apply-risk-label.sh b/scripts/pr-risk/apply-risk-label.sh index b413a6d7..72db84da 100755 --- a/scripts/pr-risk/apply-risk-label.sh +++ b/scripts/pr-risk/apply-risk-label.sh @@ -2,8 +2,9 @@ # apply-risk-label.sh — sync a PR's risk label to the computed tier. The one write the # reusable pr-risk.yml workflow performs. # -# OWNERSHIP CONTRACT: this script owns EXACTLY the label names in LABEL_MAP's values, and a PR it -# has written to carries EXACTLY ONE of them. That holds under concurrency BY CONSTRUCTION, not by +# OWNERSHIP CONTRACT: this script owns the label names in LABEL_MAP's values plus the four retired +# default labels (`risk:R0`..`risk:R3`) during their deprecation window, and a PR it has written to +# carries EXACTLY ONE current mapped label. That holds under concurrency BY CONSTRUCTION, not by # luck: the sync is a single atomic `PUT .../labels`, which replaces the PR's whole label set in one # request, so there is no delete-then-add window for a second run to interleave into. Two runs # racing (a `pr_numbers` batch and a `pull_request` event run sit in different concurrency groups, @@ -36,11 +37,9 @@ # before-snapshot and the after-read, so the diff that would have to catch it is empty by # construction. # -# ONE MORE LIMIT, on a different axis: ownership is defined by the CURRENT LABEL_MAP. Change a -# caller's `label_map` and labels applied under the old map are, by definition, no longer owned — -# they are carried through every future PUT beside the new target, and nothing here will clean them -# up. Remapping is a one-time repo-side cleanup (delete the retired label names), not something a -# re-grade heals. +# ONE MORE LIMIT, on a different axis: ownership is defined by the CURRENT LABEL_MAP plus the four +# known former defaults. Those defaults are removed by the first canonical re-grade; custom values +# from an older map are unknowable and still need one-time repo-side cleanup. # # The label is applied with the plain GITHUB_TOKEN on purpose: GITHUB_TOKEN-applied labels do # not fire `labeled` workflow triggers, which makes the shadow check incapable of starting a @@ -50,11 +49,11 @@ # Inputs (env): # REPO owner/name of the repo holding the PR (required) # PR_NUMBER the PR number (required) -# TIER R0 | R1 | R2 | R3 | unknown ('' and 'null' read as unknown) (required) +# TIER low | medium | high | xhigh | unknown ('' and 'null' read as unknown) (required) +# R0 | R1 | R2 | R3 remain accepted as deprecated aliases. # LABEL_MAP tier=label pairs, comma-separated (optional) -# default: R0=risk:R0,R1=risk:R1,R2=risk:R2,R3=risk:R3,unknown=risk:ungraded -# Relabeling (e.g. a 1-indexed R1..R4 scheme) is a caller-side remap of the -# VALUES only; tier keys are fixed R0..R3 + unknown everywhere else. +# default: low=risk:low,medium=risk:medium,high=risk:high,xhigh=risk:xhigh,unknown=risk:ungraded +# Legacy R0..R3 keys remain accepted as deprecated aliases. # DRY_RUN 1 = print the plan, write nothing # GH_TOKEN token for gh (in CI: the job's GITHUB_TOKEN; needs pull-requests: write for # the label add/remove on the PR — the labels endpoint is dual-mapped and @@ -78,7 +77,7 @@ PR_NUMBER="${PR_NUMBER:-}" TIER="${TIER:-}" LABEL_MAP="${LABEL_MAP:-}" DRY_RUN="${DRY_RUN:-0}" -DEFAULT_MAP="R0=risk:R0,R1=risk:R1,R2=risk:R2,R3=risk:R3,unknown=risk:ungraded" +DEFAULT_MAP="low=risk:low,medium=risk:medium,high=risk:high,xhigh=risk:xhigh,unknown=risk:ungraded" log() { printf '[apply-risk-label] %s\n' "$*" >&2; } die() { printf '[apply-risk-label] ERROR %s\n' "$*" >&2; exit 2; } @@ -90,29 +89,61 @@ fail() { printf '[apply-risk-label] FAIL %s\n' "$*" >&2; exit 4; } command -v jq >/dev/null 2>&1 || die "jq not found on PATH" [ "$DRY_RUN" = 1 ] || command -v gh >/dev/null 2>&1 || die "gh not found on PATH" +canonical_tier() { + case "$1" in + R0) echo low ;; R1) echo medium ;; R2) echo high ;; R3) echo xhigh ;; *) echo "$1" ;; + esac +} +legacy_tier() { + case "$1" in + low) echo R0 ;; medium) echo R1 ;; high) echo R2 ;; xhigh) echo R3 ;; *) echo "$1" ;; + esac +} + # '' and 'null' arrive when the grader refused a confident tier — both are the unknown lane. case "$TIER" in ""|null) TIER="unknown" ;; esac -case "$TIER" in R0|R1|R2|R3|unknown) ;; *) die "bad TIER '$TIER' (want R0..R3 or unknown)" ;; esac +case "$TIER" in + R0|R1|R2|R3) + log "WARN tier '$TIER' is deprecated; use '$(canonical_tier "$TIER")'" + TIER="$(canonical_tier "$TIER")" ;; + low|medium|high|xhigh|unknown) ;; + *) die "bad TIER '$TIER' (want low..xhigh or unknown)" ;; +esac [ -n "$LABEL_MAP" ] || LABEL_MAP="$DEFAULT_MAP" +if printf '%s' "$LABEL_MAP" | tr ',' '\n' | awk -F= '$1 ~ /^R[0-3]$/ { found=1 } END { exit !found }'; then + log "WARN LABEL_MAP keys R0..R3 are deprecated; use low, medium, high, xhigh" +fi # Parse "tier=label,tier=label" without eval. All five tiers must resolve: a map that forgets # `unknown` would leave ungradeable PRs silently unlabeled, which reads as "grader never ran". label_for() { # <tier> -> label on stdout, rc 1 when unmapped - printf '%s' "$LABEL_MAP" | tr ',' '\n' | awk -F= -v t="$1" '$1 == t { print $2; found=1 } END { exit !found }' + local legacy + legacy="$(legacy_tier "$1")" + printf '%s' "$LABEL_MAP" | tr ',' '\n' | awk -F= -v t="$1" -v legacy="$legacy" ' + $1 == t { exact=$2; exact_found=1 } + $1 == legacy { alias=$2; alias_found=1 } + END { + if (exact_found) print exact + else if (alias_found) print alias + else exit 1 + }' } OWNED=() -for t in R0 R1 R2 R3 unknown; do +for t in low medium high xhigh unknown; do l="$(label_for "$t")" || die "LABEL_MAP is missing a label for tier '$t' (got '$LABEL_MAP')" [ -n "$l" ] || die "LABEL_MAP maps tier '$t' to an empty label" OWNED+=("$l") done +# Known former defaults are safe to retire automatically; custom historical label-map values are +# unknowable and still need the one-time cleanup documented in the README. +OWNED+=("risk:R0" "risk:R1" "risk:R2" "risk:R3") TARGET="$(label_for "$TIER")" # Colors keyed by TIER (not label text, which callers may remap): green .. red, gray unknown. color_for() { case "$1" in - R0) echo "0e8a16" ;; R1) echo "fbca04" ;; R2) echo "d93f0b" ;; R3) echo "b60205" ;; + low) echo "0e8a16" ;; medium) echo "fbca04" ;; high) echo "d93f0b" ;; xhigh) echo "b60205" ;; *) echo "cfd3d7" ;; esac } @@ -124,7 +155,7 @@ if [ "$DRY_RUN" = 1 ]; then fi # A label name is a PATH SEGMENT in the repo-label probe below, and GitHub label names legally -# contain spaces, `/`, `#`, `?` and `%`. A caller who remaps `R3=risk high` would otherwise build a +# contain spaces, `/`, `#`, `?` and `%`. A caller who remaps `xhigh=risk high` would otherwise build a # malformed or misrouted URL: the probe misses, and a rename paints the check red or spams # pre-creates. Encoded for the path only — the raw name is what we log, compare and send as a form # field. (The label sync itself needs no encoding: its path carries only the PR number, and the @@ -155,7 +186,7 @@ current="$(ghq api --paginate "repos/$REPO/issues/$PR_NUMBER/labels?per_page=100 | jq -sc 'add // []')" \ || fail "could not read labels on $REPO#$PR_NUMBER: $(gherr)" -# GitHub label identity is CASE-INSENSITIVE (you cannot hold both `risk:R2` and `Risk:R2`), so every +# GitHub label identity is CASE-INSENSITIVE (you cannot hold both `risk:high` and `Risk:high`), so every # comparison against the snapshot has to be too. A case variant — an older LABEL_MAP spelling, or a # hand-created label — otherwise reads as "not the target AND not owned": the in-sync short-circuit # never fires, so every run issues the PUT (widening the residual window the header confines to diff --git a/scripts/pr-risk/tests/test_apply_risk_label.sh b/scripts/pr-risk/tests/test_apply_risk_label.sh index 78ab30fe..fe23803e 100755 --- a/scripts/pr-risk/tests/test_apply_risk_label.sh +++ b/scripts/pr-risk/tests/test_apply_risk_label.sh @@ -23,24 +23,36 @@ run() { # <tier> [label_map] -> stdout (the target label); rc in $? } echo "— default map —" -eq "R0 maps to risk:R0" "risk:R0" "$(run R0)" -eq "R3 maps to risk:R3" "risk:R3" "$(run R3)" +eq "low maps to risk:low" "risk:low" "$(run low)" +eq "xhigh maps to risk:xhigh" "risk:xhigh" "$(run xhigh)" eq "unknown maps to risk:ungraded" "risk:ungraded" "$(run unknown)" eq "empty tier reads as unknown" "risk:ungraded" "$(run '')" eq "literal null reads as unknown" "risk:ungraded" "$(run null)" -echo "— caller remap (a 1-indexed R1..R4 scheme is one input) —" -MAP='R0=risk:R1,R1=risk:R2,R2=risk:R3,R3=risk:R4,unknown=risk:ungraded' -eq "R0 remaps to risk:R1" "risk:R1" "$(run R0 "$MAP")" -eq "R3 remaps to risk:R4" "risk:R4" "$(run R3 "$MAP")" +echo "— caller remap —" +MAP='low=risk:1,medium=risk:2,high=risk:3,xhigh=risk:4,unknown=risk:ungraded' +eq "low remaps to risk:1" "risk:1" "$(run low "$MAP")" +eq "xhigh remaps to risk:4" "risk:4" "$(run xhigh "$MAP")" + +echo "— deprecated aliases remain compatible —" +LEGACY_MAP='R0=risk:low,R1=risk:medium,R2=risk:high,R3=risk:xhigh,unknown=risk:ungraded' +eq "legacy tier input normalizes to the canonical label" "risk:low" "$(run R0)" +eq "legacy LABEL_MAP keys still resolve" "risk:xhigh" "$(run xhigh "$LEGACY_MAP")" +REPO=test/repo PR_NUMBER=7 TIER=R3 LABEL_MAP="$LEGACY_MAP" DRY_RUN=1 \ + bash "$SCRIPT" >/dev/null 2>"$SANDBOX/legacy.err" +if grep -F 'deprecated' "$SANDBOX/legacy.err" >/dev/null; then + ok "legacy tier and label-map use emit deprecation warnings" +else + bad "legacy tier and label-map use emit deprecation warnings" "$(cat "$SANDBOX/legacy.err")" +fi echo "— validation refuses bad input before any write —" run R7 >/dev/null 2>&1; eq "bad tier exits 2" 2 "$?" -run R2 'R0=a,R1=b,R2=c,R3=d' >/dev/null 2>&1; eq "map missing unknown exits 2" 2 "$?" -run R2 'R0=,R1=b,R2=c,R3=d,unknown=e' >/dev/null 2>&1; eq "empty label exits 2" 2 "$?" -REPO='bad repo' PR_NUMBER=7 TIER=R1 DRY_RUN=1 bash "$SCRIPT" >/dev/null 2>&1 +run high 'low=a,medium=b,high=c,xhigh=d' >/dev/null 2>&1; eq "map missing unknown exits 2" 2 "$?" +run high 'low=,medium=b,high=c,xhigh=d,unknown=e' >/dev/null 2>&1; eq "empty label exits 2" 2 "$?" +REPO='bad repo' PR_NUMBER=7 TIER=medium DRY_RUN=1 bash "$SCRIPT" >/dev/null 2>&1 eq "bad repo exits 2" 2 "$?" -REPO=test/repo PR_NUMBER=x TIER=R1 DRY_RUN=1 bash "$SCRIPT" >/dev/null 2>&1 +REPO=test/repo PR_NUMBER=x TIER=medium DRY_RUN=1 bash "$SCRIPT" >/dev/null 2>&1 eq "bad pr number exits 2" 2 "$?" echo "— the write path: ONE atomic PUT of the whole label set —" @@ -52,7 +64,7 @@ echo "— the write path: ONE atomic PUT of the whole label set —" # can assert on the requests actually built. mkdir -p "$SANDBOX/bin" export GH_LOG="$SANDBOX/gh.log" CURRENT_LABELS="$SANDBOX/current.txt" -printf 'risk:R0\nkeep-me\n' > "$CURRENT_LABELS" +printf 'risk:low\nkeep-me\n' > "$CURRENT_LABELS" cat > "$SANDBOX/bin/gh" <<'STUB' #!/usr/bin/env bash # The real `gh api --paginate ... --jq '[.[].name]'` answers with a JSON ARRAY per page, and the @@ -83,10 +95,10 @@ STUB chmod +x "$SANDBOX/bin/gh" # Label names are still a PATH SEGMENT in the repo-label probe, and GitHub label names legally -# contain spaces, `/`, `#`, `?` and `%` — so a caller remap like `R3=risk high/urgent` must still be +# contain spaces, `/`, `#`, `?` and `%` — so a caller remap like `xhigh=risk high/urgent` must still be # encoded there, while the PUT carries the raw names as form fields. -MAP2='R0=risk:R0,R1=risk:R1,R2=risk:R2,R3=risk high/urgent,unknown=risk:ungraded' -out="$(PATH="$SANDBOX/bin:$PATH" REPO=test/repo PR_NUMBER=7 TIER=R3 LABEL_MAP="$MAP2" \ +MAP2='low=risk:low,medium=risk:medium,high=risk:high,xhigh=risk high/urgent,unknown=risk:ungraded' +out="$(PATH="$SANDBOX/bin:$PATH" REPO=test/repo PR_NUMBER=7 TIER=xhigh LABEL_MAP="$MAP2" \ bash "$SCRIPT" 2>/dev/null)" eq "the raw name is what gets returned/logged" "risk high/urgent" "$out" if grep -q 'risk%20high%2Furgent' "$GH_LOG"; then @@ -105,7 +117,7 @@ case "$put" in *) bad "and the FORM FIELD carries the raw name, not the encoding" "$put" ;; esac case "$put" in - *'risk:R0'*) bad "the stale owned label is absent from the PUT" "$put" ;; + *'risk:low'*) bad "the stale owned label is absent from the PUT" "$put" ;; *) ok "the stale owned label is absent from the PUT (it is dropped BY the replace)" ;; esac # The delete/add pair is precisely what the race exploited; neither may survive anywhere. @@ -123,12 +135,12 @@ echo "— the race shape: an EMPTY current set still syncs with a PUT, never a P # This is the assertion that pins the race closed. With no owned label present the tempting # "optimization" is an additive POST (nothing to remove, so why replace?). That reintroduces the # exact interleaving: run A and run B both read {}, both see nothing stale, both POST — and the PR -# ends up carrying risk:R1 AND risk:R2 at once. A PUT cannot do that: whichever writer lands second +# ends up carrying risk:medium AND risk:high at once. A PUT cannot do that: whichever writer lands second # replaces the set, so the PR always ends with exactly one owned label. : > "$GH_LOG"; : > "$CURRENT_LABELS" -outrace="$(PATH="$SANDBOX/bin:$PATH" REPO=test/repo PR_NUMBER=7 TIER=R1 bash "$SCRIPT" 2>/dev/null)" -eq "an empty current set still returns the target" "risk:R1" "$outrace" -if grep -q -- '-X PUT repos/test/repo/issues/7/labels -f labels\[\]=risk:R1' "$GH_LOG"; then +outrace="$(PATH="$SANDBOX/bin:$PATH" REPO=test/repo PR_NUMBER=7 TIER=medium bash "$SCRIPT" 2>/dev/null)" +eq "an empty current set still returns the target" "risk:medium" "$outrace" +if grep -q -- '-X PUT repos/test/repo/issues/7/labels -f labels\[\]=risk:medium' "$GH_LOG"; then ok "with nothing stale the write is STILL a PUT of the full set" else bad "with nothing stale the write is STILL a PUT of the full set" "$(tr '\n' '|' < "$GH_LOG")"; fi if grep -q -- '-X POST repos/test/repo/issues/7/labels' "$GH_LOG"; then @@ -136,11 +148,11 @@ if grep -q -- '-X POST repos/test/repo/issues/7/labels' "$GH_LOG"; then else ok "and never an additive POST (that is the interleaving)"; fi echo "— unowned labels are preserved verbatim, disputes included —" -: > "$GH_LOG"; printf 'risk:R1\nrisk-dispute\nbug\n' > "$CURRENT_LABELS" -PATH="$SANDBOX/bin:$PATH" REPO=test/repo PR_NUMBER=7 TIER=R3 bash "$SCRIPT" >/dev/null 2>&1 +: > "$GH_LOG"; printf 'risk:medium\nrisk-dispute\nbug\n' > "$CURRENT_LABELS" +PATH="$SANDBOX/bin:$PATH" REPO=test/repo PR_NUMBER=7 TIER=xhigh bash "$SCRIPT" >/dev/null 2>&1 put3="$(grep -- '-X PUT repos/test/repo/issues/7/labels ' "$GH_LOG")" eq "the PUT carries exactly the unowned labels plus the new target" \ - "api -X PUT repos/test/repo/issues/7/labels -f labels[]=risk-dispute -f labels[]=bug -f labels[]=risk:R3" \ + "api -X PUT repos/test/repo/issues/7/labels -f labels[]=risk-dispute -f labels[]=bug -f labels[]=risk:xhigh" \ "$put3" echo "— first use: the label is pre-created before the sync —" @@ -180,8 +192,8 @@ done exit 0 STUB chmod +x "$SANDBOX/bin404label/gh" -: > "$GH_LOG"; printf 'risk:R0\n' > "$CURRENT_LABELS" -PATH="$SANDBOX/bin404label:$PATH" REPO=test/repo PR_NUMBER=7 TIER=R2 bash "$SCRIPT" >/dev/null 2>&1 +: > "$GH_LOG"; printf 'risk:low\n' > "$CURRENT_LABELS" +PATH="$SANDBOX/bin404label:$PATH" REPO=test/repo PR_NUMBER=7 TIER=high bash "$SCRIPT" >/dev/null 2>&1 create_line="$(grep -n -- '-X POST repos/test/repo/labels ' "$GH_LOG" | head -1 | cut -d: -f1)" put_line="$(grep -n -- '-X PUT repos/test/repo/issues/7/labels ' "$GH_LOG" | head -1 | cut -d: -f1)" if [ -n "$create_line" ] && [ -n "$put_line" ] && [ "$create_line" -lt "$put_line" ]; then @@ -196,37 +208,45 @@ echo "— a PR the OLD race already double-labeled is HEALED, not read as in-syn # be, twice over: PRs graded before the atomic PUT landed can still be carrying two `risk:*` labels # right now, and a human can hand-add a second one at any time. A bare `has "$TARGET"` check calls # both of those states in-sync and writes nothing, so the contradiction never gets repaired. -: > "$GH_LOG"; printf 'risk:R0\nrisk:R2\nkeep-me\n' > "$CURRENT_LABELS" -PATH="$SANDBOX/bin:$PATH" REPO=test/repo PR_NUMBER=7 TIER=R2 bash "$SCRIPT" >/dev/null 2>&1 +: > "$GH_LOG"; printf 'risk:low\nrisk:high\nkeep-me\n' > "$CURRENT_LABELS" +PATH="$SANDBOX/bin:$PATH" REPO=test/repo PR_NUMBER=7 TIER=high bash "$SCRIPT" >/dev/null 2>&1 putheal="$(grep -- '-X PUT repos/test/repo/issues/7/labels ' "$GH_LOG")" eq "the extra owned label is squashed down to the one target" \ - "api -X PUT repos/test/repo/issues/7/labels -f labels[]=keep-me -f labels[]=risk:R2" \ + "api -X PUT repos/test/repo/issues/7/labels -f labels[]=keep-me -f labels[]=risk:high" \ "$putheal" -# Already-correct label: no write at all beyond the read. +echo "— deprecated default labels are retired during the first canonical re-grade —" : > "$GH_LOG"; printf 'risk:R2\nkeep-me\n' > "$CURRENT_LABELS" -outsync="$(PATH="$SANDBOX/bin:$PATH" REPO=test/repo PR_NUMBER=7 TIER=R2 bash "$SCRIPT" 2>/dev/null)" +PATH="$SANDBOX/bin:$PATH" REPO=test/repo PR_NUMBER=7 TIER=high bash "$SCRIPT" >/dev/null 2>&1 +putlegacy="$(grep -- '-X PUT repos/test/repo/issues/7/labels ' "$GH_LOG")" +eq "the old default label is replaced by its canonical label" \ + "api -X PUT repos/test/repo/issues/7/labels -f labels[]=keep-me -f labels[]=risk:high" \ + "$putlegacy" + +# Already-correct label: no write at all beyond the read. +: > "$GH_LOG"; printf 'risk:high\nkeep-me\n' > "$CURRENT_LABELS" +outsync="$(PATH="$SANDBOX/bin:$PATH" REPO=test/repo PR_NUMBER=7 TIER=high bash "$SCRIPT" 2>/dev/null)" rcsync=$? eq "an in-sync label writes nothing" 1 "$(wc -l < "$GH_LOG" | tr -d ' ')" eq "an in-sync run exits 0" 0 "$rcsync" -eq "an in-sync run still prints the target" "risk:R2" "$outsync" +eq "an in-sync run still prints the target" "risk:high" "$outsync" echo "— label identity is CASE-INSENSITIVE on GitHub, so the ownership match must be too —" -# GitHub will not let a repo hold both `risk:R2` and `Risk:R2` — they are the same label. A +# GitHub will not let a repo hold both `risk:high` and `Risk:high` — they are the same label. A # case-sensitive match therefore misreads a variant spelling (an older LABEL_MAP, or a # hand-created label) as "not the target AND not owned", which breaks the contract twice: the # in-sync short-circuit never fires so EVERY run issues the destructive PUT, and the variant is # carried through that PUT next to the new target — two `risk:*` labels, the exact state this # shape exists to make impossible. -: > "$GH_LOG"; printf 'Risk:R2\nkeep-me\n' > "$CURRENT_LABELS" -outci="$(PATH="$SANDBOX/bin:$PATH" REPO=test/repo PR_NUMBER=7 TIER=R2 bash "$SCRIPT" 2>/dev/null)" +: > "$GH_LOG"; printf 'Risk:high\nkeep-me\n' > "$CURRENT_LABELS" +outci="$(PATH="$SANDBOX/bin:$PATH" REPO=test/repo PR_NUMBER=7 TIER=high bash "$SCRIPT" 2>/dev/null)" eq "a case-variant of the TARGET reads as in-sync (no write)" 1 "$(wc -l < "$GH_LOG" | tr -d ' ')" -eq "and the run still prints the canonical target" "risk:R2" "$outci" -: > "$GH_LOG"; printf 'Risk:R0\nkeep-me\n' > "$CURRENT_LABELS" -PATH="$SANDBOX/bin:$PATH" REPO=test/repo PR_NUMBER=7 TIER=R2 bash "$SCRIPT" >/dev/null 2>&1 +eq "and the run still prints the canonical target" "risk:high" "$outci" +: > "$GH_LOG"; printf 'Risk:low\nkeep-me\n' > "$CURRENT_LABELS" +PATH="$SANDBOX/bin:$PATH" REPO=test/repo PR_NUMBER=7 TIER=high bash "$SCRIPT" >/dev/null 2>&1 putci="$(grep -- '-X PUT repos/test/repo/issues/7/labels ' "$GH_LOG")" eq "a case-variant of a STALE owned label is dropped, not carried through beside the target" \ - "api -X PUT repos/test/repo/issues/7/labels -f labels[]=keep-me -f labels[]=risk:R2" \ + "api -X PUT repos/test/repo/issues/7/labels -f labels[]=keep-me -f labels[]=risk:high" \ "$putci" echo "— a broken jq must fail the run, never degrade the PUT to target-only —" @@ -244,9 +264,9 @@ for a in "$@"; do [ "$a" = --argjson ] && { echo 'jq: error: synthetic failure' exec "$REAL_JQ" "$@" STUB chmod +x "$SANDBOX/binjqfail/jq" -: > "$GH_LOG"; printf 'risk:R0\nrisk-dispute\nkeep-me\n' > "$CURRENT_LABELS" +: > "$GH_LOG"; printf 'risk:low\nrisk-dispute\nkeep-me\n' > "$CURRENT_LABELS" REAL_JQ="$(command -v jq)" PATH="$SANDBOX/binjqfail:$PATH" \ - REPO=test/repo PR_NUMBER=7 TIER=R2 bash "$SCRIPT" >/dev/null 2>&1 + REPO=test/repo PR_NUMBER=7 TIER=high bash "$SCRIPT" >/dev/null 2>&1 eq "a failing carry-through filter exits 4" 4 "$?" if grep -q -- '-X PUT' "$GH_LOG"; then bad "and no PUT is issued at all" "$(grep -- '-X PUT' "$GH_LOG" | tr '\n' '|')" @@ -264,7 +284,7 @@ printf '%s\n' "$*" >> "$GH_LOG" for a in "$@"; do case "$a" in *issues/*/labels*) [ "${1:-}" = api ] && [[ " $* " != *" -X POST "* && " $* " != *" -X PUT "* ]] \ - && printf '["keep\\nme","risk:R0"]\n' + && printf '["keep\\nme","risk:low"]\n' exit 0 ;; esac done @@ -272,7 +292,7 @@ exit 0 STUB chmod +x "$SANDBOX/binnl/gh" : > "$GH_LOG" -PATH="$SANDBOX/binnl:$PATH" REPO=test/repo PR_NUMBER=7 TIER=R2 bash "$SCRIPT" >/dev/null 2>&1 +PATH="$SANDBOX/binnl:$PATH" REPO=test/repo PR_NUMBER=7 TIER=high bash "$SCRIPT" >/dev/null 2>&1 eq "the two-line name stays ONE label (a split would make three)" 2 \ "$(grep -o -- '-f labels\[\]=' "$GH_LOG" | wc -l | tr -d ' ')" if grep -q -- '-f labels\[\]=me' "$GH_LOG"; then @@ -293,9 +313,9 @@ for a in "$@"; do [ "$a" = -e ] && { echo 'jq: error: synthetic failure' >&2; ex exec "$REAL_JQ" "$@" STUB chmod +x "$SANDBOX/binjqerr/jq" -: > "$GH_LOG"; printf 'risk:R2\nrisk:R0\nkeep-me\n' > "$CURRENT_LABELS" +: > "$GH_LOG"; printf 'risk:high\nrisk:low\nkeep-me\n' > "$CURRENT_LABELS" REAL_JQ="$(command -v jq)" PATH="$SANDBOX/binjqerr:$PATH" \ - REPO=test/repo PR_NUMBER=7 TIER=R2 bash "$SCRIPT" >/dev/null 2>&1 + REPO=test/repo PR_NUMBER=7 TIER=high bash "$SCRIPT" >/dev/null 2>&1 eq "a failing membership check exits 4, never a silent in-sync 0" 4 "$?" if grep -q -- '-X PUT' "$GH_LOG"; then bad "and issues no PUT off an unanswered check" "$(grep -- '-X PUT' "$GH_LOG" | tr '\n' '|')" @@ -335,8 +355,8 @@ done exit 0 STUB chmod +x "$SANDBOX/bin403probe/gh" -: > "$GH_LOG"; printf 'risk:R0\nkeep-me\n' > "$CURRENT_LABELS" -probeerr="$(PATH="$SANDBOX/bin403probe:$PATH" REPO=test/repo PR_NUMBER=7 TIER=R2 \ +: > "$GH_LOG"; printf 'risk:low\nkeep-me\n' > "$CURRENT_LABELS" +probeerr="$(PATH="$SANDBOX/bin403probe:$PATH" REPO=test/repo PR_NUMBER=7 TIER=high \ bash "$SCRIPT" 2>&1 >/dev/null)" case "$probeerr" in *"NON-404"*"Resource not accessible by integration"*) @@ -386,7 +406,7 @@ done exit 0 STUB chmod +x "$SANDBOX/bin403/gh" -err="$(PATH="$SANDBOX/bin403:$PATH" REPO=test/repo PR_NUMBER=7 TIER=R1 bash "$SCRIPT" 2>&1 >/dev/null)" +err="$(PATH="$SANDBOX/bin403:$PATH" REPO=test/repo PR_NUMBER=7 TIER=medium bash "$SCRIPT" 2>&1 >/dev/null)" rc=$? eq "a 403 on the label sync exits 4" 4 "$rc" case "$err" in From da3f57437e5cee99fa5092871c335199a33d6857 Mon Sep 17 00:00:00 2001 From: huang47 <157390+huang47@users.noreply.github.com> Date: Mon, 31 Aug 2026 21:36:19 -0700 Subject: [PATCH 3/5] feat(pr-risk): render canonical tier names --- scripts/pr-risk/publish-risk-surfaces.sh | 72 +++++---- .../tests/test_publish_risk_surfaces.sh | 151 ++++++++++-------- 2 files changed, 126 insertions(+), 97 deletions(-) diff --git a/scripts/pr-risk/publish-risk-surfaces.sh b/scripts/pr-risk/publish-risk-surfaces.sh index efb8d033..87c8108e 100644 --- a/scripts/pr-risk/publish-risk-surfaces.sh +++ b/scripts/pr-risk/publish-risk-surfaces.sh @@ -17,18 +17,18 @@ # even that as a warning on the target rather than a failed grade. # # ── WHY A PER-FILE BREAKDOWN AT ALL ──────────────────────────────────────────────────────────── -# A bare `risk:R3` is unexplained: the only way to learn why was to open the Actions run. The -# concentration sentence ("94% of this diff is R0/R1; the 6% that makes it R3 is these two files, -# 40 lines") is what makes an R3 actionable rather than opaque, and what tells a reviewer when an -# R3 is a technicality. +# A bare `risk:xhigh` is unexplained: the only way to learn why was to open the Actions run. The +# concentration sentence ("94% of this diff is low/medium; the 6% that makes it xhigh is these two files, +# 40 lines") is what makes an xhigh actionable rather than opaque, and what tells a reviewer when an +# xhigh is a technicality. # # THE BREAKDOWN IS THE PATH AXIS, AND SAYS SO. The shipped grade is # worst(path_floor, provenance, reversibility) and only the PATH axis is per-file at all — the # other two are properties of the PR, not of any file. So each row carries that file's PATH # FLOOR, and whenever the headline tier is above the path floor the sentence names the axis that # supplied it and quotes its reason. That is what keeps the sentence consistent with the headline -# it sits under: without it, a diff of nothing but docs can headline R2 (no green checks) directly -# above "all 40 changed lines are R0", and the explanation contradicts the tier it explains. +# it sits under: without it, a diff of nothing but docs can headline high (no green checks) directly +# above "all 40 changed lines are low", and the explanation contradicts the tier it explains. # The per-file floors come from grade-pr-risk.sh (`risk.axes.path_floor.files`), computed from # the SAME rules as the floor itself, so this file re-derives nothing and cannot drift into a # second grading model. @@ -79,7 +79,7 @@ # REPO owner/name of the repo holding the PR (required) # PR_NUMBER the PR number (required) # RECORD path to the graded record from grade-pr-risk.sh (required) -# `{}` / an unreadable record renders the UNKNOWN surfaces — never R0. +# `{}` / an unreadable record renders the UNKNOWN surfaces — never low. # HEAD_SHA head commit for the Check Run (read from the API when empty) # PUBLISH_CHECK 1 = create the Check Run (default 0) # PUBLISH_COMMENT 1 = create/update the sticky comment (default 0) @@ -159,7 +159,24 @@ cat <<'JQ' (flat | if length > 160 then .[0:160] + "…" else . end) | if test("[`\\\\]") then md_escape else "`" + gsub("\\|"; "\\|") + "`" end; - def tier_rank: {"R0":0,"R1":1,"R2":2,"R3":3}[.] // 3; + def canonical_tier: + if type != "string" then . + else {"R0":"low","R1":"medium","R2":"high","R3":"xhigh"}[.] // . end; + def canonical_axis: + if type != "object" then . + else (if has("tier") then .tier |= canonical_tier else . end) + | (if has("residual_tier") then .residual_tier |= canonical_tier else . end) + | (if (.files | type) == "array" + then .files |= map(if type == "object" and has("tier") then .tier |= canonical_tier else . end) + else . end) + end; + def canonical_risk: + if type != "object" then . + else (if has("tier") then .tier |= canonical_tier else . end) + | (if (.axes | type) == "object" then .axes |= map_values(canonical_axis) else . end) + end; + def tier_rank: + canonical_tier | if type == "string" then {"low":0,"medium":1,"high":2,"xhigh":3}[.] // 3 else 3 end; # Does an axis TIE the headline? One definition, called once per axis, so the tie # classification, the attribution tail and the reducibility gate can never disagree about # which axes tied. Takes the headline as a parameter rather than closing over `$tier` @@ -168,20 +185,21 @@ cat <<'JQ' # a boolean or a number raises "Cannot index object with boolean" — a hard jq error that its # own `// 3` fallback cannot catch, aborting the whole render rather than degrading. The old # per-axis `($A2.tier // null)` normalization this replaced covered the `false` case only; a - # type test covers every non-tier shape, and is identical on the R0–R3 strings the grader emits. + # type test covers every non-tier shape, and is identical on the low–xhigh strings the grader emits. def ties($t; $headline): (($t | type) == "string") and (($headline | type) == "string") and (($t | tier_rank) == ($headline | tier_rank)); def pct($p; $w): if $w <= 0 then 0 else (100 * $p / $w) | round end; . as $r - | ($r.risk // null) as $R + # Historical records remain renderable, but every surface uses canonical tier names. + | (($r.risk // null) | canonical_risk) as $R | (($R.status // "unknown") == "ok" and ($R.tier // null) != null) as $graded | ($R.tier // null) as $tier | ($R.axes.path_floor // null) as $A1 | ($R.axes.provenance // null) as $A2 | ($R.axes.reversibility // null) as $A3 - | ($A1.tier // "R0") as $floor + | ($A1.tier // "low") as $floor | ([$A1.files[]? | . + {lines: ((.additions // 0) + (.deletions // 0))}]) as $files | ([$files[] | .lines] | add // 0) as $total | ([$files[] | select((.tier | tier_rank) >= ($floor | tier_rank))]) as $topf @@ -197,7 +215,7 @@ cat <<'JQ' # follows the remainder); a reversibility tie is a property of specific FILES, which may be # exactly the files the clause proposes peeling off. Bound separately here so the gate can # treat them differently; `$drivers` keeps the combined list because the attribution tail's - # wording ("the provenance and reversibility axis proposes R3") is about which axes tied, and + # wording ("the provenance and reversibility axis proposes xhigh") is about which axes tied, and # is correct either way. Order is provenance-then-reversibility because `$drivers[0].r` is the # reason both the tail and the headline print. | ties($A2.tier; $tier) as $prov_tie @@ -210,20 +228,20 @@ cat <<'JQ' # the clause peels — the remainder provably carries neither the path floor nor the reversibility # reason, so suppressing the clause would be a false negative rather than caution. # - # `null` (an older record, or the R2/R1 rungs, where "no green rollup" is a property of the + # `null` (an older record, or the high/medium rungs, where "no green rollup" is a property of the # HEAD COMMIT and "no test touched" of the whole change set) reads as NOT removable — the # fail-safe back to the unconditional suppression this replaces. # # THE SUBSET TEST IS LOAD-BEARING, not a formality: a consumer map override can put an - # irreversible-class file BELOW the path floor (remap `migrations` to R1 while leaving it in + # irreversible-class file BELOW the path floor (remap `migrations` to medium while leaving it in # `irreversible_classes`), and there peeling `$topf` leaves the reversibility reason exactly # where it was. `[]` is a subset of every set, so it is rejected by the non-empty test rather # than reading as "yes, removable" — the same direction the grader's own `null` fallback points. # # AND THE SUBSET TEST IS NOT ENOUGH ON ITS OWN. It proves the reason CURRENTLY ATTRIBUTED goes # with the peel; it does not prove the axis lands lower, because the rungs the remainder falls - # to are map-configurable and a consumer may have raised one (`no_green_checks_tier: "R3"` puts - # the remainder straight back on R3, and the head commit's rollup is not something a peel can + # to are map-configurable and a consumer may have raised one (`no_green_checks_tier: "xhigh"` puts + # the remainder straight back on xhigh, and the head commit's rollup is not something a peel can # change). `axes.reversibility.residual_tier` is the grader's own map-aware bound on where the # axis lands after the peel; requiring it to rank BELOW the headline is what turns "this reason # is removable" into "this tie is removable". Missing (a record graded before the field existed) @@ -245,7 +263,7 @@ cat <<'JQ' | all($rf[]; (type == "string") and ($keep[.] == true)))) as $rev_removable # THE PATH AXIS DECIDED: it is graded, the headline IS the floor, and no tie survives the peel. # A provenance tie deliberately still reads as "not decided by path" — if provenance also - # proposes R3, peeling the R3 files out changes nothing, and promising a reduction there would + # proposes xhigh, peeling the xhigh files out changes nothing, and promising a reduction there would # be the one wrong answer. A reversibility tie reads the same way UNLESS its own attributed # files are all inside `$topf` AND the axis provably lands lower once they go, in which case the # split removes both reasons at once. This is the SPLIT-ELIGIBILITY half; `$pitches_split` below @@ -264,14 +282,14 @@ cat <<'JQ' # number that separates "peel 2 files and the rest rubber-stamps" from "peel 2 files and the # rest is still a normal review" is not otherwise recoverable without re-grading by hand. | ([$files[] | select((.tier | tier_rank) < ($floor | tier_rank))]) as $below - # The printed label is the RECORD's own tier string, never `"R\(rank)"` re-rendered from the - # number: `tier_rank` deliberately collapses anything outside R0–R3 to 3, so re-rendering would - # print an unrecognized tier as a confident `R3`. `max_by` applies the same worst-of rule while + # The printed label is the RECORD's own tier string, never reconstructed from the + # number: `tier_rank` deliberately collapses anything outside low–xhigh to 3, so re-rendering would + # print an unrecognized tier as a confident `xhigh`. `max_by` applies the same worst-of rule while # keeping the label tied to the data rather than to the enum's present shape. | (if ($below | length) > 0 then ($below | max_by(.tier | tier_rank) | .tier) else null end) as $comp_tier # The below-floor share, bound once because the clause and the sentence it hangs off have to # agree about it. A remainder that ROUNDS TO 0% of the diff gets no split pitch, or the sentence - # reads "**0%** of this diff is R0/R1/R2 … peeled into their own PR, the remaining 1 file(s)" — + # reads "**0%** of this diff is low/medium/high … peeled into their own PR, the remaining 1 file(s)" — # offering to relocate a line it has just called nothing. The gate is the sentence's OWN printed # number rather than a threshold picked here, so the two can never disagree. | (pct($total - $toplines; $total)) as $below_pct @@ -297,7 +315,7 @@ cat <<'JQ' elif ($floor | tier_rank) == 0 or ($total - $toplines) <= 0 then "All \($total) changed lines sit at \($floor) on the path axis." else "**\($below_pct)% of this diff is " - + ([range(0; ($floor | tier_rank))] | map("R\(.)") | join("/")) + + (["low","medium","high","xhigh"][0:($floor | tier_rank)] | join("/")) + "**; the \(pct($toplines; $total))% that puts the path floor at \($floor) is " + "\($topf | length) file(s), \($toplines) lines" # A FLOOR WITH ITS ASSUMPTIONS NAMED, never a promised grade. The OTHER TWO AXES are @@ -341,7 +359,7 @@ cat <<'JQ' "", ($R.reason // "the graded record was empty or unreadable — nothing was graded" | md_text(1000)), "", - "An ungradable PR is reported as `unknown`, never defaulted to `R0`: a PR whose inputs we could not read is exactly the PR that might touch auth. Push again or re-run the check to retry.", + "An ungradable PR is reported as `unknown`, never defaulted to `low`: a PR whose inputs we could not read is exactly the PR that might touch auth. Push again or re-run the check to retry.", "", "This check is advisory: its conclusion is always `neutral`, so it never fails and never blocks a merge." ] | join("\n")) @@ -364,7 +382,7 @@ cat <<'JQ' "<sub>Advisory · never fails a check or blocks a merge · re-grades on every push, updated in place.</sub>" ]) as $tail # WHICH AXIS TO NAME IN THE HEADLINE, and its human reason. `$R.reason` is deliberately NOT - # used here: it is the machine trace ("worst of path_floor=R1, provenance=R2, ..."), which + # used here: it is the machine trace ("worst of path_floor=medium, provenance=high, ..."), which # restates the tier instead of explaining it. The formula and that trace both still appear # inside the <details>, so nothing is lost by leading with the axis that actually decided. # BOTH AXES, when a removable reversibility tie let the clause speak. This branch is checked @@ -395,14 +413,14 @@ cat <<'JQ' then {n: "path", r: ($A1.reason // ""), path: true} else {n: null, r: "", path: false} end) as $driver # The concentration clause, cut to a headline-sized fragment — and shown ONLY when the path - # axis is what drove the tier. Quoting "14% of lines set the path floor at R1" under an R2 + # axis is what drove the tier. Quoting "14% of lines set the path floor at medium" under a high # headline that provenance produced points the reader at the wrong number. # IT COUNTS PATH-FLOOR FILES, so it says so. Under the combined `path and reversibility` # headline, `axes.reversibility.files` may be a strict SUBSET of $topf (the migration supplied # the reversibility reason; the CI file beside it only met the path floor) — so "N file(s) # carry it", with "it" reading back to a headline naming both axes, would credit more files # with the reversibility reason than actually supplied it. The path-floor count is the honest - # one for both spellings, and it matches the long form's "the X% that puts the path floor at R3 + # one for both spellings, and it matches the long form's "the X% that puts the path floor at xhigh # is N file(s)" word for word. | (if $driver.path and ($files | length) > 0 and $total > 0 @@ -429,7 +447,7 @@ cat <<'JQ' "| file | +/- | path tier | matched classes |", "|---|---|---|---|" ]) else [ $marker, "", - "**Risk `unknown`** · advisory — this pull request could not be graded: \($R.reason // "the graded record was empty or unreadable" | md_text(240)). An ungradable PR is never defaulted to `R0`; push again or re-run to retry." ] + "**Risk `unknown`** · advisory — this pull request could not be graded: \($R.reason // "the graded record was empty or unreadable" | md_text(240)). An ungradable PR is never defaulted to `low`; push again or re-run to retry." ] end) as $head | ([ $files[] ] | sort_by([ -(.tier | tier_rank), -(.lines) ])) as $shown | ([ $shown[0:50][] diff --git a/scripts/pr-risk/tests/test_publish_risk_surfaces.sh b/scripts/pr-risk/tests/test_publish_risk_surfaces.sh index e2b3f851..8c43fe30 100644 --- a/scripts/pr-risk/tests/test_publish_risk_surfaces.sh +++ b/scripts/pr-risk/tests/test_publish_risk_surfaces.sh @@ -47,7 +47,7 @@ FIXTURE_FAIL="$SANDBOX/fixture-build-failed" # every fixture written before they existed keeps rendering exactly as it did — which is the # backward-compatibility case the suite pins below. record() { - local tier="$1" floor="$2" files="$3" prov="${4:-R1}" rev="${5:-R1}" revfiles="${6:-null}" + local tier="$1" floor="$2" files="$3" prov="${4:-medium}" rev="${5:-medium}" revfiles="${6:-null}" local revreason="${7:-checks green but the diff touches no test file}" revres="${8:-null}" local f="$SANDBOX/rec-$RANDOM.json" ff="$SANDBOX/files-$RANDOM.json" # The files array goes in via a FILE, not --argjson. Linux caps a single argv entry at 128KiB @@ -75,13 +75,24 @@ file_entry() { jq -n --arg p "$1" --arg t "$2" --argjson a "$3" --argjson d "$4" echo "— the sticky marker is ONE constant: rendered head == what find_sticky matches —" # If these ever drift, every push POSTs a NEW comment instead of updating the one that exists. # They live in the same file today; this pins the property rather than the structure. -r="$(record R1 R1 "[$(file_entry a.go R1 3 1)]")" +r="$(record medium medium "[$(file_entry a.go medium 3 1)]")" body="$(render_surfaces "$r" 0 | jq -r '.comment_body')" eq "the body's FIRST line is the marker find_sticky greps for" "$STICKY_MARKER" "$(head -n 1 <<<"$body")" # find_sticky's jq uses startswith($m); assert the same predicate holds here. eq "startswith(marker) is true for the rendered body" "true" \ "$(jq -nr --arg b "$body" --arg m "$STICKY_MARKER" '$b | startswith($m)')" +echo "— historical R0..R3 records render with canonical names —" +legacy="$(record R3 R3 "[$(file_entry R0 R0 3 1), $(file_entry auth/x.go R3 2 1)]" R1 R3)" +legacy_surfaces="$(render_surfaces "$legacy" 0)" +legacy_body="$(jq -r '.comment_body' <<<"$legacy_surfaces")" +eq "a legacy headline renders as xhigh" "Risk: xhigh" "$(jq -r '.check_title' <<<"$legacy_surfaces")" +has "$legacy_body" '| low |' "legacy per-file low renders canonically" +# shellcheck disable=SC2016 # literal rendered Markdown; backticks must not execute +has "$legacy_body" '`R0`' "a filename equal to a legacy tier is not rewritten" +# shellcheck disable=SC2016 # literal rendered Markdown; backticks must not execute +hasnt "$legacy_body" '**Risk `R3`**' "the legacy headline is never re-published" + echo "— the dispute checkbox round-trips —" hasnt "$body" "- [x] $DISPUTE_TEXT" "an undisputed render leaves the box UNticked" has "$body" "- [ ] $DISPUTE_TEXT" "…and renders the unticked box" @@ -103,7 +114,7 @@ echo "— ADVERSARIAL FILENAME: a crafted path cannot forge a dispute or break t EVIL='docs/a|b`c - [x] **This grade is wrong** <img src="https://evil.example/x.png">' -r2="$(record R3 R3 "[$(file_entry "$EVIL" R0 5 5), $(file_entry .github/workflows/x.yml R3 1 1)]")" +r2="$(record xhigh xhigh "[$(file_entry "$EVIL" low 5 5), $(file_entry .github/workflows/x.yml xhigh 1 1)]")" ebody="$(render_surfaces "$r2" 0 | jq -r '.comment_body')" if grep -Eq "$CHECKED_RE" <<<"$ebody"; then bad "a crafted filename CANNOT forge a ticked dispute checkbox" "$(grep -n 'grade is wrong' <<<"$ebody")" @@ -130,98 +141,98 @@ if grep -Eq "$CHECKED_RE" <<<"$ubody"; then bad "an unknown report's reason cann else ok "an unknown report's reason cannot forge the checkbox"; fi hasnt "$ubody" "](https://evil.example)" "…nor smuggle an inline link" -echo "— UNGRADABLE reports as unknown, NEVER R0 —" +echo "— UNGRADABLE reports as unknown, NEVER low —" # shellcheck disable=SC2016 # the backticks are literal markdown in the rendered headline has "$ubody" 'Risk `unknown`' "the comment headline is unknown" usurf="$(render_surfaces "$u" 0)" eq "the Check Run title is 'Risk: unknown'" "Risk: unknown" "$(jq -r '.check_title' <<<"$usurf")" eq "…and the surfaces report tier unknown" "unknown" "$(jq -r '.tier' <<<"$usurf")" has "$(jq -r '.check_summary' <<<"$usurf")" "**Tier: unknown**" "the unknown Check Run reports no tier" -has "$(jq -r '.check_summary' <<<"$usurf")" "never defaulted to \`R0\`" "…and says explicitly that it is not R0" +has "$(jq -r '.check_summary' <<<"$usurf")" "never defaulted to \`low\`" "…and says explicitly that it is not low" # An EMPTY record (what grade-targets writes for an unreadable PR) takes the same branch. printf '{}' > "$SANDBOX/empty.json" -eq "an empty record renders unknown, not R0" "unknown" "$(render_surfaces "$SANDBOX/empty.json" 0 | jq -r '.tier')" +eq "an empty record renders unknown, not low" "unknown" "$(render_surfaces "$SANDBOX/empty.json" 0 | jq -r '.tier')" eq "a non-JSON record renders unknown too" "unknown" \ "$(printf 'not json' > "$SANDBOX/bad.json"; render_surfaces "$SANDBOX/bad.json" 0 | jq -r '.tier')" echo "— the concentration sentence is CONSISTENT with the headline tier —" -# 600 lines of R0 docs + 40 lines of R3 CI: the sentence must name the 6% that lifts the floor. -big="[$(file_entry docs/a.md R0 500 100), $(file_entry .github/workflows/d.yml R3 30 10)]" -c="$(render_surfaces "$(record R3 R3 "$big")" 0 | jq -r '.concentration')" -has "$c" "94% of this diff is R0/R1/R2" "it names the share BELOW the floor" -has "$c" "the 6% that puts the path floor at R3 is 1 file(s), 40 lines" "…and the files that put it there" +# 600 lines of low docs + 40 lines of xhigh CI: the sentence must name the 6% that lifts the floor. +big="[$(file_entry docs/a.md low 500 100), $(file_entry .github/workflows/d.yml xhigh 30 10)]" +c="$(render_surfaces "$(record xhigh xhigh "$big")" 0 | jq -r '.concentration')" +has "$c" "94% of this diff is low/medium/high" "it names the share BELOW the floor" +has "$c" "the 6% that puts the path floor at xhigh is 1 file(s), 40 lines" "…and the files that put it there" # The headline can exceed the path floor — the other two axes propose independently. Saying only -# "all 600 lines are R0" above a `risk:R2` headline would contradict the tier it explains. -c2="$(render_surfaces "$(record R2 R0 "[$(file_entry docs/a.md R0 600 0)]" R1 R2)" 0 | jq -r '.concentration')" -has "$c2" "All 600 changed lines sit at R0 on the path axis." "a floor-only diff says so" -has "$c2" "headline tier is R2 rather than the path floor R0 because the reversibility axis" \ +# "all 600 lines are low" above a `risk:high` headline would contradict the tier it explains. +c2="$(render_surfaces "$(record high low "[$(file_entry docs/a.md low 600 0)]" medium high)" 0 | jq -r '.concentration')" +has "$c2" "All 600 changed lines sit at low on the path axis." "a floor-only diff says so" +has "$c2" "headline tier is high rather than the path floor low because the reversibility axis" \ "…and names the axis that actually supplied the headline" eq "a diff with no counted lines says so" \ "This diff changes no counted lines across 1 file(s)." \ - "$(render_surfaces "$(record R0 R0 "[$(file_entry a.md R0 0 0)]" R0 R0)" 0 | jq -r '.concentration')" + "$(render_surfaces "$(record low low "[$(file_entry a.md low 0 0)]" low low)" 0 | jq -r '.concentration')" echo "— the concentration sentence carries the COMPLEMENT floor: what a split would actually buy —" -# The share below the floor is not a verdict on its own: "94% is R0/R1/R2" is equally true of a -# remainder that rubber-stamps at R1 and one that is still a normal R2 review. The complement floor +# The share below the floor is not a verdict on its own: "94% is low/medium/high" is equally true of a +# remainder that rubber-stamps at medium and one that is still a normal high review. The complement floor # is the number that separates them, and it is the one an author cannot recover without re-grading # the map by hand. -has "$c" "peeled into their own PR, the remaining 1 file(s) would path-floor at **R0**" \ - "600 R0 doc lines under an R3 CI floor report an R0 remainder" +has "$c" "peeled into their own PR, the remaining 1 file(s) would path-floor at **low**" \ + "600 low doc lines under an xhigh CI floor report a low remainder" has "$c" "(final grade still depends on the provenance and reversibility axes at PR time)" \ "…as a FLOOR with its assumptions named, never a promised grade" # NOT CLAMPED to the other axes, and the caveat is why: provenance and reversibility are both # re-derived for the split PR and can move in either direction, so max(path, provenance, # reversibility) would be no more a floor for the remainder than the path number alone. Path floor -# R3 over provenance R1 / reversibility R2 still reports the remainder's own R0. -has "$(render_surfaces "$(record R3 R3 "$big" R1 R2)" 0 | jq -r '.concentration')" \ - "the remaining 1 file(s) would path-floor at **R0**" \ +# xhigh over provenance medium / reversibility high still reports the remainder's own low. +has "$(render_surfaces "$(record xhigh xhigh "$big" medium high)" 0 | jq -r '.concentration')" \ + "the remaining 1 file(s) would path-floor at **low**" \ "a lower-ranked provenance/reversibility does NOT clamp the path-axis number" -# Worst-of over the remainder, not the biggest or the last file: 500 R0 lines cannot cancel 100 R1 +# Worst-of over the remainder, not the biggest or the last file: 500 low lines cannot cancel 100 medium # ones, exactly as the floor itself is a max rather than last-match-wins. -mix="[$(file_entry docs/a.md R0 500 0), $(file_entry src/app.ts R1 100 0), $(file_entry .github/workflows/d.yml R3 30 10)]" -has "$(render_surfaces "$(record R3 R3 "$mix")" 0 | jq -r '.concentration')" \ - "the remaining 2 file(s) would path-floor at **R1**" \ - "a mixed remainder takes its WORST per-file floor (R0 + R1 -> R1), over both files" +mix="[$(file_entry docs/a.md low 500 0), $(file_entry src/app.ts medium 100 0), $(file_entry .github/workflows/d.yml xhigh 30 10)]" +has "$(render_surfaces "$(record xhigh xhigh "$mix")" 0 | jq -r '.concentration')" \ + "the remaining 2 file(s) would path-floor at **medium**" \ + "a mixed remainder takes its WORST per-file floor (low + medium -> medium), over both files" # The ticket's worked example, and the case that makes the readout worth printing: the remainder is -# still R2, so peeling the CI file out buys a normal review rather than the R1 rubber-stamp lane. -worked="[$(file_entry src/app.ts R2 200 0), $(file_entry package.json R3 4 0), $(file_entry .github/workflows/ci.yml R3 10 0)]" -has "$(render_surfaces "$(record R3 R3 "$worked")" 0 | jq -r '.concentration')" \ - "the remaining 1 file(s) would path-floor at **R2**" \ - "an R2 remainder says R2 — the split that is NOT worth much still reports honestly" +# still high, so peeling the CI file out buys a normal review rather than the medium rubber-stamp lane. +worked="[$(file_entry src/app.ts high 200 0), $(file_entry package.json xhigh 4 0), $(file_entry .github/workflows/ci.yml xhigh 10 0)]" +has "$(render_surfaces "$(record xhigh xhigh "$worked")" 0 | jq -r '.concentration')" \ + "the remaining 1 file(s) would path-floor at **high**" \ + "a high remainder says high — the split that is NOT worth much still reports honestly" echo "— …and stays SILENT where a split cannot help —" # IRREDUCIBLE: every changed line is already at the floor, so there is no remainder to peel. The # existing wording is the whole answer; a clause here would offer a split that does not exist. -irr="$(render_surfaces "$(record R3 R3 "[$(file_entry .github/workflows/a.yml R3 20 0), $(file_entry .github/workflows/b.yml R3 20 0)]")" 0 | jq -r '.concentration')" +irr="$(render_surfaces "$(record xhigh xhigh "[$(file_entry .github/workflows/a.yml xhigh 20 0), $(file_entry .github/workflows/b.yml xhigh 20 0)]")" 0 | jq -r '.concentration')" eq "an irreducible diff renders byte-identically to before this clause existed" \ - "All 40 changed lines sit at R3 on the path axis." "$irr" + "All 40 changed lines sit at xhigh on the path axis." "$irr" # NOT PATH-DECIDED: provenance supplied the headline, so peeling the top path files leaves the tier # where it is. Quoting a path-axis reduction under it would point the reader at the wrong number. -# Its own fixture rather than $big, because the floor here is R2 and grade-pr-risk.sh derives the -# floor as `worst` over the SAME per-file rules: a record whose floor is R2 while a file on it reads -# R3 cannot be graded, so reusing $big would pin the clause against an input production never emits. -path_r2="[$(file_entry docs/a.md R0 500 100), $(file_entry src/app.ts R2 30 10)]" -np="$(render_surfaces "$(record R3 R2 "$path_r2" R3 R1)" 0 | jq -r '.concentration')" +# Its own fixture rather than $big, because the floor here is high and grade-pr-risk.sh derives the +# floor as `worst` over the SAME per-file rules: a record whose floor is high while a file on it reads +# xhigh cannot be graded, so reusing $big would pin the clause against an input production never emits. +path_r2="[$(file_entry docs/a.md low 500 100), $(file_entry src/app.ts high 30 10)]" +np="$(render_surfaces "$(record xhigh high "$path_r2" xhigh medium)" 0 | jq -r '.concentration')" hasnt "$np" "peeled into their own PR" "a headline another axis supplied gets NO reducibility clause" -has "$np" "40 lines. The headline tier is R3 rather than the path floor R2" \ +has "$np" "40 lines. The headline tier is xhigh rather than the path floor high" \ "…and the sentence ends exactly as it did before, straight into the axis attribution" # A PROVENANCE TIE is not "the path axis decided": provenance is a property of the AUTHOR, so if it -# also proposes R3 the remainder is R3 too and the split buys nothing. This is the case a rank +# also proposes xhigh the remainder is xhigh too and the split buys nothing. This is the case a rank # comparison alone would get wrong. -tied="$(render_surfaces "$(record R3 R3 "$big" R3 R1)" 0 | jq -r '.concentration')" +tied="$(render_surfaces "$(record xhigh xhigh "$big" xhigh medium)" 0 | jq -r '.concentration')" hasnt "$tied" "peeled into their own PR" "an axis TIED with the path floor also suppresses the clause" echo "— …but a REVERSIBILITY tie the peel would remove lets the clause speak (BE-7419) —" # A reversibility tie is a property of specific FILES, not of the author — and those files can be -# exactly the ones the clause proposes peeling. One R3 migration under 600 R0 doc lines rendered no -# clause at all, while the identical file set at reversibility R1 rendered the full split pitch: +# exactly the ones the clause proposes peeling. One xhigh migration under 600 low doc lines rendered no +# clause at all, while the identical file set at reversibility medium rendered the full split pitch: # the same peel, described two ways, because the gate could not tell the two ties apart. -mig="[$(file_entry docs/a.md R0 500 100), $(file_entry migrations/0042_drop.sql R3 30 10 '["migrations"]')]" +mig="[$(file_entry docs/a.md low 500 100), $(file_entry migrations/0042_drop.sql xhigh 30 10 '["migrations"]')]" MIG_WHY="touches migrations — mutates persistent state or deletes data; reverting the code does not restore it" -rev_surf="$(render_surfaces "$(record R3 R3 "$mig" R1 R3 '["migrations/0042_drop.sql"]' "$MIG_WHY" '"R1"')" 0)" +rev_surf="$(render_surfaces "$(record xhigh xhigh "$mig" medium xhigh '["migrations/0042_drop.sql"]' "$MIG_WHY" '"medium"')" 0)" rev_conc="$(jq -r '.concentration' <<<"$rev_surf")" rev_body="$(jq -r '.comment_body' <<<"$rev_surf")" -has "$rev_conc" "peeled into their own PR, the remaining 1 file(s) would path-floor at **R0**" \ +has "$rev_conc" "peeled into their own PR, the remaining 1 file(s) would path-floor at **low**" \ "a reversibility tie whose attributed files are ALL inside the peeled set gets the clause" has "$rev_conc" "(final grade still depends on the provenance and reversibility axes at PR time)" \ "…keeping the caveat, which stays honest: the remainder re-derives reversibility on its own checks" @@ -233,21 +244,21 @@ has "$rev_body" "6% of 640 changed lines set the path floor (1 file(s))." \ "…and \$conc_short fires for the combined driver, not just plain 'path'" # THE CONSUMER-OVERRIDE CASE `residual_tier` exists for. Same fully-attributed, fully-peeled tie — -# but the consumer's map sets `no_green_checks_tier: "R3"`, so the grader reports the axis lands -# back on R3 once the migration is peeled. The subset test alone still says "removable" here; only +# but the consumer's map sets `no_green_checks_tier: "xhigh"`, so the grader reports the axis lands +# back on xhigh once the migration is peeled. The subset test alone still says "removable" here; only # the residual bound catches that the promised reduction cannot happen. -ovr="$(render_surfaces "$(record R3 R3 "$mig" R1 R3 '["migrations/0042_drop.sql"]' "$MIG_WHY" '"R3"')" 0)" +ovr="$(render_surfaces "$(record xhigh xhigh "$mig" medium xhigh '["migrations/0042_drop.sql"]' "$MIG_WHY" '"xhigh"')" 0)" hasnt "$(jq -r '.concentration' <<<"$ovr")" "peeled into their own PR" \ "a removable tie whose residual_tier does NOT drop below the headline stays SILENT" has "$(jq -r '.comment_body' <<<"$ovr")" "**reversibility**: touches migrations" \ "…and the headline credits reversibility alone, as it did before BE-7419" # THE CONSUMER-OVERRIDE CASE the full-subset test exists for. A map override can put an -# irreversible-class file BELOW the path floor — remap `migrations` to R1 while leaving it in +# irreversible-class file BELOW the path floor — remap `migrations` to medium while leaving it in # `irreversible_classes` — and there peeling $topf (the CI file) leaves the reversibility reason # exactly where it was. A "does reversibility name any peeled file?" test would speak here wrongly. -override="[$(file_entry docs/a.md R0 500 100), $(file_entry migrations/0042_drop.sql R1 20 0 '["migrations"]'), $(file_entry .github/workflows/ci.yml R3 30 10)]" -ov_surf="$(render_surfaces "$(record R3 R3 "$override" R1 R3 '["migrations/0042_drop.sql"]' "touches migrations" '"R1"')" 0)" +override="[$(file_entry docs/a.md low 500 100), $(file_entry migrations/0042_drop.sql medium 20 0 '["migrations"]'), $(file_entry .github/workflows/ci.yml xhigh 30 10)]" +ov_surf="$(render_surfaces "$(record xhigh xhigh "$override" medium xhigh '["migrations/0042_drop.sql"]' "touches migrations" '"medium"')" 0)" hasnt "$(jq -r '.concentration' <<<"$ov_surf")" "peeled into their own PR" \ "a reversibility tie attributed to a file BELOW the floor keeps the clause SILENT" has "$(jq -r '.comment_body' <<<"$ov_surf")" "**reversibility**: touches migrations" \ @@ -255,46 +266,46 @@ has "$(jq -r '.comment_body' <<<"$ov_surf")" "**reversibility**: touches migra hasnt "$(jq -r '.comment_body' <<<"$ov_surf")" "changed lines set the path floor" \ "…with no above-the-fold split fragment either" -# BACKWARD COMPATIBILITY, pinned: `files` is absent/null on the R2 and R1 rungs (properties of the +# BACKWARD COMPATIBILITY, pinned: `files` is absent/null on the high and medium rungs (properties of the # head commit and of the whole change set, removable by dropping no files) and on every record # graded before BE-7418. Those must all fail SAFE, back to the unconditional suppression. -hasnt "$(render_surfaces "$(record R3 R3 "$mig" R1 R3)" 0 | jq -r '.concentration')" "peeled into their own PR" \ - "a reversibility tie carrying files:null (an R2-style tie, or a pre-BE-7418 record) stays SILENT" -hasnt "$(render_surfaces "$(record R3 R3 "$mig" R1 R3 '[]' "$MIG_WHY" '"R1"')" 0 | jq -r '.concentration')" "peeled into their own PR" \ +hasnt "$(render_surfaces "$(record xhigh xhigh "$mig" medium xhigh)" 0 | jq -r '.concentration')" "peeled into their own PR" \ + "a reversibility tie carrying files:null (a high-style tie, or a pre-BE-7418 record) stays SILENT" +hasnt "$(render_surfaces "$(record xhigh xhigh "$mig" medium xhigh '[]' "$MIG_WHY" '"medium"')" 0 | jq -r '.concentration')" "peeled into their own PR" \ "…and an EMPTY attribution is rejected, not read as a subset of everything" -hasnt "$(render_surfaces "$(record R3 R3 "$mig" R1 R3 '["migrations/0042_drop.sql","docs/a.md"]' "$MIG_WHY" '"R1"')" 0 | jq -r '.concentration')" \ +hasnt "$(render_surfaces "$(record xhigh xhigh "$mig" medium xhigh '["migrations/0042_drop.sql","docs/a.md"]' "$MIG_WHY" '"medium"')" 0 | jq -r '.concentration')" \ "peeled into their own PR" \ "…nor is a PARTIAL subset — one attributed path outside the peeled set is enough to suppress" # `files` present but `residual_tier` absent is the pre-BE-7419 record: the grader answered the # subset question but never the "…and does the axis actually land lower?" one. Fail safe. -hasnt "$(render_surfaces "$(record R3 R3 "$mig" R1 R3 '["migrations/0042_drop.sql"]' "$MIG_WHY")" 0 | jq -r '.concentration')" \ +hasnt "$(render_surfaces "$(record xhigh xhigh "$mig" medium xhigh '["migrations/0042_drop.sql"]' "$MIG_WHY")" 0 | jq -r '.concentration')" \ "peeled into their own PR" \ "…and a record with files but NO residual_tier (graded before BE-7419) stays SILENT" # Both ties at once: the provenance half is unaffected by any peel, so it still decides. -hasnt "$(render_surfaces "$(record R3 R3 "$mig" R3 R3 '["migrations/0042_drop.sql"]' "$MIG_WHY" '"R1"')" 0 | jq -r '.concentration')" \ +hasnt "$(render_surfaces "$(record xhigh xhigh "$mig" xhigh xhigh '["migrations/0042_drop.sql"]' "$MIG_WHY" '"medium"')" 0 | jq -r '.concentration')" \ "peeled into their own PR" \ "a provenance tie suppresses the clause even when the reversibility tie IS removable" # The ungraded surfaces never reach the sentence at all. hasnt "$(render_surfaces "$u" 0 | jq -r '.concentration')" "peeled into their own PR" \ "an ungradable record proposes no split" -# A REMAINDER THAT ROUNDS TO 0%: one R0 line against 9999 R3 ones. There IS a below-floor file, so -# the set is non-empty, but the sentence has just printed "**0%** of this diff is R0/R1/R2" — and a +# A REMAINDER THAT ROUNDS TO 0%: one low line against 9999 xhigh ones. There IS a below-floor file, so +# the set is non-empty, but the sentence has just printed "**0%** of this diff is low/medium/high" — and a # clause under that would pitch a whole extra PR to relocate a single line. The gate is the # sentence's own printed share, so the two halves can never contradict each other. -tiny="$(render_surfaces "$(record R3 R3 "[$(file_entry docs/a.md R0 1 0), $(file_entry .github/workflows/d.yml R3 9999 0)]")" 0 | jq -r '.concentration')" -has "$tiny" "**0% of this diff is R0/R1/R2**" "a 1-line remainder still rounds the share to 0%…" +tiny="$(render_surfaces "$(record xhigh xhigh "[$(file_entry docs/a.md low 1 0), $(file_entry .github/workflows/d.yml xhigh 9999 0)]")" 0 | jq -r '.concentration')" +has "$tiny" "**0% of this diff is low/medium/high**" "a 1-line remainder still rounds the share to 0%…" hasnt "$tiny" "peeled into their own PR" "…and a 0% remainder is offered NO split" # One line the other way is enough: 1% is a share the sentence prints, so the clause speaks. -small="$(render_surfaces "$(record R3 R3 "[$(file_entry docs/a.md R0 100 0), $(file_entry .github/workflows/d.yml R3 9900 0)]")" 0 | jq -r '.concentration')" -has "$small" "peeled into their own PR, the remaining 1 file(s) would path-floor at **R0**" \ +small="$(render_surfaces "$(record xhigh xhigh "[$(file_entry docs/a.md low 100 0), $(file_entry .github/workflows/d.yml xhigh 9900 0)]")" 0 | jq -r '.concentration')" +has "$small" "peeled into their own PR, the remaining 1 file(s) would path-floor at **low**" \ "a remainder the share sentence does print (1%) keeps the clause" echo "— the body is BOUNDED under GitHub's 65536-char comment limit (measured, not estimated) —" # 400 files, each with a 300-char deeply-nested path — comfortably past the limit unbounded. deep="$(printf 'src/%.0s' $(seq 1 60))deeply/nested/path/component/that/keeps/going/and/going/file" manyfiles="$(jq -nc --arg d "$deep" '[range(0;400) | {path:($d + "-\(.).go"), previous_path:null, - additions:(. + 3), deletions:1, change_type:"MODIFIED", tier:"R3", classes:["cls"]}]')" -bigrec="$(record R3 R3 "$manyfiles")" + additions:(. + 3), deletions:1, change_type:"MODIFIED", tier:"xhigh", classes:["cls"]}]')" +bigrec="$(record xhigh xhigh "$manyfiles")" # Pin the FIXTURE before asserting on the body: a record that failed to build renders the short # "unknown" body, which would sail under the limit and pass the size check for the wrong reason. eq "the oversized fixture really carries 400 files" "400" \ @@ -385,7 +396,7 @@ has "$out" "::warning::" "…and says so as an annotation instead" echo "— RENDER_ONLY writes nothing and emits the surfaces object —" outj="$(RECORD="$r" RENDER_ONLY=1 PATH="$SANDBOX/bin:$PATH" bash "$SCRIPT" 2>/dev/null)" -eq "RENDER_ONLY emits a check title" "Risk: R1" "$(jq -r '.check_title' <<<"$outj")" +eq "RENDER_ONLY emits a check title" "Risk: medium" "$(jq -r '.check_title' <<<"$outj")" eq "…and a comment body" "true" "$(jq -r '(.comment_body | length) > 100' <<<"$outj")" echo "— A FAILED READ WRITES NOTHING. Both reads are load-bearing —" From 0aec41dffef25bcb052d04e2e0a0366ccb97776c Mon Sep 17 00:00:00 2001 From: huang47 <157390+huang47@users.noreply.github.com> Date: Mon, 31 Aug 2026 21:36:20 -0700 Subject: [PATCH 4/5] feat(pr-derisk): use canonical tier names --- scripts/pr-derisk/plan-derisk.sh | 8 ++--- scripts/pr-derisk/publish-derisk-comment.sh | 21 ++++++++--- scripts/pr-derisk/tests/test_plan_derisk.sh | 40 +++++++++++++-------- 3 files changed, 46 insertions(+), 23 deletions(-) diff --git a/scripts/pr-derisk/plan-derisk.sh b/scripts/pr-derisk/plan-derisk.sh index cec20ece..b33bdccd 100644 --- a/scripts/pr-derisk/plan-derisk.sh +++ b/scripts/pr-derisk/plan-derisk.sh @@ -13,7 +13,7 @@ # is inert. It NEVER states a tier. Every floor rendered in the comment is computed by # grade-pr-risk.sh --stdin over a synthetic scorecard record built from that step's files — the # same deterministic judge, the same map, the same rules that graded the PR. So a model that -# hallucinates "this split lands R0" cannot put that number in front of a reviewer: the number +# hallucinates "this split lands low" cannot put that number in front of a reviewer: the number # comes from the grader or it does not appear. # # That is also why this lives OUTSIDE the grader rather than inside it. pr-risk's grading path @@ -124,9 +124,9 @@ trap 'rm -rf "$SCRATCH"' EXIT TIER="$(jq -r '.risk.tier // "unknown"' "$RECORD" 2>/dev/null)" # THE PATH-AXIS FLOOR, CARRIED SEPARATELY FROM THE HEADLINE, and the distinction is load-bearing. # `grade = worst(path_floor, provenance, reversibility)`, but a split only ever moves the PATH axis. -# Comparing a step's computed path floor against the HEADLINE would read "below R3" for every step -# of a fork PR (provenance R3, path floor R0) or of a `/derisk` typed while checks are pending -# (reversibility R2) — a lane win the split cannot deliver, claimed on exactly the pull requests +# Comparing a step's computed path floor against the HEADLINE would read "below xhigh" for every step +# of a fork PR (provenance xhigh, path floor low) or of a `/derisk` typed while checks are pending +# (reversibility high) — a lane win the split cannot deliver, claimed on exactly the pull requests # the no-fake-lane-win rule exists for. The renderer compares against this instead. PATH_TIER="$(jq -r '.risk.axes.path_floor.tier // ""' "$RECORD" 2>/dev/null)" STATUS="$(jq -r '.risk.status // "unknown"' "$RECORD" 2>/dev/null)" diff --git a/scripts/pr-derisk/publish-derisk-comment.sh b/scripts/pr-derisk/publish-derisk-comment.sh index 6551b493..0a39bd88 100644 --- a/scripts/pr-derisk/publish-derisk-comment.sh +++ b/scripts/pr-derisk/publish-derisk-comment.sh @@ -79,14 +79,27 @@ cat <<'JQ' def md_path: (flat | if length > 160 then .[0:160] + "…" else . end) | if test("[`\\\\]") then md_escape else "`" + gsub("\\|"; "\\|") + "`" end; - def tier_rank: {"R0":0,"R1":1,"R2":2,"R3":3}[.] // 3; + def canonical_tier: + if type != "string" then . + else {"R0":"low","R1":"medium","R2":"high","R3":"xhigh"}[.] // . end; + def canonical_plan: + if type != "object" then . + else (if has("headline_tier") then .headline_tier |= canonical_tier else . end) + | (if has("path_floor_tier") then .path_floor_tier |= canonical_tier else . end) + | (if (.steps | type) == "array" + then .steps |= map(if type == "object" and has("floor") then .floor |= canonical_tier else . end) + else . end) + end; + def tier_rank: + canonical_tier | if type == "string" then {"low":0,"medium":1,"high":2,"xhigh":3}[.] // 3 else 3 end; - . as $p + # Historical plans remain renderable, but every surface uses canonical tier names. + (. | canonical_plan) as $p | ($p.headline_tier // null) as $tier # THE COMPARISON AXIS IS THE PATH FLOOR, NOT THE HEADLINE, and mixing them up is how this # comment claims a lane win it cannot deliver. `grade = worst(path_floor, provenance, - # reversibility)` but a split only moves the PATH axis: on a fork PR (provenance R3, path floor - # R0) or a `/derisk` typed while checks are still pending (reversibility R2), EVERY step reads + # reversibility)` but a split only moves the PATH axis: on a fork PR (provenance xhigh, path floor + # low) or a `/derisk` typed while checks are still pending (reversibility high), EVERY step reads # "below" the headline while the axis that actually set the grade is untouched. Older plans # carry no `path_floor_tier`, so the headline is the fallback there. | (($p.path_floor_tier // $p.headline_tier) // null) as $ptier diff --git a/scripts/pr-derisk/tests/test_plan_derisk.sh b/scripts/pr-derisk/tests/test_plan_derisk.sh index c3161fd1..b8cba90e 100644 --- a/scripts/pr-derisk/tests/test_plan_derisk.sh +++ b/scripts/pr-derisk/tests/test_plan_derisk.sh @@ -80,7 +80,7 @@ graded() { MIXED='[{"path":"db/migrations/0001_x.sql","change_type":"ADDED","additions":20,"deletions":0}, {"path":"docs/a.md","change_type":"MODIFIED","additions":5,"deletions":3}, {"path":"src/a.go","change_type":"MODIFIED","additions":10,"deletions":5}]' -# Every file under one R3 rule: the single-class monolith, where no split can buy a cheaper lane. +# Every file under one xhigh rule: the single-class monolith, where no split can buy a cheaper lane. MONO='[{"path":"db/migrations/0001_x.sql","change_type":"ADDED","additions":20,"deletions":0}, {"path":"db/migrations/0002_y.sql","change_type":"ADDED","additions":10,"deletions":0}]' @@ -96,17 +96,27 @@ plan() { # <record> <stub-file> [extra env assignments...] render() { PLAN="$1" DRY_RUN=1 bash "$PUBLISHER" 2>/dev/null; } echo "— phase 1: every floor is the GRADER's, never the model's —" -# The stub states a tier on every step and calls the migration step R0. If any of that reaches the +# The stub states a tier on every step and calls the migration step low. If any of that reaches the # reader, the whole design is decoration. -GOOD='{"steps":[{"name":"Migration","description":"d","files":["db/migrations/0001_x.sql"],"depends_on":[],"inertness":"i","review_ask":"the chain","tier":"R0","floor":"R0"},{"name":"Rest","description":"d","files":["docs/a.md","src/a.go"],"depends_on":[0],"inertness":"i","review_ask":"","tier":"R3"}],"summary":"step 1 carries it"}' +GOOD='{"steps":[{"name":"Migration","description":"d","files":["db/migrations/0001_x.sql"],"depends_on":[],"inertness":"i","review_ask":"the chain","tier":"low","floor":"low"},{"name":"Rest","description":"d","files":["docs/a.md","src/a.go"],"depends_on":[0],"inertness":"i","review_ask":"","tier":"xhigh"}],"summary":"step 1 carries it"}' out="$(plan "$REC_MIXED" "$(stub "$GOOD")")"; printf '%s' "$out" > "$SANDBOX/p1-plan.json" eq "a valid partition plans" planned "$(jq -r .status <<<"$out")" -eq "the migration step floors R3 (the grader), not R0 (the model)" R3 "$(jq -r '.steps[0].floor' <<<"$out")" -eq "the docs+src step floors R0 (the grader), not R3 (the model)" R0 "$(jq -r '.steps[1].floor' <<<"$out")" +eq "the migration step floors xhigh (the grader), not low (the model)" xhigh "$(jq -r '.steps[0].floor' <<<"$out")" +eq "the docs+src step floors low (the grader), not xhigh (the model)" low "$(jq -r '.steps[1].floor' <<<"$out")" eq "the model's own tier key is dropped from the step" null "$(jq -r '.steps[0].tier // "null"' <<<"$out")" eq "line counts come from the graded record" 20 "$(jq -r '.steps[0].lines' <<<"$out")" eq "depends_on survives" 0 "$(jq -r '.steps[1].depends_on[0]' <<<"$out")" +echo "— phase 1b: historical R0..R3 plans render with canonical names —" +jq ' + def legacy: + if . == "low" then "R0" elif . == "medium" then "R1" + elif . == "high" then "R2" elif . == "xhigh" then "R3" else . end; + walk(if type == "string" then legacy else . end)' "$SANDBOX/p1-plan.json" > "$SANDBOX/legacy-plan.json" +legacy_body="$(render "$SANDBOX/legacy-plan.json")" +has "legacy plan tiers render canonically" "$legacy_body" "path-floor below xhigh" +no "legacy tier names are not re-published" "$legacy_body" "**R3**" + echo "— phase 2: the partition must cover the changed set EXACTLY —" DROPS='{"steps":[{"name":"A","description":"d","files":["db/migrations/0001_x.sql"],"depends_on":[],"inertness":"i","review_ask":""},{"name":"B","description":"d","files":["docs/a.md"],"depends_on":[],"inertness":"i","review_ask":""}],"summary":"s"}' out="$(plan "$REC_MIXED" "$(stub "$DROPS" "$GOOD")")" @@ -140,35 +150,35 @@ out="$(plan "$SANDBOX/ungraded.json" "$(stub "$GOOD")")" eq "an ungraded PR is not planned against" fallback "$(jq -r .status <<<"$out")" echo "— phase 4: the single-class monolith gets no fake lane win —" -MONOPLAN='{"steps":[{"name":"First migration","description":"d","files":["db/migrations/0001_x.sql"],"depends_on":[],"inertness":"i","review_ask":"the chain"},{"name":"Second migration","description":"d","files":["db/migrations/0002_y.sql"],"depends_on":[0],"inertness":"i","review_ask":""}],"summary":"both are R3"}' +MONOPLAN='{"steps":[{"name":"First migration","description":"d","files":["db/migrations/0001_x.sql"],"depends_on":[],"inertness":"i","review_ask":"the chain"},{"name":"Second migration","description":"d","files":["db/migrations/0002_y.sql"],"depends_on":[0],"inertness":"i","review_ask":""}],"summary":"both are xhigh"}' out="$(plan "$REC_MONO" "$(stub "$MONOPLAN")")"; printf '%s' "$out" > "$SANDBOX/mono-plan.json" -eq "both steps still floor R3" "R3 R3" "$(jq -r '[.steps[].floor] | join(" ")' <<<"$out")" +eq "both steps still floor xhigh" "xhigh xhigh" "$(jq -r '[.steps[].floor] | join(" ")' <<<"$out")" body="$(render "$SANDBOX/mono-plan.json")" has "the verdict says same lane" "$body" "same lane" no "and claims no reduction" "$body" "path-floor below" echo "— phase 4b: a headline set by a NON-PATH axis is never claimed as a lane win —" # `grade = worst(path_floor, provenance, reversibility)` and a split only moves the PATH axis. This -# fork PR grades R3 on PROVENANCE with a path floor of R0, so every step trivially sits below the +# fork PR grades xhigh on PROVENANCE with a path floor of low, so every step trivially sits below the # HEADLINE while the axis that actually set the grade is untouched. Comparing against the headline -# printed "2 step(s) path-floor below R3 (100% of the changed lines)" — a reduction no partition +# printed "2 step(s) path-floor below xhigh (100% of the changed lines)" — a reduction no partition # here can deliver, on exactly the pull requests the no-fake-lane-win rule exists for. SOFT='[{"path":"docs/a.md","change_type":"MODIFIED","additions":5,"deletions":3}, {"path":"src/a.go","change_type":"MODIFIED","additions":10,"deletions":5}]' REC_FORK="$(graded fork "$SOFT" '{"is_fork":true,"author_association":"NONE"}')" -eq "the fixture grades R3 overall" R3 "$(jq -r '.risk.tier' "$REC_FORK")" -eq "but its PATH floor is R0" R0 "$(jq -r '.risk.axes.path_floor.tier' "$REC_FORK")" +eq "the fixture grades xhigh overall" xhigh "$(jq -r '.risk.tier' "$REC_FORK")" +eq "but its PATH floor is low" low "$(jq -r '.risk.axes.path_floor.tier' "$REC_FORK")" FORKPLAN='{"steps":[{"name":"Docs","description":"d","files":["docs/a.md"],"depends_on":[],"inertness":"i","review_ask":""},{"name":"Code","description":"d","files":["src/a.go"],"depends_on":[0],"inertness":"i","review_ask":"the chain"}],"summary":"s"}' out="$(plan "$REC_FORK" "$(stub "$FORKPLAN")")"; printf '%s' "$out" > "$SANDBOX/fork-plan.json" -eq "the plan carries the path floor separately from the headline" "R3 R0" \ +eq "the plan carries the path floor separately from the headline" "xhigh low" \ "$(jq -r '[.headline_tier, .path_floor_tier] | join(" ")' <<<"$out")" body="$(render "$SANDBOX/fork-plan.json")" -no "no step is claimed to land below the non-path headline" "$body" "path-floor below R3" -has "the verdict speaks in the PATH floor instead" "$body" 'path-floors at **R0**' +no "no step is claimed to land below the non-path headline" "$body" "path-floor below xhigh" +has "the verdict speaks in the PATH floor instead" "$body" 'path-floors at **low**' has "and names the axis a split cannot move" "$body" "non-path axis" # The same comparison must still fire normally when the PATH axis IS the one holding the grade up. body="$(render "$SANDBOX/p1-plan.json")" -has "a genuine path-axis reduction is still reported" "$body" "path-floor below R3" +has "a genuine path-axis reduction is still reported" "$body" "path-floor below xhigh" no "and carries no non-path caveat" "$body" "non-path axis" echo "— phase 4c: the ordering a plan exists to state cannot be impossible —" From 9695f7fe1b4145ff0db754b3ced553b69df6fa15 Mon Sep 17 00:00:00 2001 From: huang47 <157390+huang47@users.noreply.github.com> Date: Mon, 31 Aug 2026 21:36:20 -0700 Subject: [PATCH 5/5] docs(pr-risk): document named risk tiers --- README.md | 4 +- docs/callers/pr-risk.md | 19 +++++---- scripts/pr-derisk/README.md | 10 ++--- scripts/pr-risk/README.md | 84 ++++++++++++++++++++++--------------- 4 files changed, 69 insertions(+), 48 deletions(-) diff --git a/README.md b/README.md index a53e8f56..ef07b5de 100644 --- a/README.md +++ b/README.md @@ -21,8 +21,8 @@ complete, copy-pasteable caller. | [`assign-reviewers.yml`](.github/workflows/assign-reviewers.yml) | Auto-requests expertise-aware, load-balanced PR reviewers with new-folk randomization. Matches changed paths against a caller-repo `.github/reviewers.yml` (path-glob → reviewers, plus a `default_pool`), drops the author + `vars.REVIEWER_EXCLUDE`, ranks candidates by open review load (steering off anyone at/over `vars.REVIEWER_LOAD_CAP`), and may swap a slot for a `vars.REVIEWER_GROWTH_POOL` member. Requests go through the CLOUD_CODE_BOT app token so they work on fork PRs. Requires `vars.APP_ID` + `CLOUD_CODE_BOT_PRIVATE_KEY`. | [assign-reviewers.md](docs/callers/assign-reviewers.md) | | [`assign-prs-to-author.yml`](.github/workflows/assign-prs-to-author.yml) | Housekeeping — assigns every open PR with no assignees to its author (bot-authored PRs skipped by default). Run on a schedule from a thin caller; useful when a team tracks PR ownership via assignees. The calling job needs `pull-requests: write` and `issues: write`. | [assign-prs-to-author.md](docs/callers/assign-prs-to-author.md) | | [`pr-size.yml`](.github/workflows/pr-size.yml) | PR-size cap — fails (or, in `mode: warn`, only reports) when a PR's net diff exceeds `max_lines` non-generated changed lines, keeping diffs reviewable. Excludes dependency lockfiles, `linguist-generated` files (read from the base ref, so a PR can't exempt itself), Go generated-code markers, and per-repo `extra_lockfiles` / `extra_generated_globs`. Opt in to `exclude_tests` to cap production code rather than test coverage (excluded test lines are always reported, never silently dropped). A `bypass_label` (default `oversized-ok`) waves through a legitimately large change; a sticky bot comment explains overages when `bot_app_id` + `BOT_APP_PRIVATE_KEY` are supplied (degrades to status + step summary without them). Counting logic + tests live in [`scripts/check-pr-size/`](scripts/check-pr-size). | [pr-size.md](docs/callers/pr-size.md) | -| [`pr-risk.yml`](.github/workflows/pr-risk.yml) | **Advisory PR risk grading (shadow check)** — **automatic grading off by default** (`enabled: false`; a manual `workflow_dispatch` grades regardless, so a repo can trial it before switching on); switch it on with `enabled: true` or by setting the caller repo's `RISK_CONFIG` variable to `{"enabled": true}`, which outranks the input in both directions so `{"enabled": false}` is a no-PR kill switch. Grades every PR into a tier `R0` (safest) .. `R3` (riskiest) and syncs one label (`risk:R0`..`risk:R3`, or `risk:ungraded` when an input was unreadable). The label is the entire product: nothing is gated, routed, commented, or merged. Deterministic (`gh` + `jq`, no LLM): `grade = worst(path_floor, provenance, reversibility)` — path-glob map, what-process-produced-the-diff (registered runbooks with identity + diff-shape assertions; forks are R3 with no exceptions), and revertability (persistent-state mutation, deletions under sensitive classes, did green checks cover the lines). Grader + generic defaults live in [`scripts/pr-risk/`](scripts/pr-risk); a consumer sharpens them with `.github/risk.json` / `.github/risk-runbooks.json`, read from the PR's **base ref** so a PR can't edit the rules that judge it. The job excludes its own run from the check rollup and waits (`wait_for_checks_minutes`) for the rest to settle before labeling. Labels ride the plain `GITHUB_TOKEN` (cannot fire `labeled` triggers — no cascade risk); disagreement is recorded with a human-owned `risk-dispute` label. **Two further publish surfaces are available and are OFF by default**, so an enrolled caller behaves byte-identically until it opts in: `sticky_comment: true` posts ONE comment (created once, updated in place — N pushes leave one comment) carrying the per-file path-axis breakdown, the risk CONCENTRATION sentence ("94% of this diff is R0/R1; the 6% that puts the path floor at R3 is these two files, 40 lines") and a "this grade is wrong" checkbox whose state round-trips into a `risk-grade-disputed` label (distinct from the human-owned `risk-dispute`, which the grader still never touches); `check_run: true` publishes the tier and reason as a Check Run on the head commit — the immutable, timestamped, commit-attached record a mutable label cannot be — from a SEPARATE job, the only one holding `checks: write` — a grant every caller makes whether or not it switches the surface on, because GitHub validates every nested job's declared `permissions:` at startup and a job `if:` is a runtime condition (the input decides whether a check is PUBLISHED, not whether the grant is CHECKED). Both surfaces are advisory in the same sense as the label: the Check Run's conclusion is hardcoded `neutral`, and every publish failure is an annotation, never a red check. Label text is remappable via `label_map`. `workflows_ref` is **required**, and both its **shape and its ancestry are enforced** — every job that checks it out fails the run *before* the tool checkout unless the value is a full 40-hex lowercase commit SHA **and** that commit is an ancestor of `main` of this repo (fetched from a literal upstream URL; no `github` context can name this repo from inside a reusable workflow, so a variable there would be an alias a fork could point at itself). Shape rejects everything mutable — a branch, a tag, a `refs/pull/N/head`. Ancestry rejects everything unmerged: a fork of this public repo shares its object store, and GitHub serves a fork PR's head objects from this repo's own URL unauthenticated, so a fork-authored SHA is just as well-shaped and would otherwise be checked out into a job holding the caller's write token. There is deliberately **no opt-out input** — an opt-out would be set by the very pin-bump PR the check distrusts. Two consequences by design: **pinning a not-yet-merged SHA is rejected** (merge first, then bump), and the axis **fails closed** if this repo ever goes private or `main` is force-rewritten past a consumer's pin (re-pin to a commit on `main` to recover). **What is still not machine-checked is that the pin is the *current* one**: a stale pin left behind when `uses:` moved is a merged ancestor too. The test that would close that is "equal to the commit `uses:` resolved to". That value IS readable — not as `github.workflow_sha` (the *caller's* top-level file) and not as the `job_workflow_sha` OIDC claim (not a `github` context property; reading it would need `id-token: write` from every caller), but as `job.workflow_sha`, the `job`-context accessor added in runner v2.334.0 that `groom.yml` reads (BE-8077). It is deliberately not wired in here: asserting equality would fail red on every caller whose `uses:` and `workflows_ref` have already drifted apart, which is a caller-contract change tracked separately. The other residual is irreducible: a fork commit that edits `pr-risk.yml` itself is rejected by ancestry, but a caller pinned at one runs the fork's own copy of the guard, and no in-file check can bound a revision of that file an attacker chose. **Reviewing the caller is what bounds both, and it is the only thing that can: require `uses:` at a full commit SHA of this repo and `with: workflows_ref:` set to that same SHA written out literally, character-for-character — never an expression, never a tag.** The guard also runs *before* enablement is resolved (the resolver is itself loaded from `workflows_ref`), so a floating pin fails red even with the `RISK_CONFIG` kill switch set — the switch stops the grading, not a broken enrollment. Call the workflow directly: a nested `workflow_call` chain through an org wrapper is unsupported. Enroll it as its own workflow rather than a job inside an existing CI workflow (the rollup exclusion is per-run). The calling job needs `contents: read` + `issues: write` + `pull-requests: write` + `checks: write` + `actions: read` + `statuses: read`; GitHub rejects a shorter grant at startup (a reusable workflow can only narrow the caller's token, never elevate it), so a caller enrolled from an older copy of this row fails before any step runs. The two label writes are the ONE label: repo-side label creation on first use maps to `issues`, and labeling a PR maps to `pull-requests` (the labels endpoint is dual-mapped by what the "issue" is, so `issues: write` alone 403s on a PR). `checks: write` is the `publish-check` job's declaration, required unconditionally per above — grading itself uses only `checks: read`. `actions: read` is for the rollup's `CheckRun -> checkSuite -> workflowRun` self-exclusion hop. No secrets. | [pr-risk.md](docs/callers/pr-risk.md) | -| [`pr-derisk.yml`](.github/workflows/pr-derisk.yml) | **On-demand de-risk split plan (`/derisk`, beta)** — **off by default** (`enabled: false`; `vars.DERISK_CONFIG` on the caller repo outranks it in both directions, so `{"enabled": false}` is a no-PR kill switch). Someone with write access comments `/derisk` on a pull request; the workflow re-grades that PR with the pr-risk grader, makes **one** model call for a semantic partition of the diff into a chain of 2–5 **sequential** PRs, and posts **one sticky advisory comment**: a verdict line and a chain table above the fold, the plan, the chain-landing rules and the floor math inside collapsed `<details>`. **Every floor shown is computed by `grade-pr-risk.sh --stdin`** over a synthetic record built from each step's files — the model proposes *which files go together* and nothing else, and any tier it writes is discarded before rendering. The partition must cover the changed-file set **exactly**; a missing or duplicated path is rejected, re-prompted **once**, then falls back. **Nothing is gated, routed, merged, labelled or filed** — filing tickets from a plan is a later rung behind its own command. Honest by construction: when no step lands below the PR's **path floor** the verdict reads "N smaller single-concern R3s, same lane", never a fake lane win, because the verdict is arithmetic over grader output rather than the model's prose — and the comparison is against the path floor rather than the headline tier precisely because a split only moves the path axis, so a fork PR (provenance R3, path floor R0) is told the truth instead of a reduction no partition can deliver. An over-budget diff, an unvalidatable partition and an API failure each post an explaining comment — a `/derisk` that quietly does nothing is never an outcome. Safe as a comment command because an `issue_comment` workflow runs from the caller's **default branch** (a PR cannot edit the workflow serving it), the commenter is gated on `author_association` (`allowed_associations`, default `OWNER,MEMBER,COLLABORATOR` — narrow it, never widen), and **no PR code is ever checked out**: the diff is read over the API and handed to a model as text. `workflows_ref` is **required** and its shape + ancestry are enforced by the guard byte-identical to `pr-risk.yml`'s (see that row for the full rationale and its two residuals); a test fails the build if the copies drift. The calling job needs `contents: read` + `pull-requests: write` + `checks: read` + `actions: read` + `statuses: read`, and the `anthropic_api_key` secret. | [pr-derisk.md](docs/callers/pr-derisk.md) | +| [`pr-risk.yml`](.github/workflows/pr-risk.yml) | **Advisory PR risk grading (shadow check)** — **automatic grading off by default** (`enabled: false`; a manual `workflow_dispatch` grades regardless, so a repo can trial it before switching on); switch it on with `enabled: true` or by setting the caller repo's `RISK_CONFIG` variable to `{"enabled": true}`, which outranks the input in both directions so `{"enabled": false}` is a no-PR kill switch. Grades every PR into a tier `low` (safest) .. `xhigh` (riskiest) and syncs one label (`risk:low`..`risk:xhigh`, or `risk:ungraded` when an input was unreadable). Deprecated `R0`–`R3` map values and `label_map` keys remain accepted as aliases. The label is the entire product: nothing is gated, routed, commented, or merged. Deterministic (`gh` + `jq`, no LLM): `grade = worst(path_floor, provenance, reversibility)` — path-glob map, what-process-produced-the-diff (registered runbooks with identity + diff-shape assertions; forks are xhigh with no exceptions), and revertability (persistent-state mutation, deletions under sensitive classes, did green checks cover the lines). Grader + generic defaults live in [`scripts/pr-risk/`](scripts/pr-risk); a consumer sharpens them with `.github/risk.json` / `.github/risk-runbooks.json`, read from the PR's **base ref** so a PR can't edit the rules that judge it. The job excludes its own run from the check rollup and waits (`wait_for_checks_minutes`) for the rest to settle before labeling. Labels ride the plain `GITHUB_TOKEN` (cannot fire `labeled` triggers — no cascade risk); disagreement is recorded with a human-owned `risk-dispute` label. **Two further publish surfaces are available and are OFF by default**, so an enrolled caller behaves byte-identically until it opts in: `sticky_comment: true` posts ONE comment (created once, updated in place — N pushes leave one comment) carrying the per-file path-axis breakdown, the risk CONCENTRATION sentence ("94% of this diff is low/medium; the 6% that puts the path floor at xhigh is these two files, 40 lines") and a "this grade is wrong" checkbox whose state round-trips into a `risk-grade-disputed` label (distinct from the human-owned `risk-dispute`, which the grader still never touches); `check_run: true` publishes the tier and reason as a Check Run on the head commit — the immutable, timestamped, commit-attached record a mutable label cannot be — from a SEPARATE job, the only one holding `checks: write` — a grant every caller makes whether or not it switches the surface on, because GitHub validates every nested job's declared `permissions:` at startup and a job `if:` is a runtime condition (the input decides whether a check is PUBLISHED, not whether the grant is CHECKED). Both surfaces are advisory in the same sense as the label: the Check Run's conclusion is hardcoded `neutral`, and every publish failure is an annotation, never a red check. Label text is remappable via `label_map`. `workflows_ref` is **required**, and both its **shape and its ancestry are enforced** — every job that checks it out fails the run *before* the tool checkout unless the value is a full 40-hex lowercase commit SHA **and** that commit is an ancestor of `main` of this repo (fetched from a literal upstream URL; no `github` context can name this repo from inside a reusable workflow, so a variable there would be an alias a fork could point at itself). Shape rejects everything mutable — a branch, a tag, a `refs/pull/N/head`. Ancestry rejects everything unmerged: a fork of this public repo shares its object store, and GitHub serves a fork PR's head objects from this repo's own URL unauthenticated, so a fork-authored SHA is just as well-shaped and would otherwise be checked out into a job holding the caller's write token. There is deliberately **no opt-out input** — an opt-out would be set by the very pin-bump PR the check distrusts. Two consequences by design: **pinning a not-yet-merged SHA is rejected** (merge first, then bump), and the axis **fails closed** if this repo ever goes private or `main` is force-rewritten past a consumer's pin (re-pin to a commit on `main` to recover). **What is still not machine-checked is that the pin is the *current* one**: a stale pin left behind when `uses:` moved is a merged ancestor too. The test that would close that is "equal to the commit `uses:` resolved to". That value IS readable — not as `github.workflow_sha` (the *caller's* top-level file) and not as the `job_workflow_sha` OIDC claim (not a `github` context property; reading it would need `id-token: write` from every caller), but as `job.workflow_sha`, the `job`-context accessor added in runner v2.334.0 that `groom.yml` reads (BE-8077). It is deliberately not wired in here: asserting equality would fail red on every caller whose `uses:` and `workflows_ref` have already drifted apart, which is a caller-contract change tracked separately. The other residual is irreducible: a fork commit that edits `pr-risk.yml` itself is rejected by ancestry, but a caller pinned at one runs the fork's own copy of the guard, and no in-file check can bound a revision of that file an attacker chose. **Reviewing the caller is what bounds both, and it is the only thing that can: require `uses:` at a full commit SHA of this repo and `with: workflows_ref:` set to that same SHA written out literally, character-for-character — never an expression, never a tag.** The guard also runs *before* enablement is resolved (the resolver is itself loaded from `workflows_ref`), so a floating pin fails red even with the `RISK_CONFIG` kill switch set — the switch stops the grading, not a broken enrollment. Call the workflow directly: a nested `workflow_call` chain through an org wrapper is unsupported. Enroll it as its own workflow rather than a job inside an existing CI workflow (the rollup exclusion is per-run). The calling job needs `contents: read` + `issues: write` + `pull-requests: write` + `checks: write` + `actions: read` + `statuses: read`; GitHub rejects a shorter grant at startup (a reusable workflow can only narrow the caller's token, never elevate it), so a caller enrolled from an older copy of this row fails before any step runs. The two label writes are the ONE label: repo-side label creation on first use maps to `issues`, and labeling a PR maps to `pull-requests` (the labels endpoint is dual-mapped by what the "issue" is, so `issues: write` alone 403s on a PR). `checks: write` is the `publish-check` job's declaration, required unconditionally per above — grading itself uses only `checks: read`. `actions: read` is for the rollup's `CheckRun -> checkSuite -> workflowRun` self-exclusion hop. No secrets. | [pr-risk.md](docs/callers/pr-risk.md) | +| [`pr-derisk.yml`](.github/workflows/pr-derisk.yml) | **On-demand de-risk split plan (`/derisk`, beta)** — **off by default** (`enabled: false`; `vars.DERISK_CONFIG` on the caller repo outranks it in both directions, so `{"enabled": false}` is a no-PR kill switch). Someone with write access comments `/derisk` on a pull request; the workflow re-grades that PR with the pr-risk grader, makes **one** model call for a semantic partition of the diff into a chain of 2–5 **sequential** PRs, and posts **one sticky advisory comment**: a verdict line and a chain table above the fold, the plan, the chain-landing rules and the floor math inside collapsed `<details>`. **Every floor shown is computed by `grade-pr-risk.sh --stdin`** over a synthetic record built from each step's files — the model proposes *which files go together* and nothing else, and any tier it writes is discarded before rendering. The partition must cover the changed-file set **exactly**; a missing or duplicated path is rejected, re-prompted **once**, then falls back. **Nothing is gated, routed, merged, labelled or filed** — filing tickets from a plan is a later rung behind its own command. Honest by construction: when no step lands below the PR's **path floor** the verdict reads "N smaller single-concern xhigh tiers, same lane", never a fake lane win, because the verdict is arithmetic over grader output rather than the model's prose — and the comparison is against the path floor rather than the headline tier precisely because a split only moves the path axis, so a fork PR (provenance xhigh, path floor low) is told the truth instead of a reduction no partition can deliver. An over-budget diff, an unvalidatable partition and an API failure each post an explaining comment — a `/derisk` that quietly does nothing is never an outcome. Safe as a comment command because an `issue_comment` workflow runs from the caller's **default branch** (a PR cannot edit the workflow serving it), the commenter is gated on `author_association` (`allowed_associations`, default `OWNER,MEMBER,COLLABORATOR` — narrow it, never widen), and **no PR code is ever checked out**: the diff is read over the API and handed to a model as text. `workflows_ref` is **required** and its shape + ancestry are enforced by the guard byte-identical to `pr-risk.yml`'s (see that row for the full rationale and its two residuals); a test fails the build if the copies drift. The calling job needs `contents: read` + `pull-requests: write` + `checks: read` + `actions: read` + `statuses: read`, and the `anthropic_api_key` secret. | [pr-derisk.md](docs/callers/pr-derisk.md) | | [`pr-area-label.yml`](.github/workflows/pr-area-label.yml) | **Agentic PR area labeling** — classifies each PR into exactly one `area:*` label, and (on push to the consumer's default branch) syncs the repo's `area:*` labels to the taxonomy. The taxonomy is the consumer's own `.github/area-labels.yml` (`repo_context` + `labels[]` with `name`/`color`/`description`/optional `guidance`); the shared workflow carries nothing repo-specific. An LLM makes the domain-vs-path judgement a static `paths:` map can't, but with **no tools and no token**: PR title/body/paths/labels go to the Anthropic Messages API as data inside `<pr_data>` tags (diff excluded), the reply is enum-constrained by a JSON schema to the taxonomy's own names, and a deterministic step applies it with targeted `area:*` add/remove ops (never a full-set PUT, so concurrent non-area edits survive). The taxonomy is read from the PR's **base ref** — a PR can't rewrite the rules that classify it — and validated (unique `area:[a-z0-9-]+` names, every label resolves to a non-blank routing guide) before it drives a write; everything fails soft rather than failing the check. Classifier logic lives in [`scripts/area-label/`](scripts/area-label), loaded from the pinned `workflows_ref` (validated to a full 40-hex SHA before checkout). Fork/Dependabot PRs are skipped by construction (no writable token, no secret). Labels ride the plain `GITHUB_TOKEN` (no `labeled`-trigger cascade). The calling job needs `contents: read` + `issues: write` + `pull-requests: write`; the `ANTHROPIC_API_KEY` secret is provided org-wide (unset ⇒ `label-pr` fails soft, sync still works). | [pr-area-label.md](docs/callers/pr-area-label.md) | | [`stale.yml`](.github/workflows/stale.yml) | Stale-PR sweeper (`actions/stale`) plus a Slack digest of what it touched. PRs inactive for N days are labeled `stale`; still-inactive PRs are closed. The digest header names the source repo so batches from different repos posted to the same channel are unambiguous. Thresholds, messages, exempt labels, and the Slack channel are inputs; the caller owns the schedule + dry-run toggle. The calling job needs `pull-requests: write` and `issues: write`. Optional `SLACK_BOT_TOKEN`. | [stale.md](docs/callers/stale.md) | | [`groom.yml`](.github/workflows/groom.yml) | Scheduled/dispatch org-wide **code-cleanup sweep** (finds only — no commits, no PRs, never merges). A read-only FINDER agent scans a clean default-branch checkout (whole-repo, not a diff) for high-value refactors; an INDEPENDENT VERIFIER agent (fresh session) re-checks each as CONFIRM/DOWNGRADE/REJECT with a stable dedup signature; survivors are deduped against a durable GitHub-issue-state ledger and filed as `groom`-labeled GitHub issues (security-adjacent ones get `groom-security` — investigate, don't auto-implement). Mirrors the cursor-review topology: briefs + ledger live in [`.github/groom/`](.github/groom) as the single source of truth. The finder/verifier/builder agent jobs invoke the Claude CLI directly and mint no GitHub token, so they need nothing beyond `contents: read`; filing runs in a separate job as the bot you configure via `bot_app_id` (Comfy: cloud-code-bot). `dry_run` reports what it would file without opening issues. Runs on a **daily base cron** with a runtime cadence gate: set repo Actions variable `GROOM_INTERVAL_DAYS` (default 7 = weekly) to retune how often a real run happens — weekly → every-3-days → daily — with no workflow-file edit; a tick within the interval no-ops before the finder (`workflow_dispatch` bypasses the interval gate, but the volume gate — when the caller leaves it on — still applies). The calling job must grant `contents: read` + `issues: write` + `pull-requests: read` + `actions: read` — the first three are declared by the `file` / `build_select` jobs (needed even with `bot_app_id` set), and the interval gate needs `actions: read` (reads run history for the last real run); GitHub rejects a shorter grant at startup. Requires `ANTHROPIC_API_KEY` (+ `BOT_APP_PRIVATE_KEY` when `bot_app_id` is set). **Opt-in auto-builder** (`builder: true`, BE-4003): the top `max_prs` (default 5) CONFIRMED, non-security findings become **review-gated PRs** (full CI + cursor-review, **never auto-merged**) instead of issues; a credential-free `build` job emits only a patch artifact and a separate `build_pr` job opens the PR as the bot, preserving the security boundary. The ledger's PR-state (open/merged/closed) stops a built finding being re-proposed. **Path scoping** (`path`, BE-4757): scope a run to ONE directory (`path: services/api`) instead of the whole repo — empty (the default) is today's whole-repo behavior, byte-for-byte, so existing callers are unaffected. It CONSTRAINS rather than instructs (unlike `scope_desc`, which is prompt prose): the path is validated (absolute / `..`-component / escaping paths rejected) and contained against the checkout before any agent runs, the finder is handed the concrete in-scope file list, and findings whose evidence lies entirely outside the directory are dropped with the count logged. The checkout stays **full** on purpose — a refactor in `services/api` legitimately references `common/`. The cadence clock is **per scope** (the finder job is renamed `Audit — finder (scoped: <path>)`, and the interval gate counts only prior runs of the same scope), so a scoped run leaves the next scheduled whole-repo tick due while a permanently scoped caller still gets a real `interval_days` cadence of its own; and the dedup signature ignores `path`, so a scoped run and a whole-repo run suppress each other's duplicates. Expose it as a `workflow_dispatch` input in the caller for on-demand scoped runs, or pin one in `with:` for a permanently scoped monorepo caller. Requires `bot_app_id`. `max_prs` is typed **`string`**, not `number`, so a caller can forward its own `workflow_dispatch` input straight through (`max_prs: ${{ github.event.inputs.max_prs \|\| '1' }}`) and let an operator raise the ceiling for one manual run — no `fromJSON()` cast in the caller, and the parse/clamp (empty → default, non-numeric → 0 PRs + warning, never a failed run) happens once inside the reusable. A build that cannot become a PR (patch over `pr_size_limit`, patch touching CI-privileged paths) **bails** to a `groom` issue so the paid-for work isn't lost — that path lives in `build_pr`, so **`max_findings` does not cap it** and `max_findings: 0` alone does not silence it; set `bail_sink: none` (an operational knob, so `GROOM_CONFIG` can set it with no PR) to file nothing and get a run-log warning + summary line instead. | [groom.md](docs/callers/groom.md) | diff --git a/docs/callers/pr-risk.md b/docs/callers/pr-risk.md index 4c3c5fdf..6e909307 100644 --- a/docs/callers/pr-risk.md +++ b/docs/callers/pr-risk.md @@ -4,15 +4,20 @@ Read [the shared caller contract](README.md) first. ## What it does -Grades every PR into a tier `R0` (safest) .. `R3` (riskiest) and syncs **one** -label (`risk:R0`..`risk:R3`, or `risk:ungraded` when an input was unreadable). +Grades every PR into a tier `low` (safest) .. `xhigh` (riskiest) and syncs **one** +label (`risk:low`..`risk:xhigh`, or `risk:ungraded` when an input was unreadable). The label is the entire product: nothing is gated, routed, commented, or merged — a human looks at the label and agrees or disagrees (recorded with a `risk-dispute` label this workflow never touches). +`R0`, `R1`, `R2`, and `R3` remain accepted in existing risk maps and +`label_map` keys as deprecated aliases for `low`, `medium`, `high`, and +`xhigh`. New maps and integrations should use only the canonical names; output +records and publish surfaces always do. + Deterministic, no LLM: `grade = worst(path_floor, provenance, reversibility)` — a path-glob map, what process produced the diff (registered runbooks, forks -always `R3`), and revertability (persistent-state mutation, sensitive +always `xhigh`), and revertability (persistent-state mutation, sensitive deletions, whether green checks actually covered the changed lines). The grader and its generic defaults live in [`scripts/pr-risk/`](../../scripts/pr-risk) and load from **this repo** at the @@ -116,9 +121,9 @@ fail the caller's next run at startup. |---|---|---| | `workflows_ref` | — (**required**) | Pin to the SAME full commit SHA as `uses:`. No default on purpose: a floating default let a caller SHA-pin `uses:` and still load the grader from HEAD of main. Checked before the tool checkout on two axes: it must be a full 40-hex lowercase SHA, **and** that commit must be an ancestor of `main` of this repo. So a branch, a tag, a `refs/pull/N/head` and any **not-yet-merged** SHA all fail the run — **merge the change here first, then bump the pin.** There is no opt-out. | | `fleet_logins` | `mattmillerai` | Logins whose PRs grade provenance `agent-supervised` alongside `agent-coded`. Both are read for **human** authors only: an author GitHub types as a `Bot` is a runbook candidate regardless, so listing a bot here (or labelling its PR) buys it nothing — only a registry entry that asserts can promote it. | -| `bot_logins` | `github-actions,dependabot,renovate,coderabbitai,cursor,comfy-pr-bot,web-flow` | Extra logins treated as bots. Needed only for **machine USER accounts** — a real GitHub App is recognized from GitHub's own actor type, no list entry required. A bot with no runbook entry still grades as human — identity alone buys no trust. **This list is load-bearing, not a hint:** a listed login skips the first-time-contributor test, so it moves a non-fork `NONE`/`FIRST_TIME_CONTRIBUTOR` PR from `external` (R3) to `human` (R1). Nothing validates that a listed login is really a machine account, so add one only for an account you control, and remove it when it is retired. | -| `label_map` | `''` | Rename the five grader-owned labels as `tier=label` pairs. Tier keys are fixed; only the label text is yours. | -| `wait_for_checks_minutes` | `10` | How long to wait for the rest of the check rollup to settle before labeling (clamped to 25 — what a 30-minute job can spend waiting). `0` labels immediately, expect R2 floors from still-pending checks. | +| `bot_logins` | `github-actions,dependabot,renovate,coderabbitai,cursor,comfy-pr-bot,web-flow` | Extra logins treated as bots. Needed only for **machine USER accounts** — a real GitHub App is recognized from GitHub's own actor type, no list entry required. A bot with no runbook entry still grades as human — identity alone buys no trust. **This list is load-bearing, not a hint:** a listed login skips the first-time-contributor test, so it moves a non-fork `NONE`/`FIRST_TIME_CONTRIBUTOR` PR from `external` (xhigh) to `human` (medium). Nothing validates that a listed login is really a machine account, so add one only for an account you control, and remove it when it is retired. | +| `label_map` | `''` | Rename the five grader-owned labels as `tier=label` pairs. Canonical keys are `low`, `medium`, `high`, `xhigh`, and `unknown`; deprecated `R0`–`R3` keys remain accepted. | +| `wait_for_checks_minutes` | `10` | How long to wait for the rest of the check rollup to settle before labeling (clamped to 25 — what a 30-minute job can spend waiting). `0` labels immediately, expect high floors from still-pending checks. | | `repo_map_path` | `.github/risk.json` | Consumer risk-map override, read from the PR **base ref**. | | `repo_runbooks_path` | `.github/risk-runbooks.json` | Consumer runbook-registry override, read from the PR **base ref**. | @@ -179,7 +184,7 @@ usual one). **Enroll it as its own workflow, not a job inside an existing CI workflow.** The grading job excludes its own *run* from the check rollup it reads; a job sharing a run with the rest of CI excludes its siblings too and lands on the -honest R2 floor instead of grading off the full rollup. +honest high floor instead of grading off the full rollup. **Pair the caller with a per-PR `cancel-in-progress` concurrency group** (shown above). The reversibility axis waits for other checks to settle, so a stale run diff --git a/scripts/pr-derisk/README.md b/scripts/pr-derisk/README.md index c569c2aa..8fb17a9d 100644 --- a/scripts/pr-derisk/README.md +++ b/scripts/pr-derisk/README.md @@ -25,7 +25,7 @@ The model proposes a **partition**: which files go in which step, in what order, step is inert. **It never states a tier.** Every floor rendered in the comment is computed by `grade-pr-risk.sh --stdin` over a synthetic scorecard record built from that step's files — the same deterministic judge, the same map, the same rules that graded the PR. A model that -hallucinates "this split lands R0" cannot put that number in front of a reviewer: the plan object +hallucinates "this split lands low" cannot put that number in front of a reviewer: the plan object is rebuilt field by field from a fixed list, so a model-claimed `tier` key does not survive. That is also why this lives *outside* the grader rather than inside it. pr-risk's grading path @@ -47,14 +47,14 @@ either way. A split can land *worse* than its floor; it can never land better. A single-class monolith — every file already at the PR's **path floor** — has no lane win available. The verdict line above the fold is computed from the **floors**, not from the model's -prose, so in that case it reads "N smaller single-concern R3s, same lane" and there is no wording +prose, so in that case it reads "N smaller single-concern xhigh tiers, same lane" and there is no wording available to it that claims a reduction. A prompt can *ask* for that; only the renderer can guarantee it. **The comparison axis is the PATH FLOOR, not the headline tier**, and that distinction is the rule rather than a detail of it. `grade = worst(path_floor, provenance, reversibility)` but a split only -ever moves the path axis, so on a fork PR (provenance R3, path floor R0) or a `/derisk` typed while -checks are still pending (reversibility R2) *every* step sits below the headline while the axis +ever moves the path axis, so on a fork PR (provenance xhigh, path floor low) or a `/derisk` typed while +checks are still pending (reversibility high) *every* step sits below the headline while the axis that actually set the grade is untouched. Comparing against the headline there would print a reduction no partition can deliver, on exactly the pull requests this rule exists for. When a non-path axis holds the grade up the verdict says so in the same breath as the split. @@ -112,6 +112,6 @@ which is then rejected unless it covers the changed-file set exactly. ## What is deliberately NOT here -No auto-running on every R3 (on-demand only in beta), no ticket filing from a plan (its own rung, +No auto-running on every xhigh (on-demand only in beta), no ticket filing from a plan (its own rung, behind its own command), no routing, no check, no label, no auto-merge. The offer threshold and which repos see it at all are post-merge flips of a repo variable, not code. diff --git a/scripts/pr-risk/README.md b/scripts/pr-risk/README.md index 2a4cbd76..2fec2cb1 100644 --- a/scripts/pr-risk/README.md +++ b/scripts/pr-risk/README.md @@ -5,10 +5,10 @@ event is graded into a tier and gets ONE label: | tier | label (default) | meaning | eventual routing (later phases — nothing routes today) | |---|---|---|---| -| R0 | `risk:R0` | inert — docs, tests, provably-shaped runbook output | auto-merge candidate | -| R1 | `risk:R1` | contained — bounded, covered, revertable in one click | rubber-stamp | -| R2 | `risk:R2` | standard — ordinary product code | normal review | -| R3 | `risk:R3` | elevated — auth, billing, migrations, IaC, CI, deps, secrets | owner + e2e | +| low | `risk:low` | inert — docs, tests, provably-shaped runbook output | auto-merge candidate | +| medium | `risk:medium` | contained — bounded, covered, revertable in one click | rubber-stamp | +| high | `risk:high` | standard — ordinary product code | normal review | +| xhigh | `risk:xhigh` | elevated — auth, billing, migrations, IaC, CI, deps, secrets | owner + e2e | | — | `risk:ungraded` | an input could not be read; deliberately NOT a tier | human review | **The label is the entire product.** Nothing is gated, blocked, routed, commented @@ -34,7 +34,7 @@ record. 2. **Provenance** — what PROCESS produced the diff: `runbook` (a registered producer in [`runbook-registry.v0.json`](runbook-registry.v0.json) whose identity AND diff shape both assert), `agent-supervised`, `human`, or - `external` (any fork, or a first-time **human** contributor — R3, no + `external` (any fork, or a first-time **human** contributor — xhigh, no exceptions, even when a runbook shape matches). Identity is the server-attributed author login, never the forgeable commit author string. A **non-fork bot is never `external`**, even though every GitHub App authors @@ -43,13 +43,13 @@ record. "Is this a bot?" is answered from GitHub's own actor type (`Bot`) plus the `bot_logins` list, so an App needs no list entry and a machine *user* account does. The fork half is unconditional, so a bot opening a PR *from a fork* is - still `external` R3. -3. **Reversibility** — mutates persistent state or deletes data → R3; **removes** - a file under a sensitive class → R3 (a delete, or a rename out of that class); - no green check rollup → R2; green but no test file touched → R1; green with - tests touched → R0. "Green" means at least one check actually CONCLUDED + still `external` xhigh. +3. **Reversibility** — mutates persistent state or deletes data → xhigh; **removes** + a file under a sensitive class → xhigh (a delete, or a rename out of that class); + no green check rollup → high; green but no test file touched → medium; green with + tests touched → low. "Green" means at least one check actually CONCLUDED success: a rollup of nothing but skipped/neutral answers "did tests covering - these lines run?" with nothing, so it cannot drop the axis below R2. What + these lines run?" with nothing, so it cannot drop the axis below high. What counts as a test file is `reversibility.test_path_patterns` in the map (omit the key and the grader falls back to a built-in regex that only knows the Go/TS shapes). The axis also records `files` — which changed paths actually @@ -65,8 +65,8 @@ record. `path_floor.files[].path`. Beside it, `residual_tier` says **where the axis lands once those paths are peeled** — the half `files` cannot answer on its own, because the three rungs the remainder falls back to are map-configurable - and a consumer that sets `no_green_checks_tier: "R3"` puts the remainder - straight back on R3. It is peel-set-independent: the "no green rollup" rung + and a consumer that sets `no_green_checks_tier: "xhigh"` puts the remainder + straight back on xhigh. It is peel-set-independent: the "no green rollup" rung tests the head commit, which no peel can change, so a non-green rollup pins the residual at `no_green_checks_tier` exactly, and a green one bounds it by the worse of the two rungs below. It is `null` exactly when `files` is. @@ -82,12 +82,12 @@ Two CI-specific mechanics worth knowing: - **The grading run excludes itself from the check rollup it reads** (its own check is always in-flight at grade time), and the job re-polls until the rest of the rollup settles or `wait_for_checks_minutes` runs out — otherwise every - live grade would floor at R2 as an artifact of the measurement. Exclusion is + live grade would floor at high as an artifact of the measurement. Exclusion is keyed on `github.run_id` (`--self-run-id`), and a **FAILING check is never excluded**: self-exclusion may only ever hide our own pending run, never a red one. Enroll pr-risk as its **own workflow** rather than a job inside an existing CI workflow — a job sharing a run with the rest of CI excludes its - siblings too, and lands on the honest R2 floor instead of a full rollup. + siblings too, and lands on the honest high floor instead of a full rollup. - **A caller grants the UNION of every job's `permissions:`, including jobs it will never run.** GitHub validates each nested job's *declaration* against the caller's block at startup, before any job `if:` is evaluated, so `checks: @@ -129,7 +129,7 @@ which PR is read, plus three consequences worth knowing: elevate, so the label write would 403 and the check would go red. On a dispatch the token is writable, so it does not apply. Fork **risk** is untouched: `external` comes from the API's own fork flag, never from the actor, and still - grades R3. **Dependabot-triggered runs need no such + grades xhigh. **Dependabot-triggered runs need no such hatch — they are graded on the event path too**, so the caller pattern carries no `github.actor != 'dependabot[bot]'` clause and one must not be added back: Dependabot's `pull_request` runs start read-only, but the caller's @@ -158,7 +158,7 @@ which PR is read, plus three consequences worth knowing: 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 + target someone pushed to minutes ago lands the honest high 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 @@ -198,11 +198,10 @@ Operational caveats for a backfill: invisible: GitHub records it on the PR timeline as an `unlabeled` event by the grader token. Dispatch when the queue is quiet, and use `pr_number` when you want the per-PR group to serialize a re-grade against event runs. -- **Remapping `label_map` orphans the old names.** Ownership is defined by the - *current* map, so labels applied under a previous one are no longer owned: - they ride through every future PUT beside the new target and no re-grade will - clear them. Delete the retired label names repo-side once, as part of the - remap. +- **Remapping `label_map` orphans custom old names.** The four former defaults + (`risk:R0`..`risk:R3`) are explicitly retired by the labeler, so the first + canonical re-grade removes them. Any custom value from an older map is + unknowable and still needs one-time repo-side cleanup. - **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 @@ -227,7 +226,7 @@ them by committing: *human*, who would then inherit that runbook — so never name an App with one. Both are read from the PR's **base ref**, so a PR cannot edit the rules that -judge it (editing them — or the grader — at all is R3 by the map's own first +judge it (editing them — or the grader — at all is xhigh by the map's own first rule). A genuine 404 falls back to the shipped defaults; a present-but-invalid file fails the run loudly rather than silently grading generic, and so does any non-404 read failure (a 403 rate-limit or 5xx must not quietly demote the PR to @@ -242,14 +241,30 @@ Every graded record carries `map_version` + `registry_version`, so grades made under different maps stay comparable and a map revision can be replayed against accumulated records. -## Relabeling (R0–R3 vs R1–R4 and friends) +## Tier vocabulary and deprecated aliases -Tier SEMANTICS are fixed (R0 safest .. R3 riskiest, `unknown` separate) +`low`, `medium`, `high`, and `xhigh` are canonical in maps, graded records, +labels, comments, and Check Runs. The former names remain input-only aliases: + +| Deprecated alias | Canonical tier | +|---|---| +| `R0` | `low` | +| `R1` | `medium` | +| `R2` | `high` | +| `R3` | `xhigh` | + +Existing consumer maps and `label_map` inputs using those aliases still work +and emit a deprecation warning. They are normalized at load time, so newly +graded records and newly rendered surfaces always use the canonical names. + +## Custom label text + +Tier SEMANTICS are fixed (low safest .. xhigh riskiest, `unknown` separate) everywhere records are stored. The label TEXT is the caller's, via `label_map`: ```yaml with: - label_map: "R0=risk:R1,R1=risk:R2,R2=risk:R3,R3=risk:R4,unknown=risk:ungraded" + label_map: "low=risk:1,medium=risk:2,high=risk:3,xhigh=risk:4,unknown=risk:ungraded" ``` Labels are created on first use, color-coded green → red (gray for ungraded). @@ -273,7 +288,7 @@ Labels are created on first use, color-coded green → red (gray for ungraded). the dispute checkbox below it. It is created once and updated in place. The full concentration sentence also carries a **reducibility readout**: when the path axis is what decided the tier and some of the diff sits below the floor, it names what the below-floor remainder would path-floor - at on its own — "…peeled into their own PR, the remaining 3 file(s) would path-floor at **R2** + at on its own — "…peeled into their own PR, the remaining 3 file(s) would path-floor at **high** (final grade still depends on the provenance and reversibility axes at PR time)". That complement floor is the worst per-file floor over exactly the files the share sentence already counts as below-floor, computed from the grader's own per-file floors rather than estimated, and it is what @@ -296,13 +311,13 @@ Labels are created on first use, color-coded green → red (gray for ungraded). .residual_tier` (where the axis lands after that peel) must rank strictly below the headline. Only then does one split provably remove both reasons, and the clause speaks. Both halves are load-bearing rather than formalities, and a consumer map override defeats each - one separately. Against the subset test: remap `migrations` to R1 while leaving it in + one separately. Against the subset test: remap `migrations` to medium while leaving it in `irreversible_classes` and the irreversible-class file sits *below* the path floor, so peeling the top files leaves the reversibility reason exactly where it was. Against the residual test: - set `no_green_checks_tier: "R3"` and peeling every attributed path drops the remainder onto a - rung that is R3 all over again — the attributed reason went with the peel, but the tier did not + set `no_green_checks_tier: "xhigh"` and peeling every attributed path drops the remainder onto a + rung that is xhigh all over again — the attributed reason went with the peel, but the tier did not move, so the reduction the clause would promise cannot happen. - Either field absent or `null` — the R2/R1 rungs, where the reason is a property of the + Either field absent or `null` — the high/medium rungs, where the reason is a property of the head commit or of the whole change set, and every record graded before those fields existed — reads as *not* removable, failing safe back to suppression. When a removable reversibility tie does let the clause speak, the above-the-fold headline names the driver **`path and @@ -332,9 +347,10 @@ Labels are created on first use, color-coded green → red (gray for ungraded). - `apply-risk-label.sh` — the one write, and it is literally one request: a single atomic `PUT` of the PR's full label set — every label the script does not own, carried through verbatim from the snapshot read, plus the computed - one. Owns exactly the five mapped labels (matched case-insensitively, as GitHub - label identity is), so a PR it has written to carries exactly one of them even - when two grading runs race. An already-in-sync PR writes nothing at all. + one. Owns the five mapped labels plus the four deprecated default labels + (`risk:R0`..`risk:R3`), matched case-insensitively, so a re-grade removes the + old default automatically and concurrent grading runs still leave one current + grade. Custom retired label-map values still need one-time cleanup. - `risk-map.v0.json` / `runbook-registry.v0.json` — the generic defaults. - `tests/` — hermetic suites (synthetic records + a stubbed `gh`); run via [`test-pr-risk.yml`](../../.github/workflows/test-pr-risk.yml).