Skip to content

Commit d7c3d5b

Browse files
committed
Improve project picking logic so it handles Milestone and Release Candidates when triaging dependabot PRs
1 parent 0f32578 commit d7c3d5b

5 files changed

Lines changed: 115 additions & 24 deletions

File tree

.github/actions/dependabot-scan/action.yml

Lines changed: 28 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -70,8 +70,15 @@ runs:
7070
run: |
7171
node - << 'JSEOF'
7272
const fs = require('fs');
73+
const path = require('path');
7374
const { execFileSync } = require('child_process');
7475
76+
// The milestone/board pre-release rule, shared with dependabot-triage.yml.
77+
// GITHUB_ACTION_PATH is this action's own directory, so the relative path holds
78+
// wherever the caller checked the repository out.
79+
const { best } = require(
80+
path.join(process.env.GITHUB_ACTION_PATH, '..', '..', 'scripts', 'prerelease-rank.js'));
81+
7582
const REPO = process.env.REPO;
7683
const PROJECT = process.env.PROJECT;
7784
const TYPE = process.env.TYPE;
@@ -341,23 +348,33 @@ runs:
341348
return version;
342349
};
343350
344-
// Existing milestone titles, fetched once and reused for every PR in this repo.
351+
// Open milestone titles, fetched once and reused for every PR in this repo.
345352
let milestoneTitles = null;
346-
const milestoneExists = title => {
353+
const openMilestones = () => {
347354
if (milestoneTitles === null) {
348355
// --paginate, with --slurp instead of --jq because gh refuses both together:
349356
// a repository past a hundred open milestones would otherwise lose the newest
350357
// ones, and those are the only ones this ever looks for.
351358
const r = gh(['api', '--paginate', '--slurp',
352359
`repos/${REPO}/milestones?state=open&per_page=100`]);
353360
milestoneTitles = r.ok
354-
? new Set(parseJson(r.out, []).flat().map(m => m.title))
355-
: new Set();
361+
? parseJson(r.out, []).flat().map(m => m.title)
362+
: [];
356363
if (!r.ok) warnings.push(`could not list milestones: ${r.err}`);
357364
}
358-
return milestoneTitles.has(title);
365+
return milestoneTitles;
359366
};
360367
368+
// The branch version gives the train's GA number (5.1.0), but a train ships
369+
// milestones first, so the milestone that actually exists is 5.1.0-M1, then -M2,
370+
// then -RC1. Matching the GA title literally means every PR against a freshly
371+
// branched train is reported as "milestone 5.1.0 does not exist" until GA - which
372+
// is most of the train's life, and is exactly what happened when 2026.0.0 opened.
373+
//
374+
// Only open milestones are considered, so a shipped milestone is never chosen; of
375+
// what remains the furthest along wins, which is the one being worked toward.
376+
const resolveMilestone = base => best(openMilestones(), base);
377+
361378
const scanned = prs.map(pr => {
362379
const checks = (pr.statusCheckRollup || []).map(normalizeCheck);
363380
const failing = checks.filter(c => FAILING.has(c.state));
@@ -398,11 +415,15 @@ runs:
398415
milestoneState = 'unresolved';
399416
projectState = 'unresolved';
400417
} else {
401-
expectedMilestone = version.replace(/-SNAPSHOT$/, '');
418+
// Fall back to the bare GA title when nothing matches, so a 'missing'
419+
// verdict still names the version it was looking for.
420+
const milestoneBase = version.replace(/-SNAPSHOT$/, '');
421+
const resolved = resolveMilestone(milestoneBase);
422+
expectedMilestone = resolved || milestoneBase;
402423
const current = pr.milestone?.title || null;
403424
if (current === expectedMilestone) milestoneState = 'set';
404425
else if (current) milestoneState = 'mismatch';
405-
else if (!milestoneExists(expectedMilestone)) milestoneState = 'missing';
426+
else if (!resolved) milestoneState = 'missing';
406427
else milestoneState = 'unset';
407428
408429
// Boards are OSS-only by design; commercial PRs get a milestone alone.

.github/scripts/prerelease-rank.js

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
'use strict';
2+
3+
// A release train ships milestones and release candidates before GA, and the GitHub
4+
// artifacts named after it follow suit. The 2026.0.0 train's project board and its
5+
// per-repo milestones are titled 2026.0.0-M1, then -M2, then -RC1, and only finally
6+
// 2026.0.0 itself.
7+
//
8+
// The releaser config only ever names the GA version, so anything matching a train to a
9+
// board or a milestone has to treat that version as a base to match against rather than a
10+
// literal title - otherwise every PR against a freshly branched train goes unfiled for
11+
// most of the train's life.
12+
//
13+
// Shared by the dependabot-scan action (milestones) and dependabot-triage.yml (boards).
14+
// It lives here rather than inline in each because those two started as copies of one
15+
// script, drifted, and a fix to one silently left the other broken for a week.
16+
17+
// Anchored at both ends: 2026.0.0-SNAPSHOT, 2026.0.01 and 2026.0.0-M1-extra are not
18+
// pre-releases of 2026.0.0.
19+
const PRERELEASE = /^-(M|RC)(\d+)$/;
20+
21+
// Is `title` the base itself or one of its pre-releases, and how far along? null when
22+
// unrelated. Ordered later-wins: GA > RC<n> > M<n>, higher <n> beating lower.
23+
const rank = (title, base) => {
24+
if (title === base) return [2, 0];
25+
if (!title.startsWith(base)) return null;
26+
const m = title.slice(base.length).match(PRERELEASE);
27+
if (!m) return null;
28+
return [m[1] === 'RC' ? 1 : 0, Number(m[2])];
29+
};
30+
31+
// Comparator putting the furthest-along entry first. Entries need a `rank` property.
32+
// Numeric throughout, so M10 sorts above M9 rather than below it as a string would.
33+
const byRankDesc = (a, b) => (b.rank[0] - a.rank[0]) || (b.rank[1] - a.rank[1]);
34+
35+
// The furthest-along title in `titles` that belongs to `base`, or null when none does.
36+
const best = (titles, base) => {
37+
const candidates = [];
38+
for (const title of titles) {
39+
const r = rank(title, base);
40+
if (r) candidates.push({ title, rank: r });
41+
}
42+
if (!candidates.length) return null;
43+
candidates.sort(byRankDesc);
44+
return candidates[0].title;
45+
};
46+
47+
module.exports = { rank, byRankDesc, best };

.github/workflows/README-dependabot-report.md

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -100,10 +100,20 @@ workflow has something to act on and the report can flag gaps. PRs on
100100
they belong to no release train and need neither a milestone nor a board:
101101

102102
- **Milestone** — the base branch's project version with `-SNAPSHOT` stripped (the same
103-
string [`post-release.yml`](post-release.yml) creates milestones with). Reported as
104-
`set`, `unset` (exists but not applied), `mismatch` (a different one is applied — never
103+
string [`post-release.yml`](post-release.yml) creates milestones with), resolved against
104+
the repository's **open** milestones by the pre-release rule below. Reported as `set`,
105+
`unset` (exists but not applied), `mismatch` (a different one is applied — never
105106
overwritten, only reported), or `missing` (no such milestone exists, which is the warning
106107
the design calls for).
108+
109+
A train ships milestones before GA, so the milestone that exists while `main` is at
110+
`5.1.0-SNAPSHOT` is `5.1.0-M1`, not `5.1.0`. Matching `5.1.0` literally reported *every*
111+
PR on a freshly branched train as `missing`; the base is now matched against `5.1.0-M1`,
112+
`-M2`, `-RC1` and finally `5.1.0` itself, furthest-along first. Only open milestones are
113+
candidates, so a shipped one is never chosen. The shared rule lives in
114+
[`.github/scripts/prerelease-rank.js`](../scripts/prerelease-rank.js) — triage applies
115+
the same rule to project boards, see
116+
[Picking the board](README-dependabot-triage.md#picking-the-board).
107117
- **Project** — OSS only, resolved as below. Commercial PRs get a milestone and no board.
108118

109119
### Project resolution

.github/workflows/README-dependabot-triage.md

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,12 @@ So the resolved train is treated as a **base**, and any board titled `<base>`,
6464
`<base>-M<n>` or `<base>-RC<n>` is a candidate. The match is anchored at both ends —
6565
`2026.0.0-SNAPSHOT`, `2026.0.01` and `2026.0.0-M1-extra` are not candidates for `2026.0.0`.
6666

67+
The rule lives in [`.github/scripts/prerelease-rank.js`](../scripts/prerelease-rank.js)
68+
rather than inline here, because the scan action needs the identical rule for **milestones**
69+
`5.1.0-M1` exists while `5.1.0` does not, in the same way and for the same reason. Two
70+
inline copies of one script is what left triage broken for a week when only the report's
71+
copy was fixed.
72+
6773
Candidates are ranked:
6874

6975
1. **Open before closed.** A milestone's board is closed once it ships, and filing into a
@@ -123,7 +129,9 @@ this one not in the list?" rather than leaving it out:
123129
| `conflicts with the base branch` | Needs a rebase — see [Rebase idempotency](#rebase-idempotency) |
124130
| `all checks pass but GitHub reports BLOCKED` | Branch protection, usually a required review |
125131
| `GitHub has not resolved mergeability yet` | Transient; resolves on a later run |
126-
| `not filed yet - waiting on milestone and project` | Triage has not finished filing it |
132+
| `not filed yet - waiting on milestone 5.0.4, expected 5.1.0-M1 …` | The PR carries the **previous** train's milestone. Triage never overwrites a mismatched milestone, so this needs a human — the reason names both so the row does not read as "has no milestone" |
133+
| `not filed yet - waiting on milestone 5.1.0-M1 not set` | No milestone yet; triage sets it on this run or the next |
134+
| `not filed yet - waiting on project` | Triage has not finished filing it |
127135
| `no board titled 2026.0.0 or 2026.0.0-M*/-RC*` | The train has no board at all yet |
128136
| `only board for 2026.0.0 is 2026.0.0-M2, which is closed` | The next milestone's board has not been opened — see [Picking the board](#picking-the-board) |
129137
| `maven is never auto-merged` | Policy — **dry runs only** |

.github/workflows/dependabot-triage.yml

Lines changed: 19 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -204,8 +204,15 @@ jobs:
204204
run: |
205205
node - << 'JSEOF'
206206
const fs = require('fs');
207+
const path = require('path');
207208
const { execFileSync } = require('child_process');
208209
210+
// The milestone/board pre-release rule, shared with the dependabot-scan action,
211+
// which applies it to milestones. This job checks the repository out, so the
212+
// module sits under the workspace.
213+
const prerelease = require(path.join(process.env.GITHUB_WORKSPACE,
214+
'.github', 'scripts', 'prerelease-rank.js'));
215+
209216
const DRY = process.env.DRY_RUN !== 'false';
210217
const CLOSE_UNMAINTAINED = process.env.CLOSE_UNMAINTAINED === 'true';
211218
const MERGE_GREEN = process.env.MERGE_GREEN === 'true';
@@ -307,25 +314,16 @@ jobs:
307314
// adding to a closed board is worse than adding to none. Within that, later
308315
// wins - GA over RC over M, and a higher number over a lower one - so the board
309316
// picked is the one the train is currently working toward.
310-
const PRERELEASE = /^-(M|RC)(\d+)$/;
311-
const boardRank = (title, base) => {
312-
if (title === base) return [2, 0];
313-
const m = title.startsWith(base) && title.slice(base.length).match(PRERELEASE);
314-
if (!m) return null;
315-
return [m[1] === 'RC' ? 1 : 0, Number(m[2])];
316-
};
317-
318317
const resolveBoard = base => {
319318
const candidates = [];
320319
for (const b of allBoards()) {
321-
const rank = boardRank(b.title, base);
320+
const rank = prerelease.rank(b.title, base);
322321
if (rank) candidates.push({ ...b, rank });
323322
}
324323
if (!candidates.length) return null;
325-
candidates.sort((x, y) =>
326-
(x.closed - y.closed) ||
327-
(y.rank[0] - x.rank[0]) ||
328-
(y.rank[1] - x.rank[1]));
324+
// Open before closed, then furthest along. The scan applies the same rule to
325+
// milestones, minus the closed test - it only ever lists open ones.
326+
candidates.sort((x, y) => (x.closed - y.closed) || prerelease.byRankDesc(x, y));
329327
return candidates[0];
330328
};
331329
@@ -552,7 +550,14 @@ jobs:
552550
if (pr.isReleaseBranch) {
553551
const needsProject = pr.projectState !== 'n/a';
554552
if (!milestoneOk || (needsProject && !projectOk)) {
555-
const missing = [!milestoneOk && 'milestone',
553+
// Say which milestone is in the way, not just that one is. "waiting on
554+
// milestone" reads as "has none" when the usual cause is the opposite - a
555+
// PR carrying the previous train's milestone, which triage will not
556+
// overwrite, so the row otherwise contradicts the visible PR.
557+
const milestoneReason = pr.currentMilestone
558+
? `milestone is ${pr.currentMilestone}, expected ${pr.expectedMilestone}`
559+
: `milestone ${pr.expectedMilestone} not set`;
560+
const missing = [!milestoneOk && milestoneReason,
556561
needsProject && !projectOk && 'project'].filter(Boolean).join(' and ');
557562
record(pr, 'merge', 'skipped', `not filed yet - waiting on ${missing}`);
558563
continue;

0 commit comments

Comments
 (0)