Repository navigation
Add shadow-mode PR risk classifier - #25
Conversation
|
Picked sjmiller609 to review these related PRs (#25, kernel/infra#1825, kernel/kernel#5021, kernel/kernel-images-private#517, kernel/hypeman#531): recent commits in |
| Shadow-mode risk check for small PRs. When a PR is opened or marked ready, and on every push, | ||
| it leaves one comment saying whether the PR would have been auto-approved and why. It never | ||
| approves. | ||
|
|
||
| These PRs are skipped with no comment: drafts, fork PRs, PRs over 50 changed lines, and PRs | ||
| that touch a deny-listed path. The defaults live in `scripts/pr_risk.py`, and callers can add | ||
| more through `deny-paths`. Claude classifies everything else and can deny a PR on its own | ||
| judgment. The comment stores its verdict in a hidden `pr-risk:v1` JSON marker for later analysis. |
There was a problem hiding this comment.
It doesn't seem like it would be too expensive because we skip anything over 50 lines.
I'm wondering if this use case seems right for Jev kind of model, it would just be a lot faster maybe?
There was a problem hiding this comment.
Good call on cost, it should stay cheap since everything over 50 lines is skipped before the model runs.
I'll test Jev before we turn on auto-approve after the two-week shadow run. By then we'll have a labeled set (each verdict plus 👍/👎 reactions and what reviewers actually did), so I can run Jev and Claude on the same PRs and compare them directly.
For now I think Claude is the better fit:
- The hard part is the deny check, not the yes/no. Getting it right means reading the diff and the code around it to work out what a change actually does. For example, a one-line flag edit that adds an org to the BAA group, or a dropped
site-per-processentry that turns out to be a sandboxing control. Claude caught both in the dry runs, and that kind of reasoning seems like a stretch for a model that returns probabilities. - Speed doesn't matter much here. The comment is posted in the background and nobody waits on it.
- The reason is the useful output during the shadow run. Reviewers judge a verdict by the sentence explaining it. That's what the 👍/👎 reactions measure, and it's what we'll go back to in two weeks.
Where Jev could be a good fit is phase 2. Its probabilities would give us a tunable approval threshold, possibly with Claude still writing the reason or handling the uncertain cases.
Summary
Adds
pr-risk.yml, a reusable workflow that comments on small PRs to say whether they would have been auto-approved and why. It never approves. This is a two-week shadow run, so we can measure how many PRs would qualify, whether they were really low risk, and how much review wait they would have saved, before deciding on real auto-approval.Flow (on
opened,ready_for_reviewandsynchronize; drafts are skipped):scripts/pr_risk.py gather, caller's read-only token). The PR is skipped with no comment if it is a draft, comes from a fork, changes more than 50 lines, touches a deny-listed path (defaults in the script plus the caller'sdeny-paths), or has a file with no text diff. Otherwise it builds the model input: title and description, the diff, ±30 line-numbered lines of head content around each hunk, andAGENTS.mdread from the base branch so the PR cannot rewrite it.claude -p --tools ""runs in an empty directory with--json-schema. The prompt treats all PR content as untrusted. It defines zero/low/medium/high tiers and adenylist the model applies on its own judgment (auth, secrets, security controls, billing, compliance groups like BAA/ZDR, schema and data deletion, CI/CD, capacity and routing, removed safety checks, anything it can't understand). The model step only seesANTHROPIC_API_KEY.scripts/pr_risk.py comment). Thekernel-internaltoken is minted after the model step, scoped to the current repo withpull-requests: writeonly. It writes one sticky comment under 100 words. "Would auto-approve" means not denied, tier zero or low, and confidence not low. The comment ends with a hiddenpr-risk:v1JSON marker holding the verdict and per-commit history. A later push that makes the PR ineligible rewrites an existing comment to "no", so a stale "yes" never stays up. A run whose head commit has moved does not post.Secrets are named rather than inherited, matching
breakglass-merge.yml.Testing
scripts/test_pr_risk.py, 14 tests, run by the newpr-risk-tests.yml. Covers the deny-list globs, every reason a PR is ineligible, silent skips, creating vs. updating the comment, rewriting a stale comment, ignoring marker comments from people, classifier failures, a head that moved mid-run, the <100-word limit, how the model input is built (base-branch guidance, tag scrubbing, context ranges), thegh apicalls the client makes (via a stubgh), and the workflow's credential layout. All existing tests still pass.actionlintis clean.Note
Medium Risk
Adds CI/CD and PR-comment automation with GitHub App write access and external script fetch; shadow-only today, but it shapes future auto-approval policy and handles sensitive PR metadata via the model API.
Overview
Introduces a shadow-mode PR risk classifier as a reusable
pr-risk.ymlworkflow plusscripts/pr_risk.py, so consumer repos can see whether small PRs would have been auto-approved—without ever approving or merging.Eligibility is enforced before any model call: skip silently for drafts, forks, non–org-member authors, >50 changed lines, deny-listed paths (lockfiles,
.github/**, secrets patterns, etc., with optionaldeny-paths), or files without a text diff. Eligible PRs get a gather → classify → comment pipeline: build untrusted-tagged model input (diff, base-branchAGENTS.md, surrounding line context), run Claude Code with no tools and onlyANTHROPIC_API_KEY, then post or update one bot comment via a repo-scoped GitHub App token minted only after classification (pull-requests: write).Comments state Would auto-approve: yes/no from tier (
zero/low+ notdeny+ confidence notlow), embed hiddenpr-risk:v1JSON for analytics, rewrite stale “yes” when a PR later becomes ineligible, and avoid races when HEAD moves mid-run.Also adds
classify-prompt.md,pr-risk-tests.yml(unittest + PyYAML), README consumer wiring, and 14 unit tests covering globs, ineligibility, comment lifecycle, prompt injection scrubbing, and workflow credential isolation.Reviewed by Cursor Bugbot for commit cbd706d. Bugbot is set up for automated code reviews on this repo. Configure here.