chore: add review enforcement workflow#534
Conversation
![baz-reviewer[bot]](https://avatars.githubusercontent.com/in/933528?s=60&v=4){kind=small}
| if (mergeablePr.mergeable === false || mergeablePr.mergeable_state === 'dirty') { | ||
| core.info(`PR #${prNumber} is not mergeable (state: ${mergeablePr.mergeable_state}) - requesting human review.`); | ||
| const requestProductReviewer = require('./.github/scripts/request-product-reviewer.js'); | ||
| await requestProductReviewer({ github, context, core, prNumber, teamSlug: '"product"' }); | ||
| return; |
There was a problem hiding this comment.
dependabot-auto-merge requires ./.github/scripts/request-product-reviewer.js which doesn't exist and throws Cannot find module — should we add the missing helper or remove the import?
Finding type: Logical Bugs | Severity: 🔴 High
Want Baz to fix this for you? Activate Fixer
Other fix methods
Prompt for AI Agents:
Before applying, verify this suggestion against the current code. In
.github/workflows/review-enforcement.yml around lines 164-168, the dependabot-auto-merge
job calls require('./.github/scripts/request-product-reviewer.js') but that module
doesn't exist, causing the job to throw. Add a new file
.github/scripts/request-product-reviewer.js that exports an async function with
signature ({ github, context, core, prNumber, teamSlug }) which selects a reviewer
(e.g., a random member from the given teamSlug or a deterministic fallback), requests a
review via github.rest.pulls.requestReview or github.rest.teams.listMembers +
pulls.requestReview, and returns when the request is created; ensure the module uses
module.exports = requestProductReviewer so require() works. Alternatively, if you prefer
not to add a file, modify the workflow at those lines to inline the reviewer-request
logic instead of requiring a non-existent module.
There was a problem hiding this comment.
Commit 549b297 addressed this comment by introducing .github/scripts/request-team-reviewer.js (which exports the required reviewer helper) and updating the dependabot jobs to require that existing script instead of the missing request-product-reviewer.js.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #534 +/- ##
==========================================
+ Coverage 82.67% 82.76% +0.09%
==========================================
Files 119 119
Lines 9725 9750 +25
==========================================
+ Hits 8040 8070 +30
+ Misses 1685 1680 -5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
john-weiler
force-pushed
the
chore/review-enforcement-workflow
branch
3 times, most recently
from
549b297 to
470668c
Compare
| // Fetch reviews with retry for eventual consistency | ||
| let reviews; | ||
| let attempts = 0; | ||
| while (attempts < 3) { | ||
| const result = await github.rest.pulls.listReviews({ | ||
| owner: context.repo.owner, | ||
| repo: context.repo.repo, | ||
| pull_number: pr.number | ||
| }); | ||
| reviews = result.data; | ||
|
|
||
| // If we have reviews or this is not a review event, stop retrying |
There was a problem hiding this comment.
listReviews returns full history but filtering state === 'APPROVED' without deduping by reviewer lets approvals and hasNonBazApproval stay true after later negative reviews — should we collapse reviews to each reviewer's latest review or ignore approvals superseded by a non-APPROVED state before deciding the policy is satisfied?
Finding type: Logical Bugs | Severity: 🔴 High
Want Baz to fix this for you? Activate Fixer
Other fix methods
Prompt for AI Agents:
Before applying, verify this suggestion against the current code. In
.github/workflows/review-enforcement.yml around lines 57 to 95, the
review-fetching/approval-check logic incorrectly treats every APPROVED entry in the
review history the same and does not deduplicate by reviewer, so an older approval can
be counted even if that reviewer later submitted CHANGES_REQUESTED or had their approval
dismissed. Change the logic to collapse reviews to each reviewer's latest review (use
submitted_at or the most recent entry) before computing approvals — build a map keyed
by reviewer login selecting the latest review, then derive approvals from that
deduplicated set and check for a non-baz-reviewer APPROVED state. Ensure DISMISSED and
CHANGES_REQUESTED override earlier APPROVED entries so they are not counted.
470668c to
671e78b
Compare
john-weiler
force-pushed
the
chore/review-enforcement-workflow
branch
from
671e78b to
7567416
Compare
| const { data: reviewRequests } = | ||
| await github.rest.pulls.listRequestedReviewers({ | ||
| owner, | ||
| repo, | ||
| pull_number: prNumber | ||
| }); | ||
| const hasPendingTeamReview = reviewRequests.users.some((u) => | ||
| teamMemberSet.has(u.login) | ||
| ); |
There was a problem hiding this comment.
request-team-reviewer only checks reviewRequests.users for a member login — should we treat an existing entry in reviewRequests.teams for the requested slug as already requested before selecting a random member?
Finding type: Logical Bugs | Severity: 🔴 High
Want Baz to fix this for you? Activate Fixer
Other fix methods
Prompt for AI Agents:
Before applying, verify this suggestion against the current code. In
.github/scripts/request-team-reviewer.js around lines 45 to 53, the logic that
determines whether a team review is already requested only checks reviewRequests.users
and ignores reviewRequests.teams. Update this block to also inspect reviewRequests.teams
and treat an existing team request for the same team slug (or a team entry whose slug
equals teamSlug or whose name/full_name equals `rungalileo/${teamSlug}`) as already
requested; if found, log the skip and return without selecting a random member. Ensure
the check works whether the API returns a slug property or a full/team name.
|
No activity for 30 days — this PR will be closed in 5 days unless updated. |
1 similar comment
|
No activity for 30 days — this PR will be closed in 5 days unless updated. |
3 participants
User description
Summary
This PR adds review enforcement workflow to support automatic merging of Dependabot PRs when CI passes, with automatic review request escalation when CI fails.
The workflow ensures:
The team for review requests is configured per-repo:
Test plan
Generated description
Below is a concise technical summary of the changes proposed in this PR:
Add a review enforcement workflow that tracks approvals and blocks non-Dependabot pull requests until a non-
baz-reviewerteam member signs off. Add Dependabot automation that leveragesgetDependabotPrandrequest-team-reviewerhelpers to auto-approve/merge passing Dependabot PRs and to escalate failures with random team reviewers.baz-reviewerUserapproval before allowing progress.Modified files (1)
Latest Contributors(0)
getDependabotPrandrequest-team-reviewerto approve and merge successful CI runs or to pick a random team reviewer when CI fails, including random team selection safeguards and auto-merge state handling.Modified files (3)
Latest Contributors(0)