Skip to content

Commit 8182986

Browse files
bfopsbfops
committed
[bfops/pr-approval-check-again]: CI - maybe fix PR approval check
1 parent 5c7d7e7 commit 8182986

2 files changed

Lines changed: 111 additions & 88 deletions

File tree

‎.github/workflows/pr_approval_check.yml‎

Lines changed: 51 additions & 88 deletions
Original file line numberDiff line numberDiff line change
@@ -1,120 +1,83 @@
1-
name: Review Checks
2-
3-
# SECURITY: This workflow uses pull_request_target so that it has write access to
4-
# set commit statuses on external (fork) PRs. pull_request_target runs in the
5-
# context of the base branch, which grants the GITHUB_TOKEN write permissions
6-
# that a regular pull_request event on a fork would not have.
7-
#
8-
# IMPORTANT: This workflow must NEVER check out, build, or execute code from the
9-
# PR branch. Doing so would allow a malicious fork to run arbitrary code with
10-
# write access to the repository. This workflow only reads PR metadata via the
11-
# GitHub API, which is safe.
1+
name: Review Checks Evaluator
122

133
on:
14-
pull_request_target:
4+
pull_request:
155
types: [opened, synchronize, reopened]
166
pull_request_review:
177
types: [submitted, dismissed]
188
merge_group:
199

20-
permissions:
21-
contents: read
22-
pull-requests: read
23-
statuses: write
10+
permissions: read-all
2411

2512
concurrency:
2613
group: pr-approval-check-${{ github.event.pull_request.number || github.sha }}
2714
cancel-in-progress: true
2815

2916
jobs:
30-
publish-approval-status:
31-
name: Set approval status
17+
evaluate-approval-status:
18+
name: Evaluate approval status
3219
runs-on: ubuntu-latest
3320

