Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/workflows/pr-risk.yml
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ jobs:
env:
GH_TOKEN: ${{ github.token }}
DENY_PATHS: ${{ inputs.deny-paths }}
AUTHOR_ASSOCIATION: ${{ github.event.pull_request.author_association }}
run: python3 /tmp/pr_risk.py gather --repo "$REPO" --number "$NUMBER" --dir "$WORKDIR"

- name: Install Claude Code CLI
Expand Down
13 changes: 8 additions & 5 deletions scripts/pr_risk.py
Original file line number Diff line number Diff line change
Expand Up @@ -160,15 +160,18 @@ def deny_patterns(extra: str) -> list[str]:
return list(DEFAULT_DENY_PATHS) + [line for line in lines if line and not line.startswith("#")]


def ineligible_reason(pr: dict[str, Any], files: list[dict[str, Any]], patterns: list[str]) -> str | None:
def ineligible_reason(pr: dict[str, Any], files: list[dict[str, Any]], patterns: list[str],
association: str) -> str | None:
if pr.get("draft"):
return "draft PR"
head_repo = (pr["head"].get("repo") or {}).get("full_name")
if head_repo != pr["base"]["repo"]["full_name"]:
return "PR comes from a fork"
# Only org members' own changes are candidates. Bots are never members, so their PRs are
# skipped too: no person wrote them, and approving them unread is a separate decision.
if pr.get("author_association") not in ("OWNER", "MEMBER"):
# The association comes from the event payload: the API answers for the caller's token,
# and the workflow token cannot see private org membership, so it reports CONTRIBUTOR.
if association not in ("OWNER", "MEMBER"):
return "author is not an org member"
changed = pr["additions"] + pr["deletions"]
if changed > MAX_CHANGED_LINES:
Expand Down Expand Up @@ -280,12 +283,12 @@ def previous_data(body: str) -> dict[str, Any] | None:
return None


