chore(deps): address Copilot review round 5 on #2243

Both findings were "previously missed" suppressed ones; the two against
scripts/dependency-refresh.mjs and its workflow are relayed to #2239.

- Serialize the sweep with a fixed concurrency group and
  cancel-in-progress: false. The marker check is a read-before-write and
  a workflow_dispatch can land on top of the scheduled run, so two runs
  could both see no open issue and both file one — the duplicate the
  whole idempotency design exists to prevent. The queued run must wait
  and re-read, never be cancelled.
- Pick the milestone in JS, not in jq. jq sorts null before every
  string, so sort_by(.due_on) | .[0] returns an UNDATED open milestone
  in preference to every dated one. An undated bucket has no due date
  and so cannot be the nearest; pickMilestone drops it, and files the
  issue unmilestoned if nothing dated is open.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013H6Cew3KB3jVmQ5x4Dq1sq
Signed-off-by: cliffhall <cliff@futurescale.com>
This commit is contained in:
cliffhall 2026-09-04 01:15:28 -04:00 • committed by GitHub
commit a17eace0bb
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 73 additions and 8 deletions

View file

@ -55,6 +55,16 @@ on:
- cron: "17 6 * * *" # 06:17 UTC daily; alerts are not minute-sensitive
workflow_dispatch:
# The marker check is a read-before-write, not an atomic one, and nothing stops
# a `workflow_dispatch` from landing on top of the scheduled run. Two overlapping
# runs would both see no open issue and both file one, which is the exact
# duplicate this sweep's whole idempotency design exists to prevent (Copilot).
# `cancel-in-progress: false` because the queued run must WAIT and then re-read
# the state the first run wrote — cancelling it would drop a sweep instead.
concurrency:
group: dependabot-alert-sweep
cancel-in-progress: false
permissions:
contents: read
issues: write

View file

@ -308,6 +308,27 @@ export function groupAlerts(alerts) {
);
}
/**
* The milestone a new issue takes: the open one with the NEAREST due date.
*
* ⚠️ Selected here rather than in a `jq` expression because jq sorts `null`
* BEFORE every string, so a `sort_by(.due_on) | .[0]` over the raw list hands
* back an undated milestone in preference to every dated one (Copilot). An
* undated bucket has no due date and so cannot be the nearest; it is dropped
* rather than sorted last, and if nothing dated is open the issue is filed
* unmilestoned and triage places it.
*
* @param {Array<{title: string, state?: string, due_on?: string | null}>} milestones
* @returns {string | null}
*/
export function pickMilestone(milestones) {
const dated = (milestones ?? []).filter(
(m) => (m.state ?? "open") === "open" && m.due_on,
);
if (dated.length === 0) return null;
return dated.sort((a, b) => a.due_on.localeCompare(b.due_on))[0].title;
}
/**
* Narrow a group to the advisories that actually apply to what is installed.
*
@ -683,16 +704,11 @@ function announcedAdvisories(repo, number, spawn) {
}
function currentMilestone(repo, spawn) {
const result = gh(spawn, [
"api",
`repos/${repo}/milestones`,
"--jq",
'map(select(.state=="open")) | sort_by(.due_on) | .[0].title // empty',
]);
const result = gh(spawn, ["api", `repos/${repo}/milestones?state=open`]);
if (result.status !== 0) {
throw new Error(`milestone lookup failed: ${(result.stderr ?? "").trim()}`);
}
return result.stdout.trim() || null;
return pickMilestone(JSON.parse(result.stdout || "[]"));
}
/**

View file

@ -16,6 +16,7 @@ import { dirname, join } from "node:path";
import { tmpdir } from "node:os";
import {
buildCommentMarker,
pickMilestone,
narrowToApplicable,
lockfileEntries,
remediation,
@ -334,6 +335,31 @@ test("buildIssueBody escapes a disjoint range so the table survives it", () => {
assert.ok(row.includes(String.raw`\|\|`), "the range's own pipes survive");
});
test("pickMilestone takes the nearest due date, never an undated bucket", () => {
// jq sorts null before every string, so `sort_by(.due_on) | .[0]` over this
// list would return "Backlog" — an open bucket with no release date at all.
const milestones = [
{ title: "Backlog", state: "open", due_on: null },
{ title: "v2.7.0", state: "open", due_on: "2026-09-16T00:00:00Z" },
{ title: "v2.6.0", state: "open", due_on: "2026-09-09T00:00:00Z" },
];
assert.equal(pickMilestone(milestones), "v2.6.0");
});
test("pickMilestone ignores closed milestones and empty input", () => {
assert.equal(
pickMilestone([
{ title: "v2.5.0", state: "closed", due_on: "2026-01-01T00:00:00Z" },
]),
null,
);
// Nothing dated and open means no bucket to take: filed unmilestoned, and
// the board write is skipped so triage places it.
assert.equal(pickMilestone([{ title: "Backlog", due_on: null }]), null);
assert.equal(pickMilestone([]), null);
assert.equal(pickMilestone(undefined), null);
});
test("buildIssueTitle names the bump and pluralizes the advisory count", () => {
const [many] = groupAlerts([
alert({ ghsa: "GHSA-a" }),
@ -530,7 +556,20 @@ function fakeSpawn({
// `--slurp` yields one array PER PAGE; main() must flatten them.
return ok(JSON.stringify(alertPages));
}
if (joined.includes("milestones")) return ok(milestone);
if (joined.includes("milestones"))
return ok(
JSON.stringify(
milestone
? [
{
title: milestone,
state: "open",
due_on: "2026-09-09T00:00:00Z",
},
]
: [],
),
);
// `--slurp`, so one array per page — `issues` may be a flat list or pages.
if (joined.includes("/issues?"))
return ok(JSON.stringify(Array.isArray(issues[0]) ? issues : [issues]));