Skip to content

ci: Add PR review enforcement GitHub Action #490

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
john-weiler wants to merge 1 commit into
base: main
Choose a base branch
from
Open

ci: Add PR review enforcement GitHub Action #490

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
128 changes: 128 additions & 0 deletions .github/workflows/review-enforcement.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
name: Enforce Review Policy

on:
pull_request:
types: [opened, synchronize, reopened]
pull_request_review:
types: [submitted, dismissed]
workflow_run:
workflows: ["Run Tests"]
types: [completed]

jobs:
enforce-review:
runs-on: ubuntu-latest
permissions:
pull-requests: write
Copy link

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This job only reads PR data; it doesn’t perform any PR write operations. Consider reducing permissions from pull-requests: write to pull-requests: read here to follow least-privilege.

Suggested change
pull-requests: write
pull-requests: read

Copilot uses AI. Check for mistakes.
steps:
- name: Enforce review policy
uses: actions/github-script@v7
with:
script: |
const pr = context.payload.pull_request;
const event = context.eventName;
const isDependabot = pr.user?.login === 'dependabot[bot]';

Comment on lines +22 to +25
Copy link

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

enforce-review runs for workflow_run events too, but this script assumes context.payload.pull_request exists. On workflow_run, pull_request is undefined, so pr.user?.login will throw before the optional chain is evaluated. Add a job-level if: github.event_name != 'workflow_run' (or handle workflow_run explicitly by resolving the PR from context.payload.workflow_run.pull_requests and fetching PR data).

Copilot uses AI. Check for mistakes.
// Skip for Dependabot on all events
if (isDependabot) {
core.info('Dependabot PR detected - skipping review enforcement.');
return;
}

// Only enforce review requirement on pull_request_review events
if (event === 'pull_request') {
core.info('PR opened/synchronized - waiting for review, not enforcing yet.');
return;
}

// From here on, we know it's a pull_request_review event (submitted or dismissed): enforce rule
core.info('Review event detected - checking current approval status...');