def gather(gh: GitHub, number: int, extra_deny: str, out: Path) -> dict[str, Any]:
def gather(gh: GitHub, number: int, extra_deny: str, association: str, out: Path) -> dict[str, Any]:
out.mkdir(parents=True, exist_ok=True)
pr = gh.pull(number)
files = gh.files(number)
existing = find_comment(gh.comments(number))
reason = ineligible_reason(pr, files, deny_patterns(extra_deny))
reason = ineligible_reason(pr, files, deny_patterns(extra_deny), association)
gate = {
"sha": pr["head"]["sha"],
"eligible": reason is None,
Expand Down Expand Up @@ -436,7 +439,7 @@ def main() -> int:
return 0
gh = GitHub(args.repo)
if args.cmd == "gather":
gather(gh, args.number, os.environ.get("DENY_PATHS", ""), args.dir)
gather(gh, args.number, os.environ.get("DENY_PATHS", ""), os.environ["AUTHOR_ASSOCIATION"], args.dir)
return 0
return comment(gh, args.number, args.dir, os.environ.get("RUN_URL", ""))

Expand Down
37 changes: 20 additions & 17 deletions scripts/test_pr_risk.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,10 @@
PATCH = "@@ -10,3 +10,4 @@ flags:\n - a\n - b\n+ - c\n - d"


def pr(additions=1, deletions=0, draft=False, fork=False, sha="abc1234def", association="MEMBER"):
def pr(additions=1, deletions=0, draft=False, fork=False, sha="abc1234def"):
return {
"number": 7, "draft": draft, "title": "Add an org to a flag", "body": "Adds one org.",
"user": {"login": "someone"}, "author_association": association,
"user": {"login": "someone"}, "author_association": "CONTRIBUTOR",
"additions": additions, "deletions": deletions,
"head": {"sha": sha, "repo": {"full_name": "external/infra" if fork else "kernel/infra"}},
"base": {"sha": "base000", "ref": "main", "repo": {"full_name": "kernel/infra"}},
Expand Down Expand Up @@ -97,10 +97,10 @@ def visible_words(body):


class PRRiskTests(unittest.TestCase):
def run_pr(self, gh, verdict=VERDICT, result=None, extra_deny=""):
def run_pr(self, gh, verdict=VERDICT, result=None, extra_deny="", association="MEMBER"):
with tempfile.TemporaryDirectory() as tmp:
work = Path(tmp)
gate = pr_risk.gather(gh, 7, extra_deny, work)
gate = pr_risk.gather(gh, 7, extra_deny, association, work)
if gate["eligible"]:
(work / "result.json").write_text(json.dumps(result or {"is_error": False, "structured_output": verdict}))
code = pr_risk.comment(gh, 7, work, "https://example.com/run")
Expand Down Expand Up @@ -130,21 +130,22 @@ def test_ineligible_reasons(self):
renamed = file("b.yaml", status="renamed", previous_filename="a.yaml", additions=0, changes=0)
del renamed["patch"]
cases = {
"draft": (pr(draft=True), [file()]),
"fork": (pr(fork=True), [file()]),
"not an org member": (pr(association="CONTRIBUTOR"), [file()]),
"author is not an org member": (pr(association="NONE"), [file()]),
"51 changed lines": (pr(additions=30, deletions=21), [file()]),
"deny list (`identity/**`)": (pr(), [file("identity/access.go")]),
"deny list (`.github/**`)": (pr(), [file("ci.yml", previous_filename=".github/workflows/ci.yml")]),
"`bin/helper` has no reviewable text diff": (pr(additions=0), [binary]),
"`b.yaml` has no reviewable text diff": (pr(additions=0), [renamed]),
"draft": (pr(draft=True), [file()], "MEMBER"),
"fork": (pr(fork=True), [file()], "MEMBER"),
"not an org member": (pr(), [file()], "CONTRIBUTOR"),
"author is not an org member": (pr(), [file()], "NONE"),
"51 changed lines": (pr(additions=30, deletions=21), [file()], "MEMBER"),
"deny list (`identity/**`)": (pr(), [file("identity/access.go")], "MEMBER"),
"deny list (`.github/**`)": (pr(), [file("ci.yml", previous_filename=".github/workflows/ci.yml")], "MEMBER"),
"`bin/helper` has no reviewable text diff": (pr(additions=0), [binary], "MEMBER"),
"`b.yaml` has no reviewable text diff": (pr(additions=0), [renamed], "MEMBER"),
}
for expected, (p, files) in cases.items():
for expected, (p, files, association) in cases.items():
with self.subTest(expected=expected):
self.assertIn(expected, pr_risk.ineligible_reason(p, files, patterns))
self.assertIsNone(pr_risk.ineligible_reason(pr(additions=25, deletions=25), [file()], patterns))
self.assertIsNone(pr_risk.ineligible_reason(pr(association="OWNER"), [file()], patterns))
self.assertIn(expected, pr_risk.ineligible_reason(p, files, patterns, association))
# The API reports CONTRIBUTOR for private members; only the event payload's value counts.
self.assertIsNone(pr_risk.ineligible_reason(pr(additions=25, deletions=25), [file()], patterns, "MEMBER"))
self.assertIsNone(pr_risk.ineligible_reason(pr(), [file()], patterns, "OWNER"))

def test_ineligible_without_comment_is_silent(self):
gh = FakeGitHub(pr(additions=200), [file()])
Expand Down Expand Up @@ -280,6 +281,8 @@ def test_workflow_isolates_the_model(self):
names = list(steps)
self.assertLess(names.index("Classify"), names.index("Generate app token"))
self.assertEqual(steps["Generate app token"]["with"]["permission-pull-requests"], "write")
self.assertEqual(steps["Gather"]["env"]["AUTHOR_ASSOCIATION"],
"${{ github.event.pull_request.author_association }}")
triggers = workflow[True]["workflow_call"]
self.assertEqual(set(triggers["secrets"]), {"ADMIN_APP_ID", "ADMIN_APP_PRIVATE_KEY", "ANTHROPIC_API_KEY"})

Expand Down
Loading