chore(deps): paginate the release list so the highest version is actual
Addresses Copilot review round 7 on #2239. The lookup read one page of 100 and the doc comment justified it with an assumption rather than a guarantee: "the greatest version is within it for any real action". The list is ordered by release DATE, not version, so an action that keeps cutting maintenance releases on lower majors pushes the genuine maximum off page 1. Ranking what remained would report a lower major as highest — the exact false-current result that dropping `releases/latest` in round 3 was meant to prevent, arrived at by a different route. Now passes `--paginate`. Cost is unchanged for every action this repo uses (57 and 64 releases are the largest, so still one request each); it only costs more where correctness actually required it. The command-shape test now requires `--paginate` as well as the list endpoint. It has to: pagination is invisible in the result — one page and every page return an identical-looking tag list until the day they do not — so nothing about the returned value can detect its absence. Verified by mutation: removing `--paginate` fails exactly one test. Adding the flag also moved the URL out of args[1], which the test's own filter had hardcoded — it caught that itself. Both the filter and fakeSpawn's matcher are now argument-position agnostic. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NGtzPg3UxMLszysQXXfqax Signed-off-by: cliffhall <cliff@futurescale.com>
This commit is contained in:
parent
8414685ad9
commit
0cb12a19ca
2 changed files with 23 additions and 7 deletions
|
|
@ -301,9 +301,15 @@ function comparePadded(a, b) {
|
|||
*
|
||||
* Reads the release *list* rather than `releases/latest`, for the reason on
|
||||
* `highestVersionTag`. Drafts and prereleases are excluded — neither is
|
||||
* something a workflow should be told to move to. One page of 100 is taken
|
||||
* rather than paginating every release an action has ever cut: the list comes
|
||||
* back newest-first, so the greatest version is within it for any real action.
|
||||
* something a workflow should be told to move to.
|
||||
*
|
||||
* EVERY page is fetched, not just the newest 100. The list comes back
|
||||
* newest-first, which is ordering by release date and not by version: an
|
||||
* action that keeps cutting maintenance releases on lower majors pushes the
|
||||
* genuine maximum off page 1, and ranking what remains would report a lower
|
||||
* major as highest — recreating the exact false-current result that dropping
|
||||
* `releases/latest` was meant to prevent (Copilot). Paginating is the only
|
||||
* way the "highest released version" this function promises is actually that.
|
||||
*
|
||||
* @returns {string | null} `null` ONLY for a successful response carrying no
|
||||
* usable release — an action that has never cut one, or whose tags are all
|
||||
|
|
@ -317,6 +323,7 @@ function latestReleaseTag(action, spawn) {
|
|||
"gh",
|
||||
[
|
||||
"api",
|
||||
"--paginate",
|
||||
`repos/${repo}/releases?per_page=100`,
|
||||
"--jq",
|
||||
'[.[] | select(.draft == false and .prerelease == false) | .tag_name] | join("\\n")',
|
||||
|
|
|
|||
|
|
@ -62,7 +62,7 @@ function fakeSpawn({
|
|||
// MUST be tested before the milestone branch below: both are `gh api`,
|
||||
// so matching on args[0] alone hands the release lookup the milestone
|
||||
// string and the assertion silently checks nothing.
|
||||
if (args[0] === "api" && args[1].includes("/releases"))
|
||||
if (args[0] === "api" && args.some((a) => a.includes("/releases")))
|
||||
return {
|
||||
status: releasesStatus ?? 0,
|
||||
stdout: releasesStatus ? "" : releaseTags.join("\n"),
|
||||
|
|
@ -454,13 +454,22 @@ test("main asks for release lists, not the designated latest release", () => {
|
|||
captureLog(() => main("o/r", spawn));
|
||||
const lookups = spawn.calls.filter(
|
||||
(c) =>
|
||||
c.cmd === "gh" && c.args[0] === "api" && c.args[1].includes("/releases"),
|
||||
c.cmd === "gh" &&
|
||||
c.args[0] === "api" &&
|
||||
c.args.some((a) => a.includes("/releases")),
|
||||
);
|
||||
assert.ok(lookups.length > 0, "expected at least one release lookup");
|
||||
for (const call of lookups) {
|
||||
// `releases/latest` is GitHub's designated latest, not the greatest
|
||||
// version — ranking must come from the list.
|
||||
assert.doesNotMatch(call.args[1], /releases\/latest/);
|
||||
assert.match(call.args[1], /\/releases\?/);
|
||||
assert.ok(!call.args.some((a) => /releases\/latest/.test(a)));
|
||||
assert.ok(call.args.some((a) => /\/releases\?/.test(a)));
|
||||
// And every page of it: the list is ordered by release DATE, so a genuine
|
||||
// maximum can sit past page 1 behind newer maintenance releases on lower
|
||||
// majors. Ranking one page would report a lower major as highest.
|
||||
assert.ok(
|
||||
call.args.includes("--paginate"),
|
||||
"release lookup must paginate, or the highest version can be missed",
|
||||
);
|
||||
}
|
||||
});
|
||||
|
|
|
|||
Loading…
Reference in a new issue