const reviews = await github.rest.pulls.listReviews({
owner: context.repo.owner,
repo: context.repo.repo,
pull_number: pr.number
Comment on lines +22 to +44
Copy link

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On pull_request (opened/synchronize/reopened) this job exits successfully without checking approvals. If this workflow is used as a required status check, it can allow merging without approval and also won’t re-validate after new commits (when approvals may be dismissed). Consider running the approval check on pull_request events as well and failing when no valid approval exists.

Suggested change
const pr = context.payload.pull_request;
const event = context.eventName;
const isDependabot = pr.user?.login === 'dependabot[bot]';
// Skip for Dependabot on all events
if (isDependabot) {
core.info('Dependabot PR detected - skipping review enforcement.');
return;
}
// Only enforce review requirement on pull_request_review events
if (event === 'pull_request') {
core.info('PR opened/synchronized - waiting for review, not enforcing yet.');
return;
}
// From here on, we know it's a pull_request_review event (submitted or dismissed): enforce rule
core.info('Review event detected - checking current approval status...');
const reviews = await github.rest.pulls.listReviews({
owner: context.repo.owner,
repo: context.repo.repo,
pull_number: pr.number
const event = context.eventName;
const { owner, repo } = context.repo;
let prNumber;
let prUserLogin;
if (event === 'pull_request' || event === 'pull_request_review') {
const pr = context.payload.pull_request;
if (!pr) {
core.setFailed('Pull request context not found for event: ' + event);
return;
}
prNumber = pr.number;
prUserLogin = pr.user?.login;
} else if (event === 'workflow_run') {
const workflowRun = context.payload.workflow_run;
const associatedPr = workflowRun?.pull_requests?.[0];
if (!associatedPr) {
core.info('Workflow run is not associated with a pull request - skipping review enforcement.');
return;
}
prNumber = associatedPr.number;
const prResponse = await github.rest.pulls.get({
owner,
repo,
pull_number: prNumber
});
prUserLogin = prResponse.data.user?.login;
} else {
core.info(`Event '${event}' is not supported by this workflow - skipping review enforcement.`);
return;
}
const isDependabot = prUserLogin === 'dependabot[bot]';
// Skip for Dependabot on all events
if (isDependabot) {
core.info('Dependabot PR detected - skipping review enforcement.');
return;
}
core.info('Checking current approval status for pull request #' + prNumber + '...');
const reviews = await github.rest.pulls.listReviews({
owner,
repo,
pull_number: prNumber

Copilot uses AI. Check for mistakes.
});
Comment on lines +22 to +45
Copy link

@baz-reviewer baz-reviewer bot Feb 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

context.payload.pull_request is undefined on workflow_run events, but the script reads pr.user?.login before checking the event type, so the job will throw before reaching the guard whenever the workflow_run trigger fires; can we guard against missing pull_request (or skip workflow_run) before dereferencing pr?

Finding type: Logical Bugs

Prompt for AI Agents:

In .github/workflows/review-enforcement.yml around lines 22 to 45, the script assigns
const pr = context.payload.pull_request and then immediately reads pr.user?.login, but
on workflow_run events payload.pull_request is undefined causing a crash. Change the
logic to first check the event type and/or the existence of context.payload.pull_request
before dereferencing pr: either move the event/type check above the isDependabot check
or add a guard like if (!context.payload.pull_request) { core.info('No pull_request in
payload - skipping'); return; } so workflow_run runs don’t throw when pull_request is
missing.

Fix in Cursor


const approvals = reviews.data.filter(r => r.state === 'APPROVED');
Comment on lines +41 to +47
Copy link

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pulls.listReviews is paginated (defaults to 30). For PRs with many reviews, the required approval could be on a later page and be missed. Set per_page: 100 and paginate through all pages (or use Octokit pagination helpers) before evaluating approvals.

Suggested change
const reviews = await github.rest.pulls.listReviews({
owner: context.repo.owner,
repo: context.repo.repo,
pull_number: pr.number
});
const approvals = reviews.data.filter(r => r.state === 'APPROVED');
const reviews = await github.paginate(
github.rest.pulls.listReviews,
{
owner: context.repo.owner,
repo: context.repo.repo,
pull_number: pr.number,
per_page: 100
}
);
const approvals = reviews.filter(r => r.state === 'APPROVED');

Copilot uses AI. Check for mistakes.
const hasNonBazApproval = approvals.some(
r => r.user?.login &&
r.user.login !== 'baz-reviewer' &&
r.user.type === 'User'
Comment on lines +47 to +51
Copy link

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The approval check counts any review with state APPROVED, even if the same reviewer later requested changes or the approval was dismissed. GitHub review state is effectively “latest review per user”, so this can incorrectly pass. Compute each reviewer’s latest review state (e.g., by sorting by submitted_at and keeping the most recent per user.login) before deciding whether an approval exists.

Suggested change
const approvals = reviews.data.filter(r => r.state === 'APPROVED');
const hasNonBazApproval = approvals.some(
r => r.user?.login &&
r.user.login !== 'baz-reviewer' &&
r.user.type === 'User'
// Determine the latest review per user (GitHub treats review state as latest review per user)
const latestReviewsByUser = new Map();
for (const r of reviews.data) {
const user = r.user;
const login = user?.login;
if (!login || user.type !== 'User') {
continue;
}
const existing = latestReviewsByUser.get(login);
if (
!existing ||
(r.submitted_at && new Date(r.submitted_at) > new Date(existing.submitted_at))
) {
latestReviewsByUser.set(login, r);
}
}
const approvals = Array.from(latestReviewsByUser.values()).filter(
r => r.state === 'APPROVED'
);
const hasNonBazApproval = approvals.some(
r => r.user?.login && r.user.login !== 'baz-reviewer'

Copilot uses AI. Check for mistakes.
);

if (!hasNonBazApproval) {
core.setFailed('At least one approval from a non-baz-reviewer is required.');
Copy link

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The failure message says “non-baz-reviewer”, but the logic also enforces “non-bot” via r.user.type === 'User'. Consider updating the message to reflect the actual requirement (human approval, excluding baz-reviewer) so it’s actionable for contributors.

Suggested change
core.setFailed('At least one approval from a non-baz-reviewer is required.');
core.setFailed("At least one approval from a human reviewer other than 'baz-reviewer' is required.");

Copilot uses AI. Check for mistakes.
}

dependabot-ci-failure:
runs-on: ubuntu-latest
if: github.event.workflow_run.conclusion == 'failure'
permissions:
pull-requests: write
steps:
- name: Request review from random product team member on CI failure
uses: actions/github-script@v7
with:
github-token: ${{ secrets.GALILEO_AUTOMATION_GITHUB_TOKEN }}
script: |
// Get the PR associated with this workflow run
const workflowRun = context.payload.workflow_run;

if (!workflowRun.pull_requests || workflowRun.pull_requests.length === 0) {
core.info('No pull request associated with this workflow run.');
return;
}

const prNumber = workflowRun.pull_requests[0].number;

// Get PR details
const { data: pr } = await github.rest.pulls.get({
owner: context.repo.owner,
repo: context.repo.repo,
pull_number: prNumber
});

const isDependabot = pr.user?.login === 'dependabot[bot]';

if (!isDependabot) {
core.info('PR is not from Dependabot - skipping CI failure notification.');
return;
}

core.info(`Dependabot PR #${prNumber} CI failed - requesting review from random product team member.`);

// Get members of the rungalileo/product team
let teamMembers;
try {
const { data: members } = await github.rest.teams.listMembersInOrg({
org: 'rungalileo',
team_slug: 'product',
});
teamMembers = members.map(m => m.login);
} catch (error) {
core.setFailed(`Failed to fetch team members: ${error.message}`);
return;
}

if (teamMembers.length === 0) {
core.setFailed('No members found in rungalileo/product team.');
return;
}

// Select a random team member
const randomMember = teamMembers[Math.floor(Math.random() * teamMembers.length)];
core.info(`Selected random reviewer: ${randomMember}`);

// Request review from the selected member
try {
await github.rest.pulls.requestReviewers({
owner: context.repo.owner,
repo: context.repo.repo,
pull_number: prNumber,
reviewers: [randomMember]
});
core.info(`Successfully requested review from ${randomMember} for PR #${prNumber}`);
} catch (error) {
core.setFailed(`Failed to request review: ${error.message}`);
}
Loading