34-
# SECURITY: Do not add a checkout step to this job. See comment at the top of this file.
3521
steps:
36-
- name: Evaluate and publish approval status
22+
- name: Evaluate approval status
3723
uses: actions/github-script@v7
3824
with:
3925
github-token: ${{ secrets.GITHUB_TOKEN }}
4026
script: |
41-
const contextName = "PR approval check";
27+
if (context.eventName === "merge_group") {
28+
core.info("Merge group entry; approvals already satisfied.");
29+
return;
30+
}
4231
43-
let targetSha;
44-
let state;
45-
let description;
32+
const pr = context.payload.pull_request;
4633
47-
if (context.eventName === "merge_group") {
48-
targetSha = process.env.GITHUB_SHA;
49-
state = "success";
50-
description = "Merge group entry; approvals already satisfied";
51-
} else {
52-
const pr = context.payload.pull_request;
53-
targetSha = pr.head.sha;
34+
if (pr.head.repo.fork) {
35+
core.info("External PR; evaluator intentionally does not publish statuses.");
36+
return;
37+
}
38+
39+
if (pr.user.login !== "clockwork-labs-bot") {
40+
core.info("PR author is not clockwork-labs-bot; no extra approval requirement.");
41+
return;
42+
}
5443
55-
if (pr.head.repo.fork) {
56-
state = "success";
57-
description = "Skipped for external PR";
58-
} else if (pr.user.login !== "clockwork-labs-bot") {
59-
state = "success";
60-
description = "PR author is not clockwork-labs-bot";
61-
} else {
62-
const result = await github.graphql(
63-
`
64-
query($owner: String!, $repo: String!, $number: Int!) {
65-
repository(owner: $owner, name: $repo) {
66-
pullRequest(number: $number) {
67-
latestOpinionatedReviews(first: 100, writersOnly: true) {
68-
nodes {
69-
state
70-
author {
71-
login
72-
}
73-
}
44+
const result = await github.graphql(
45+
`
46+
query($owner: String!, $repo: String!, $number: Int!) {
47+
repository(owner: $owner, name: $repo) {
48+
pullRequest(number: $number) {
49+
latestOpinionatedReviews(first: 100, writersOnly: true) {
50+
nodes {
51+
state
52+
author {
53+
login
7454
}
7555
}
7656
}
7757
}
78-
`,
79-
{
80-
owner: context.repo.owner,
81-
repo: context.repo.repo,
82-
number: pr.number,
83-
}
84-
);
58+
}
59+
}
60+
`,
61+
{
62+
owner: context.repo.owner,
63+
repo: context.repo.repo,
64+
number: pr.number,
65+
}
66+
);
8567
86-
const effectiveApprovers =
87-
result.repository.pullRequest.latestOpinionatedReviews.nodes
88-
.filter((review) => review.state === "APPROVED")
89-
.map((review) => review.author?.login)
90-
.filter(Boolean);
68+
const effectiveApprovers =
69+
result.repository.pullRequest.latestOpinionatedReviews.nodes
70+
.filter((review) => review.state === "APPROVED")
71+
.map((review) => review.author?.login)
72+
.filter(Boolean);
9173
92-
core.info(
93-
`Latest effective approvers (${effectiveApprovers.length}): ${effectiveApprovers.join(", ")}`
94-
);
74+
core.info(
75+
`Latest effective approvers (${effectiveApprovers.length}): ${effectiveApprovers.join(", ")}`
76+
);
9577
96-
if (effectiveApprovers.length < 2) {
97-
state = "failure";
98-
description = "PRs from clockwork-labs-bot require at least 2 approvals";
99-
} else {
100-
state = "success";
101-
description = "PR has the required number of approvals";
102-
}
103-
}
78+
if (effectiveApprovers.length < 2) {
79+
core.setFailed("PRs from clockwork-labs-bot require at least 2 approvals.");
80+
return;
10481
}
10582
106-
core.info(`Publishing status ${state} for ${targetSha}: ${description}`);
107-
108-
// We need to set a separate commit status for this, because it runs on both
109-
// pull_request and pull_request_review events. If we don't set an explicit context,
110-
// what happens is that there are sometimes two separate statuses on the same commit -
111-
// one from each event type. This leads to weird cases where one copy of the check is failed,
112-
// and the other is successful, and the failed one blocks the PR from merging.
113-
await github.rest.repos.createCommitStatus({
114-
owner: context.repo.owner,
115-
repo: context.repo.repo,
116-
sha: targetSha,
117-
state,
118-
context: contextName,
119-
description,
120-
});
83+
core.info("PR has the required number of approvals.");

‎.github/workflows/pr_approval_report.yml‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
name: Review Checks Reporter
2+
3+
# SECURITY: This workflow runs with base-repository privileges after the
4+
# evaluator completes. It must not check out or execute PR code, and it must not
5+
# consume artifacts from the untrusted workflow. It only reads trusted workflow
6+
# metadata and publishes a narrow commit-status update.
7+
8+
on:
9+
workflow_run:
10+
workflows:
11+
- Review Checks Evaluator
12+
types:
13+
- completed
14+
15+
permissions:
16+
contents: read
17+
statuses: write
18+
19+
jobs:
20+
publish-approval-status:
21+
name: Publish approval status
22+
runs-on: ubuntu-latest
23+
24+
# SECURITY: Do not add a checkout step to this job. See comment at the top of this file.
25+
steps:
26+
- name: Publish approval status
27+
uses: actions/github-script@v7
28+
with:
29+
github-token: ${{ secrets.GITHUB_TOKEN }}
30+
script: |
31+
const contextName = "PR approval check";
32+
const conclusion = context.payload.workflow_run.conclusion;
33+
34+
let state;
35+
let description;
36+
37+
if (conclusion === "success") {
38+
state = "success";
39+
description = "Approval requirements satisfied";
40+
} else if (conclusion === "cancelled" || conclusion === "timed_out") {
41+
state = "error";
42+
description = `Approval evaluation ${conclusion}`;
43+
} else {
44+
state = "failure";
45+
description = "Approval requirements not satisfied";
46+
}
47+
48+
core.info(
49+
`Publishing ${state} for ${context.payload.workflow_run.head_sha} from evaluator conclusion ${conclusion}`
50+
);
51+
52+
await github.rest.repos.createCommitStatus({
53+
owner: context.repo.owner,
54+
repo: context.repo.repo,
55+
sha: context.payload.workflow_run.head_sha,
56+
state,
57+
context: contextName,
58+
description,
59+
target_url: context.payload.workflow_run.html_url,
60+
});

0 commit comments

Comments
 (0)