chore: Add PR validation workflow#5078
Conversation
Automatically validates non-maintainer PRs by checking: - Issue reference exists in PR body - Referenced issue has discussion between author and maintainer - Referenced issue is not assigned to someone else Also enforces that all PRs start as drafts. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Semver Impact of This PR⚪ None (no version bump detected) 📋 Changelog PreviewThis is how your changes will appear in the changelog. Breaking Changes 🛠
Features ✨
Fixes 🐛
Dependencies ⬆️Deps
Other
🤖 This preview updates automatically when you update the PR. |
![sentry-warden[bot]](https://avatars.githubusercontent.com/in/2755838?s=60&v=4){kind=small}
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5078 +/- ##
=======================================
Coverage 74.00% 74.01%
=======================================
Files 499 499
Lines 18066 18066
Branches 3518 3518
=======================================
+ Hits 13370 13371 +1
Misses 3837 3837
+ Partials 859 858 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
stephanie-anderson
requested review from
Flash0ver and
jamescrosswell
as code owners
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| for (const user of usersToCheck) { | ||
| if (user === prAuthor) continue; | ||
| if (await isMaintainer(repo.owner, repo.repo, user)) { |
There was a problem hiding this comment.
Bug: The maintainer check at line 204 incorrectly uses the PR's repository (repo.owner, repo.repo) instead of the issue's repository (ref.owner, ref.repo) for validation.
Severity: HIGH
Suggested Fix
Update the call to the isMaintainer function to use the correct variables for the issue's repository. Change isMaintainer(repo.owner, repo.repo, user) to isMaintainer(ref.owner, ref.repo, user).
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: .github/workflows/validate-pr.yml#L204
Potential issue: The workflow is designed to check if a maintainer has participated in a
linked issue, even if that issue is in a different repository. However, the call to
`isMaintainer` on line 204 incorrectly uses the PR's repository details (`repo.owner`,
`repo.repo`) instead of the issue's repository details (`ref.owner`, `ref.repo`). This
causes the check to fail when a maintainer of the issue's repository (but not the PR's
repository) comments on the issue, leading to the PR being incorrectly closed for
lacking maintainer discussion.
Did we get this right? 👍 / 👎 to inform future reviews.
| if: | | ||
| always() | ||
| && github.event.pull_request.draft == false | ||
| && needs.validate-non-maintainer-pr.outputs.was-closed != 'true' |
There was a problem hiding this comment.
Bug: The enforce-draft job incorrectly runs on maintainer PRs because the was-closed output is unset, causing the conditional check to pass erroneously.
Severity: MEDIUM
Suggested Fix
The validate-non-maintainer-pr job should produce an output to indicate if the author was a maintainer. The enforce-draft job's conditional check should be updated to use this new output to skip execution for maintainers.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: .github/workflows/validate-pr.yml#L262-L265
Potential issue: When a maintainer opens a pull request, the
`validate-non-maintainer-pr` job correctly exits early but fails to set the `was-closed`
output. This output defaults to an empty string. The subsequent `enforce-draft` job has
a condition `needs.validate-non-maintainer-pr.outputs.was-closed != 'true'`, which
evaluates to `true` because an empty string is not equal to `'true'`. As a result, the
`enforce-draft` job incorrectly runs and converts the maintainer's non-draft PR into a
draft, which is contrary to the intended design.
Did we get this right? 👍 / 👎 to inform future reviews.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
|
|
||
| - name: Convert PR to draft | ||
| env: | ||
| GH_TOKEN: ${{github.token}} |
There was a problem hiding this comment.
Wrong token used for converting PR to draft
High Severity
The Convert PR to draft step sets GH_TOKEN to ${{github.token}} (the default GITHUB_TOKEN), but gh pr ready --undo uses the convertPullRequestToDraft GraphQL mutation, which is known to fail with the default GITHUB_TOKEN — even with pull-requests: write permission. The app token is generated in the previous step specifically for this purpose but goes unused here. GH_TOKEN needs to use ${{ steps.app-token.outputs.token }} instead.
Additional Locations (1)
| for (const user of usersToCheck) { | ||
| if (user === prAuthor) continue; | ||
| if (await isMaintainer(repo.owner, repo.repo, user)) { |
There was a problem hiding this comment.
Maintainer check uses PR's repo instead of issue's repo
Medium Severity
The comment on line 193 says "admin/maintain access on the issue's repo," but isMaintainer is called with repo.owner, repo.repo (the PR's target repo) instead of ref.owner, ref.repo (the referenced issue's repo). For cross-repo references (e.g., a PR to sentry-dotnet referencing getsentry/sentry#123), a sentry maintainer who discussed the issue would not be recognized, incorrectly closing the PR for "missing maintainer discussion."
Additional Locations (1)
2 participants
Summary
validate-pr.ymlworkflow to automatically validate non-maintainer PRsRollout of getsentry/sentry-python#4233 across all SDK repos.
Test plan
SDK_MAINTAINER_BOT_APP_IDvar andSDK_MAINTAINER_BOT_PRIVATE_KEYsecret are available to this repoCo-Authored-By: Claude Opus 4.6 (1M context) noreply@anthropic.com