Repository navigation
ci: draft inactive PRs awaiting author action - #26715
Conversation
jayzhan211
left a comment
There was a problem hiding this comment.
The drafting rule looks right to me. The main problem is that conflicts are mostly missed on a cold run; details inline.
| per_page: 100, | ||
| }); | ||
| const results = []; | ||
| for (const listed of prs) { |
There was a problem hiding this comment.
GitHub computes mergeable lazily, so the first query for a PR usually returns UNKNOWN, which this treats as "no conflict". I dry-ran the script twice against the repo: the first run found 9 conflicting PRs (27 eligible in total) and the second found 60 (69 in total). Since the cron usually runs after main has moved, most conflicts would be missed, and which PRs get drafted would depend on whether something else queried them first. Could we warm the state up with one pass and a short wait before deciding? With this change the dry run gives the same 69 PRs from a cold start:
- const results = [];
- for (const listed of prs) {
- if (listed.draft || !(Date.parse(listed.updated_at) <= cutoff)) continue;
+ const candidates = prs.filter(
+ (listed) => !listed.draft && Date.parse(listed.updated_at) <= cutoff
+ );
+ // GitHub computes `mergeable` lazily: the first query starts the job and
+ // returns UNKNOWN, so query every candidate once and wait before deciding.
+ for (const listed of candidates) {
+ await loadPullRequest(github, repo, listed.number);
+ }
+ await setTimeout(30_000);
+ const results = [];
+ for (const listed of candidates) {
let pr = await loadPullRequest(github, repo, listed.number);| ) | ||
| reasons.push("failing CI checks"); | ||
|
|
||
| const forcePush = pr.timelineItems.nodes[0]?.createdAt; |
There was a problem hiding this comment.
nit: review.commit?.oid === pr.headRefOid already means nothing was pushed after the review, so the force-push and pushedDate checks aren't needed. pushedDate is also deprecated (its removal date was 2023-07-01) and returns null for every PR I checked, and if GitHub removes it the whole query will fail. Could we drop it together with timelineItems and submittedAt? The dry run gives the same results without them:
- const forcePush = pr.timelineItems.nodes[0]?.createdAt;
- const pushedDate = pr.commits.nodes[0]?.commit.pushedDate;
- // pushedDate can be null; also check the reviewed commit and last force push.
if (
pr.reviews.some(
(review) =>
review.state === "CHANGES_REQUESTED" &&
- review.commit?.oid === pr.headRefOid &&
- (!forcePush ||
- Date.parse(review.submittedAt) > Date.parse(forcePush)) &&
- (!pushedDate || Date.parse(review.submittedAt) > Date.parse(pushedDate))
+ review.commit?.oid === pr.headRefOid
)
)d3644c5 to
ef5709f
Compare
|
Thanks @jayzhan211 Pushed the changes |
|
Thank you for this |
Which issue does this PR close?
Related to #26684.
Rationale for this change
Keep the active review queue focused on PRs that can progress.
What changes are included in this PR?
Automatically draft PRs inactive for 14 days with merge conflicts, failing CI, or changes requested on the latest push. Includes a brief explanation and a manual dry-run mode.
What is the testing strategy for this PR?
Validated with workflow lint and a live dry run (69 eligible PRs); no automated tests added.
Are there any user-facing changes?
Authors can mark their PR ready for review again. The draft notification restarts the existing 60-day stale timer.