Skip to content

Commit 617ff95

Browse files
edelaunaedelauna
committed
feat(label-pr-review-state): updating logic for awaiting-review
1 parent 515437b commit 617ff95

1 file changed

Lines changed: 138 additions & 41 deletions

File tree

‎.github/workflows/label-pr-review-state.yml‎

Lines changed: 138 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,16 @@ name: Label PR review state
22

33
on:
44
schedule:
5-
- cron: '0 * * * *' # hourly
5+
- cron: '0 * * * *' # hourly fallback
66
workflow_dispatch:
7+
pull_request:
8+
types: [opened, reopened, ready_for_review, synchronize, review_requested]
9+
pull_request_review:
10+
types: [submitted, dismissed]
711

812
permissions:
913
pull-requests: write
14+
checks: read
1015

1116
concurrency:
1217
group: label-pr-review-state
@@ -22,70 +27,162 @@ jobs:
2227
script: |
2328
const { owner, repo } = context.repo;
2429
const stateLabels = ['awaiting-author', 'awaiting-review'];
25-
const failures = [];
2630
27-
const prs = await github.paginate(github.rest.pulls.list, {
28-
owner, repo, state: 'open', per_page: 100,
29-
});
31+
// When triggered by a single PR event, only reconcile that PR.
32+
// The hourly schedule and workflow_dispatch reconcile all open PRs.
33+
let prs;
34+
const prNumber = context.payload.pull_request?.number;
35+
if (prNumber) {
36+
const { data: pr } = await github.rest.pulls.get({
37+
owner, repo, pull_number: prNumber,
38+
});
39+
prs = [pr];
40+
} else {
41+
prs = await github.paginate(github.rest.pulls.list, {
42+
owner, repo, state: 'open', per_page: 100,
43+
});
44+
}
45+
46+
// Strips stateLabels from a PR, optionally keeping one.
47+
// Also removes stale-awaiting-author when not keeping awaiting-author.
48+
async function reconcileLabels(pr, desiredLabel) {
49+
const currentLabels = new Set(pr.labels.map(l => l.name));
50+
for (const label of stateLabels) {
51+
if (label !== desiredLabel && currentLabels.has(label)) {
52+
try {
53+
await github.rest.issues.removeLabel({
54+
owner, repo, issue_number: pr.number, name: label,
55+
});
56+
} catch (err) {
57+
if (err.status !== 404) throw err; // 404 = already gone, benign
58+
}
59+
}
60+
}
61+
if (desiredLabel && !currentLabels.has(desiredLabel)) {
62+
await github.rest.issues.addLabels({
63+
owner, repo, issue_number: pr.number, labels: [desiredLabel],
64+
});
65+
}
66+
if (desiredLabel !== 'awaiting-author' && currentLabels.has('stale-awaiting-author')) {
67+
try {
68+
await github.rest.issues.removeLabel({
69+
owner, repo, issue_number: pr.number, name: 'stale-awaiting-author',
70+
});
71+
} catch (err) {
72+
if (err.status !== 404) throw err;
73+
}
74+
}
75+
}
76+
77+
const failures = [];
3078
3179
for (const pr of prs) {
3280
try {
81+
// Draft PRs never get a state label.
82+
if (pr.draft) {
83+
core.info(`PR #${pr.number}: draft — stripping state labels`);
84+
await reconcileLabels(pr, null);
85+
continue;
86+
}
87+
88+
// Check aggregate CI status for the PR's head commit.
89+
// We combine check runs (Actions) and commit statuses (external CIs).
90+
const [checkRuns, commitStatusRes] = await Promise.all([
91+
github.paginate(github.rest.checks.listForRef, {
92+
owner, repo, ref: pr.head.sha, per_page: 100,
93+
}),
94+
github.rest.repos.getCombinedStatusForRef({
95+
owner, repo, ref: pr.head.sha,
96+
}),
97+
]);
98+
99+
// Exclude this workflow's own check run to avoid self-referential loops.
100+
const relevantRuns = checkRuns.filter(
101+
run => run.name !== 'Reconcile PR review state labels',
102+
);
103+
104+
core.debug(`PR #${pr.number}: ${relevantRuns.length} check run(s), commit status=${commitStatusRes.data.state}`);
105+
for (const run of relevantRuns) {
106+
core.debug(` check: "${run.name}" status=${run.status} conclusion=${run.conclusion}`);
107+
}
108+
109+
const ciPending = relevantRuns.some(
110+
run => run.status === 'queued' || run.status === 'in_progress',
111+
) || commitStatusRes.data.state === 'pending';
112+
113+
const ciFailed = !ciPending && (
114+
relevantRuns.some(
115+
run => run.status === 'completed' &&
116+
run.conclusion !== 'success' &&
117+
run.conclusion !== 'skipped' &&
118+
run.conclusion !== 'neutral',
119+
) || commitStatusRes.data.state === 'failure' ||
120+
commitStatusRes.data.state === 'error'
121+
);
122+
123+
// While CI is running or has failed, remove state labels and move on.
124+
// CI failure is its own signal; the label would add noise, not clarity.
125+
if (ciPending || ciFailed) {
126+
core.info(`PR #${pr.number}: CI ${ciPending ? 'pending' : 'failed'} — stripping state labels`);
127+
await reconcileLabels(pr, null);
128+
continue;
129+
}
130+
131+
// CI is passing. Now determine review state.
33132
const reviews = await github.paginate(github.rest.pulls.listReviews, {
34133
owner, repo, pull_number: pr.number, per_page: 100,
35134
});
36135
37-
// Reviews are returned chronologically, so later entries replace
38-
// each reviewer's earlier decision.
136+
// Reduce to each reviewer's latest meaningful state.
137+
// Reviews are returned oldest-first, so last-write-wins yields the latest state.
138+
// COMMENTED and DISMISSED are treated as neutral — they do not
139+
// block the PR or indicate the author needs to act.
39140
const latest = new Map();
40141
for (const r of reviews) {
41-
if (r.state !== 'COMMENTED') {
142+
if (r.state !== 'COMMENTED' && r.state !== 'DISMISSED') {
42143
latest.set(r.user.login, r);
43144
}
44145
}
45146
46-
const changeRequestReviewers = [...latest.entries()]
47-
.filter(([, review]) => review.state === 'CHANGES_REQUESTED')
48-
.map(([login]) => login);
49147
const requestedReviewers = new Set(
50-
pr.requested_reviewers.map(reviewer => reviewer.login),
148+
pr.requested_reviewers.map(r => r.login),
51149
);
52150
53-
let desiredLabel = null;
54-
if (changeRequestReviewers.length > 0) {
55-
desiredLabel = changeRequestReviewers.every(
56-
reviewer => requestedReviewers.has(reviewer),
57-
)
151+
const changeRequesters = [...latest.entries()]
152+
.filter(([, r]) => r.state === 'CHANGES_REQUESTED')
153+
.map(([login]) => login);
154+
155+
let desiredLabel;
156+
if (changeRequesters.length > 0) {
157+
// If every change-requester has been re-requested for review,
158+
// the author has addressed feedback and re-opened it for review.
159+
desiredLabel = changeRequesters.every(login => requestedReviewers.has(login))
58160
? 'awaiting-review'
59161
: 'awaiting-author';
60-
}
162+
} else {
163+
// No outstanding change requests: awaiting first review, or all approved.
164+
// awaiting-review if: there are pending requested reviewers, or nobody
165+
// has given a meaningful review yet. null (approved) if everyone approved.
166+
const allApproved = latest.size > 0 &&
167+
[...latest.values()].every(r => r.state === 'APPROVED') &&
168+
requestedReviewers.size === 0;
61169
62-
const currentLabels = new Set(pr.labels.map(label => label.name));
63-
for (const label of stateLabels) {
64-
if (label !== desiredLabel && currentLabels.has(label)) {
65-
await github.rest.issues.removeLabel({
66-
owner, repo, issue_number: pr.number, name: label,
67-
});
68-
}
170+
desiredLabel = allApproved ? null : 'awaiting-review';
69171
}
70172
71-
if (desiredLabel && !currentLabels.has(desiredLabel)) {
72-
await github.rest.issues.addLabels({
73-
owner, repo, issue_number: pr.number, labels: [desiredLabel],
74-
});
75-
}
173+
core.info(
174+
`PR #${pr.number}: CI passing, reviews=${latest.size}, ` +
175+
`changeRequesters=[${changeRequesters.join(',')}], ` +
176+
`requestedReviewers=[${[...requestedReviewers].join(',')}] → ${desiredLabel ?? '(none)'}`
177+
);
76178
77-
if (
78-
desiredLabel !== 'awaiting-author' &&
79-
currentLabels.has('stale-awaiting-author')
80-
) {
81-
await github.rest.issues.removeLabel({
82-
owner, repo, issue_number: pr.number,
83-
name: 'stale-awaiting-author',
84-
});
85-
}
179+
await reconcileLabels(pr, desiredLabel);
86180
} catch (error) {
87-
failures.push(`#${pr.number}: ${error.message}`);
88-
core.error(`Failed to reconcile PR #${pr.number}: ${error.message}`);
181+
const detail = error.status
182+
? `${error.message} (HTTP ${error.status}${error.response?.data?.message ? `: ${error.response.data.message}` : ''})`
183+
: error.message;
184+
failures.push(`#${pr.number}: ${detail}`);
185+
core.error(`Failed to reconcile PR #${pr.number}: ${detail}`);
89186
}
90187
}
91188

0 commit comments

Comments
 (0)