Skip to content

Commit 154287c

Browse files
authored
CI - Simplify PR approval check (#4578)
# Description of Changes The previous version ended up incurring two different entries on the PR (one for the `pull_request` event and one for the `pull_request_review` event). Both versions were marked "required", so PRs could end up in an unmergeable state if one check had succeeded but the other had failed (e.g. if you submitted a PR approval, the previous `pull_request` version of the check would still be failed since it didn't refresh). See the entries at top and bottom here: <img width="481" height="225" alt="image" src="https://github.com/user-attachments/assets/5b7a4302-6bc2-47e9-93c8-812cb9ece60b" /> This PR fixes it by only allowing the `pull_request_review` events. I _think_ this covers all the cases, but I'm not sure. # API and ABI breaking changes None. # Expected complexity level and risk 1 # Testing I don't know how to test it really 🤷 --------- Co-authored-by: Zeke Foppa <bfops@users.noreply.github.com>
1 parent 21b5af0 commit 154287c

1 file changed

Lines changed: 93 additions & 32 deletions

File tree

Lines changed: 93 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1,45 +1,106 @@
1-
name: PR Approval Check
1+
name: Review Checks
22

33
on:
44
pull_request:
5-
types: [opened, synchronize, reopened, ready_for_review]
5+
types: [opened, synchronize, reopened]
66
pull_request_review:
7-
types: [submitted]
7+
types: [submitted, dismissed]
8+
merge_group:
89

910
permissions:
1011
contents: read
12+
pull-requests: read
13+
statuses: write
14+
15+
concurrency:
16+
group: pr-approval-check-${{ github.event.pull_request.number || github.sha }}
17+
cancel-in-progress: true
1118

1219
jobs:
13-
check-approvals:
20+
publish-approval-status:
21+
name: Set approval status
1422
runs-on: ubuntu-latest
15-
# Skip this check if PR is in draft state
16-
if: github.event.pull_request.draft == false
1723

1824
steps:
19-
- name: Checkout code
20-
uses: actions/checkout@v3
25+
- name: Evaluate and publish approval status
26+
uses: actions/github-script@v7
2127
with:
22-
fetch-depth: 0
23-
24-
- name: Check PR approvals
25-
env:
26-
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
27-
run: |
28-
# Check if PR author is clockwork-labs-bot
29-
if [[ "${{ github.event.pull_request.user.login }}" != "clockwork-labs-bot" ]]; then
30-
echo "PR opened by ${{ github.event.pull_request.user.login }}, not clockwork-labs-bot. Skipping check."
31-
exit 0
32-
fi
33-
34-
PR_NUMBER="${{ github.event.pull_request.number }}"
35-
# Get approval count
36-
APPROVALS=$(gh pr view $PR_NUMBER --json reviews -q '.reviews | map(select(.state == "APPROVED")) | length')
37-
38-
echo "PR has $APPROVALS approvals"
39-
40-
if [[ $APPROVALS -lt 2 ]]; then
41-
echo "Error: PRs from clockwork-labs-bot require at least 2 approvals"
42-
exit 1
43-
else
44-
echo "PR has the required number of approvals"
45-
fi
28+
github-token: ${{ secrets.GITHUB_TOKEN }}
29+
script: |
30+
const contextName = "PR approval check";
31+
32+
let targetSha;
33+
let state;
34+
let description;
35+
36+
if (context.eventName === "merge_group") {
37+
targetSha = process.env.GITHUB_SHA;
38+
state = "success";
39+
description = "Merge group entry; approvals already satisfied";
40+
} else {
41+
const pr = context.payload.pull_request;
42+
targetSha = pr.head.sha;
43+
44+
if (pr.user.login !== "clockwork-labs-bot") {
45+
state = "success";
46+
description = "PR author is not clockwork-labs-bot";
47+
} else {
48+
const result = await github.graphql(
49+
`
50+
query($owner: String!, $repo: String!, $number: Int!) {
51+
repository(owner: $owner, name: $repo) {
52+
pullRequest(number: $number) {
53+
latestOpinionatedReviews(first: 100, writersOnly: true) {
54+
nodes {
55+
state
56+
author {
57+
login
58+
}
59+
}
60+
}
61+
}
62+
}
63+
}
64+
`,
65+
{
66+
owner: context.repo.owner,
67+
repo: context.repo.repo,
68+
number: pr.number,
69+
}
70+
);
71+
72+
const effectiveApprovers =
73+
result.repository.pullRequest.latestOpinionatedReviews.nodes
74+
.filter((review) => review.state === "APPROVED")
75+
.map((review) => review.author?.login)
76+
.filter(Boolean);
77+
78+
core.info(
79+
`Latest effective approvers (${effectiveApprovers.length}): ${effectiveApprovers.join(", ")}`
80+
);
81+
82+
if (effectiveApprovers.length < 2) {
83+
state = "failure";
84+
description = "PRs from clockwork-labs-bot require at least 2 approvals";
85+
} else {
86+
state = "success";
87+
description = "PR has the required number of approvals";
88+
}
89+
}
90+
}
91+
92+
core.info(`Publishing status ${state} for ${targetSha}: ${description}`);
93+
94+
// We need to set a separate commit status for this, because it runs on both
95+
// pull_request and pull_request_review events. If we don't set an explicit context,
96+
// what happens is that there are sometimes two separate statuses on the same commit -
97+
// one from each event type. This leads to weird cases where one copy of the check is failed,
98+
// and the other is successful, and the failed one blocks the PR from merging.
99+
await github.rest.repos.createCommitStatus({
100+
owner: context.repo.owner,
101+
repo: context.repo.repo,
102+
sha: targetSha,
103+
state,
104+
context: contextName,
105+
description,
106+
});

0 commit comments

Comments
 (0)