Skip to content

Commit 4fed5ce

Browse files
julian-rischclaude
andauthored
ci: fix CLA gate reviewer restore and exclude team members from gates (#12088)
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent 04fe770 commit 4fed5ce

2 files changed

Lines changed: 65 additions & 11 deletions

File tree

.github/workflows/cla_draft_gate.yml

Lines changed: 45 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,13 @@ name: CLA draft gate
66
# requested reviewers (remembering them in a hidden comment marker), label it
77
# `cla-pending`, and explain why in a comment.
88
# - When the CLA is signed (the `license/cla` commit status turns green), mark
9-
# the PR ready for review again and re-request the same reviewers. The
10-
# scheduled sweep doubles as a backstop in case a status event is missed.
9+
# the PR ready for review again and re-request the same reviewers. This also
10+
# covers the case where the contributor marks the PR ready for review
11+
# themselves after signing. The scheduled sweep doubles as a backstop in case
12+
# a status event is missed.
13+
# - Team members are never gated. author_association is unreliable for this
14+
# (private org members appear as CONTRIBUTOR/NONE), so effective repository
15+
# permission is used instead.
1116
# - While a PR stays gated, escalate: remind the contributor 5 days after the
1217
# PR was opened, post a final warning after 10 days, and close the PR after
1318
# 14 days. Each step waits for the previous comment to be a few days old, so
@@ -58,6 +63,21 @@ jobs:
5863
const FIRST_TIMER = new Set(["FIRST_TIME_CONTRIBUTOR", "FIRST_TIMER", "NONE"]);
5964
const { owner, repo } = context.repo;
6065
66+
// Team members must never be gated. author_association is unreliable
67+
// for this: PRIVATE org members show up as CONTRIBUTOR/NONE in webhook
68+
// and API payloads, so check the effective repository permission
69+
// instead (write/admin => part of the team).
70+
async function isTeamMember(login) {
71+
try {
72+
const { data } = await github.rest.repos.getCollaboratorPermissionLevel({
73+
owner, repo, username: login,
74+
});
75+
return ["admin", "write"].includes(data.permission);
76+
} catch {
77+
return false;
78+
}
79+
}
80+
6181
async function claSigned(sha) {
6282
const { data } = await github.rest.repos.getCombinedStatusForRef({
6383
owner, repo, ref: sha, per_page: 100,
@@ -149,10 +169,14 @@ jobs:
149169
const comment = await findMarkerComment(pr.number);
150170
const meta = comment ? parseMarker(comment.body) : { reviewers: [], team_reviewers: [] };
151171
152-
await github.graphql(
153-
"mutation($id: ID!) { markPullRequestReadyForReview(input: { pullRequestId: $id }) { pullRequest { isDraft } } }",
154-
{ id: pr.node_id },
155-
);
172+
// The contributor may have marked the PR ready for review themselves
173+
// after signing; only run the mutation when it's still a draft.
174+
if (pr.draft) {
175+
await github.graphql(
176+
"mutation($id: ID!) { markPullRequestReadyForReview(input: { pullRequestId: $id }) { pullRequest { isDraft } } }",
177+
{ id: pr.node_id },
178+
);
179+
}
156180
157181
// Prefer the individual reviewers we removed; re-requesting the team
158182
// instead would make round-robin pick somebody new.
@@ -253,7 +277,7 @@ jobs:
253277
owner, repo, commit_sha: context.payload.sha,
254278
});
255279
for (const prLite of prs) {
256-
if (prLite.state !== "open" || !prLite.draft) continue;
280+
if (prLite.state !== "open") continue;
257281
if (!prLite.labels.some((l) => l.name === LABEL)) continue;
258282
const { data: pr } = await github.rest.pulls.get({ owner, repo, pull_number: prLite.number });
259283
await restore(pr);
@@ -269,18 +293,30 @@ jobs:
269293
try {
270294
if (pr.user?.type === "Bot") continue;
271295
const labels = pr.labels.map((l) => l.name);
272-
if (pr.draft) {
273-
if (!labels.includes(LABEL) || labels.includes(EXEMPT_LABEL)) continue;
296+
if (labels.includes(EXEMPT_LABEL)) continue;
297+
298+
// Already gated: drive the PR to the right state. Handle the
299+
// non-draft case too, since the contributor can mark a gated PR
300+
// ready for review themselves.
301+
if (labels.includes(LABEL)) {
274302
if (await claSigned(pr.head.sha)) {
275303
await restore(pr);
304+
} else if (pr.draft) {
305+
await escalate(pr);
276306
} else {
307+
// Readied without signing: put it back in draft, then escalate.
308+
await gate(pr);
277309
await escalate(pr);
278310
}
279311
continue;
280312
}
313+
314+
// Not yet gated: only gate fresh, external first-timer PRs.
315+
if (pr.draft) continue;
281316
if (!FIRST_TIMER.has(pr.author_association)) continue;
282317
if (Date.now() - new Date(pr.created_at).getTime() < GRACE_MS) continue;
283318
if (await claSigned(pr.head.sha)) continue;
319+
if (await isTeamMember(pr.user.login)) continue;
284320
await gate(pr);
285321
} catch (error) {
286322
core.warning(`PR #${pr.number}: ${error.message}`);

.github/workflows/pr_flood_guard.yml

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,10 @@ on:
1414

1515
permissions:
1616
contents: read
17-
issues: write
18-
pull-requests: read
17+
# The warning is posted on the PR itself, which counts as a pull request (not
18+
# an issue) for token scopes, so this needs pull-requests: write. issues: write
19+
# alone yields "Resource not accessible by integration" on the comment.
20+
pull-requests: write
1921

2022
jobs:
2123
guard:
@@ -28,8 +30,24 @@ jobs:
2830
const { owner, repo } = context.repo;
2931
const pr = context.payload.pull_request;
3032
33+
// Team members are exempt. author_association is unreliable for this
34+
// (private org members appear as CONTRIBUTOR/NONE in the payload), so
35+
// fall back to the effective repository permission. The association
36+
// check still short-circuits the common case without an API call.
37+
async function isTeamMember(login) {
38+
try {
39+
const { data } = await github.rest.repos.getCollaboratorPermissionLevel({
40+
owner, repo, username: login,
41+
});
42+
return ["admin", "write"].includes(data.permission);
43+
} catch {
44+
return false;
45+
}
46+
}
47+
3148
if (pr.user?.type === "Bot") return;
3249
if (["MEMBER", "OWNER", "COLLABORATOR"].includes(pr.author_association)) return;
50+
if (await isTeamMember(pr.user.login)) return;
3351
3452
const warnings = [];
3553

0 commit comments

Comments
 (0)