From 6af74424d8dc7ddfe085dfa087a1e7ef469e380b Mon Sep 17 00:00:00 2001 From: cliffhall Date: Thu, 3 Sep 2026 21:41:31 -0400 Subject: [PATCH 1/4] chore(deps): remove dependabot.yml, fold action bumps into the sweep MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #2235. Switches the old dependency flow off now that #2232 landed the replacement. Removes the five npm ecosystem entries (root plus each client under clients/*) and, resolving the question #2229 left open, the github-actions entry as well — so .github/dependabot.yml goes away outright rather than being emptied, which its schema does not allow. The github-actions entry had exactly the property #2229 exists to remove: it opened a grouped monthly PR carrying no `Closes #N` and no board card, the one standing exception to "every PR references an issue". Deleting it unreplaced would have left 9 actions unwatched, and `npm outdated` says nothing about actions, so the monthly sweep now also checks every `uses:` ref under .github/workflows and renders the stale ones as one more section of the same tracking issue. Ranking comes from the release LIST, not `releases/latest`. That endpoint returns the release GitHub designates most recent, not the greatest version, so an action publishing a maintenance release for an older major (a v6.9.1 cut after v8.0.0) would make a workflow pinned to v7 compare against v6 and read as current — silently missing a whole major upgrade, the one thing this check exists to catch. Staleness is compared only to the precision the ref specifies. `v7` is a moving major tag that GitHub repoints at every v7.x release, so `v7` against a highest of `v7.0.1` is current and only `v8` makes it stale; an exactly-pinned `v7.0.0` is behind `v7.0.1`; a SHA pin is deliberately immovable and is never reported. A release lookup suppresses only a 404 — the legitimate "this action has never cut a release" answer — and throws on anything else. Treating a rate limit or an expired token as "no release" is indistinguishable from "not stale", and since every action here already sits on its latest major, the resulting empty section is byte-identical to a healthy run. `buildClearedBody` now speaks for both halves: once actions are in scope, its npm-only wording would assert a clean bill of health the sweep never checked. Dependabot security updates are unaffected — they are configured in repo settings, not in this file, and kept working while it was missing entirely (#1833, #1840). That note moves into the workflow header rather than dying with the file; #2233 is where they are turned off deliberately. The script header, the workflow header and the generated issue body all say version-update PRs rather than claiming Dependabot is replaced wholesale. Also renames the workflow's npm-outdated job to dependency-sweep now that the sweep is no longer npm-only. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01NGtzPg3UxMLszysQXXfqax Signed-off-by: cliffhall --- .github/dependabot.yml | 93 -------- .github/workflows/dependency-refresh.yml | 40 ++-- scripts/dependency-refresh.mjs | 276 +++++++++++++++++++++-- scripts/dependency-refresh.test.mjs | 197 ++++++++++++++++ 4 files changed, 472 insertions(+), 134 deletions(-) delete mode 100644 .github/dependabot.yml diff --git a/.github/dependabot.yml b/.github/dependabot.yml deleted file mode 100644 index 6888530a7..000000000 --- a/.github/dependabot.yml +++ /dev/null @@ -1,93 +0,0 @@ -version: 2 - -# Version updates target `v2/main`, the develop branch where all v2 work lands, -# not the default branch — `main` is release-only, holding the latest released -# v2 and receiving milestone merges from `v2/main`. (The deprecated v1 line -# lives on `v1/main` and takes security fixes only.) -# -# Two things to know about this file: -# 1. Dependabot reads it from the DEFAULT branch (`main`). Changes here are -# inert until the next milestone merge carries them there. -# 2. `target-branch` scopes VERSION updates. Dependabot SECURITY updates are -# enabled in repo settings, not here, and are raised against the default -# branch — they kept working while this file was missing entirely -# (see #1833, #1840). Per GitHub's Dependabot options reference, an entry -# whose `target-branch` names a non-default branch is NOT applied to -# security updates, so the schedule/labels/groups below shape version -# updates only. Re-confirm security PRs still appear after this lands. -# -# v2 is not an npm workspace: the root and each client under `clients/*` carry -# their own package.json + lockfile, so each needs its own entry. - -updates: - - package-ecosystem: "github-actions" - directory: "/" - target-branch: "v2/main" - schedule: - interval: "monthly" - labels: - - "v2" - groups: - github-actions: - patterns: - - "*" - - - package-ecosystem: "npm" - directory: "/" - target-branch: "v2/main" - schedule: - interval: "monthly" - labels: - - "v2" - groups: - root-dependencies: - patterns: - - "*" - - - package-ecosystem: "npm" - directory: "/clients/web" - target-branch: "v2/main" - schedule: - interval: "monthly" - labels: - - "v2" - groups: - web-dependencies: - patterns: - - "*" - - - package-ecosystem: "npm" - directory: "/clients/cli" - target-branch: "v2/main" - schedule: - interval: "monthly" - labels: - - "v2" - groups: - cli-dependencies: - patterns: - - "*" - - - package-ecosystem: "npm" - directory: "/clients/tui" - target-branch: "v2/main" - schedule: - interval: "monthly" - labels: - - "v2" - groups: - tui-dependencies: - patterns: - - "*" - - - package-ecosystem: "npm" - directory: "/clients/launcher" - target-branch: "v2/main" - schedule: - interval: "monthly" - labels: - - "v2" - groups: - launcher-dependencies: - patterns: - - "*" diff --git a/.github/workflows/dependency-refresh.yml b/.github/workflows/dependency-refresh.yml index 4412fa7c3..89687b71b 100644 --- a/.github/workflows/dependency-refresh.yml +++ b/.github/workflows/dependency-refresh.yml @@ -1,21 +1,27 @@ -# Monthly npm-outdated sweep (#2229), replacing Dependabot version-update PRs. +# Monthly dependency sweep (#2229), replacing Dependabot's VERSION-UPDATE PRs. # -# A Dependabot version-update PR carries no issue and no board card, so npm -# version updates are being switched off in `.github/dependabot.yml`. That file -# is changed in #2235, not here — until it lands, Dependabot's npm PRs and this -# sweep overlap, which is duplicate signal rather than conflicting action. +# A Dependabot version-update PR carries no issue and no board card, so +# `.github/dependabot.yml` was removed outright in #2235 — npm and +# github-actions alike. This workflow is what replaced those PRs (security +# updates are a separate mechanism and stay on; see below): it runs +# `scripts/dependency-refresh.mjs` against `v2/main` once a month and files or +# updates ONE tracking issue listing every outdated npm package across the root +# install and each client, plus any workflow `uses:` ref behind its action's +# highest released version. No PR is opened automatically. A maintainer +# reviews the +# issue, picks what to bump, and opens a normal PR against `v2/main`. # -# This workflow runs `scripts/dependency-refresh.mjs` against `v2/main` once a -# month and files or updates ONE tracking issue listing every outdated package -# across the root install and each client — no PR is opened automatically. A -# maintainer reviews the issue, picks what to bump, and opens a normal PR -# against `v2/main`. +# Dependabot SECURITY updates are unaffected: they are enabled in repo +# settings, not in a config file, and kept working while `dependabot.yml` was +# missing entirely (see #1833, #1840). They are raised against the default +# branch and still need retargeting by hand. # -# `GITHUB_TOKEN` is sufficient: it only needs to read milestones (public) and -# create/edit an issue (`issues: write`). Board placement is intentionally NOT -# attempted here — that needs an org-project PAT this token cannot have — so a -# filed-but-unboarded issue is picked up by the next `/issue-triage` sweep, -# same as any other maintainer-filed issue. +# `GITHUB_TOKEN` is sufficient: it only needs to read milestones and the public +# release feeds of the actions we use, and to create/edit an issue +# (`issues: write`). Board placement is intentionally NOT attempted here — that +# needs an org-project PAT this token cannot have — so a filed-but-unboarded +# issue is picked up by the next `/issue-triage` sweep, same as any other +# maintainer-filed issue. name: Dependency Refresh on: @@ -28,7 +34,7 @@ permissions: issues: write jobs: - npm-outdated: + dependency-sweep: runs-on: ubuntu-latest steps: - name: Checkout v2/main @@ -45,7 +51,7 @@ jobs: - name: Install dependencies (root + all clients) run: npm install - - name: Run the npm-outdated sweep + - name: Run the dependency sweep (npm packages + workflow actions) run: node scripts/dependency-refresh.mjs env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} diff --git a/scripts/dependency-refresh.mjs b/scripts/dependency-refresh.mjs index 6d6ad5a8b..17b540e18 100644 --- a/scripts/dependency-refresh.mjs +++ b/scripts/dependency-refresh.mjs @@ -1,23 +1,36 @@ #!/usr/bin/env node -// Monthly npm-outdated sweep (#2229), replacing Dependabot version-update PRs. +// Monthly dependency sweep (#2229), replacing Dependabot's VERSION-UPDATE PRs. // // A Dependabot version-update PR carries no issue and no board card — the same // carve-out from "every PR references an issue" that the security-update flow // had (that half is handled separately by the alert-driven pipeline, also -// #2229). Turning npm version updates off in `.github/dependabot.yml` is #2235; -// this script is the replacement it switches over to, and lands first, so the -// two flows overlap until #2235 does. Once a month it runs -// `npm outdated` across the root install and every client under `clients/*` -// (each has its own package.json + lockfile — v2 is not a workspace), and -// files or updates ONE tracking issue listing everything behind. A maintainer -// picks what to bump and opens a normal PR against `v2/main`; there is no -// auto-generated PR here at all. +// #2229). #2235 removed `.github/dependabot.yml` outright, so Dependabot opens +// no version-update PRs against this repo at all and this script is what +// replaced them. Dependabot SECURITY updates are a separate mechanism, enabled +// in repo settings rather than in that file, and are deliberately still on — +// so this replaces the version-update half only, not Dependabot wholesale. +// +// Once a month it runs `npm outdated` across the root install and every client +// under `clients/*` (each has its own package.json + lockfile — v2 is not a +// workspace), checks every `uses:` ref under `.github/workflows` against that +// action's highest released version, and files or updates ONE tracking issue +// listing everything behind. A maintainer picks what to bump and opens a +// normal PR against `v2/main`; there is no auto-generated PR here at all. +// +// The actions half is here rather than left on Dependabot because the +// `github-actions` entry had exactly the property #2229 exists to remove: it +// opened a grouped monthly PR carrying no `Closes #N` and no board card. +// Deleting that entry without replacing it would have left action versions +// unwatched, and `npm outdated` says nothing about actions — hence the +// separate release lookup below. // // Idempotent by design: the issue body starts with a fixed HTML marker // (ISSUE_MARKER below), which is how a second run in the same month finds and // updates the existing open issue instead of filing a duplicate. // -// `parseOutdated`, `buildIssueBody` and `buildClearedBody` are pure. `main()` +// `parseOutdated`, `parseActionRefs`, `parseVersionRef`, `isActionStale`, +// `staleActions`, `isMissingRelease`, `highestVersionTag`, `buildIssueBody` +// and `buildClearedBody` are pure. `main()` // shells out to `npm outdated` and `gh`, so it takes its spawn function as a // parameter (defaulting to the real one) and `dependency-refresh.test.mjs` // drives it with a fake — covering npm failure, create vs. edit, the milestone @@ -26,6 +39,8 @@ // non-zero `npm outdated` exit report a clean sweep. import { spawnSync } from "node:child_process"; +import { readdirSync, readFileSync } from "node:fs"; +import { join } from "node:path"; export const ISSUE_MARKER = ""; @@ -38,6 +53,9 @@ export const INSTALLS = [ { dir: "clients/launcher", label: "clients/launcher" }, ]; +/** Where the `uses:` refs this sweep checks live, relative to the repo root. */ +export const WORKFLOW_DIR = ".github/workflows"; + /** * Normalize one install's `npm outdated --json` output. * @@ -58,13 +76,99 @@ export function parseOutdated(json) { .sort((a, b) => a.name.localeCompare(b.name)); } +/** + * Pull every action reference out of one workflow file. + * + * Deliberately a line regex rather than a YAML parse: `uses:` is always a + * scalar on its own line in this repo's workflows, and a real parser would be + * this script's only dependency. Local (`./…`) and container (`docker://…`) + * steps are skipped — neither has a releases feed to compare against — as is + * an unpinned `uses:` with no `@ref` at all. + * + * @param {string} yaml raw contents of a workflow file + * @returns {Array<{action: string, ref: string}>} in file order, duplicates kept + */ +export function parseActionRefs(yaml) { + const refs = []; + for (const line of yaml.split("\n")) { + const match = /^\s*(?:-\s+)?uses:\s*(?:"([^"]+)"|'([^']+)'|([^\s#]+))/.exec( + line, + ); + if (!match) continue; + const uses = match[1] ?? match[2] ?? match[3]; + if (uses.startsWith("./") || uses.startsWith("docker://")) continue; + const at = uses.lastIndexOf("@"); + if (at === -1) continue; + refs.push({ action: uses.slice(0, at), ref: uses.slice(at + 1) }); + } + return refs; +} + +/** + * Split a `v`-prefixed numeric ref into its components, or `null` when it is + * not one — a SHA pin or a branch name, which a tag comparison cannot rank. + * + * @param {string} ref e.g. `v7`, `v7.0`, `7.0.1` + * @returns {number[] | null} + */ +export function parseVersionRef(ref) { + const match = /^v?(\d+)(?:\.(\d+))?(?:\.(\d+))?$/.exec(ref); + if (!match) return null; + return match + .slice(1) + .filter((part) => part !== undefined) + .map(Number); +} + +/** + * Is `current` behind `latest`? + * + * Compared only to the precision `current` actually specifies, because that is + * what pinning to it means: `v7` is a moving major tag that GitHub repoints at + * every `v7.x` release, so `v7` against a latest of `v7.0.1` is up to date and + * only `v8` makes it stale. An exactly-pinned `v7.0.0` *is* behind `v7.0.1`. + * + * @param {string} current the `uses:` ref + * @param {string} latest the action's latest release tag + * @returns {boolean} `false` when either side is not a numeric ref + */ +export function isActionStale(current, latest) { + const from = parseVersionRef(current); + const to = parseVersionRef(latest); + if (from === null || to === null) return false; + for (let i = 0; i < from.length; i++) { + const other = to[i] ?? 0; + if (other !== from[i]) return other > from[i]; + } + return false; +} + +/** + * @param {Array<{action: string, ref: string}>} refs every ref found across the workflows + * @param {Record} latestByAction latest release tag per action, `null` when unknown + * @returns {Array<{action: string, current: string, latest: string}>} the stale ones, deduped and sorted + */ +export function staleActions(refs, latestByAction) { + const stale = new Map(); + for (const { action, ref } of refs) { + const latest = latestByAction[action]; + if (!latest || !isActionStale(ref, latest)) continue; + stale.set(`${action}@${ref}`, { action, current: ref, latest }); + } + return [...stale.values()].sort( + (a, b) => + a.action.localeCompare(b.action) || a.current.localeCompare(b.current), + ); +} + /** * @param {Array<{label: string, packages: ReturnType}>} installs - * @returns {string | null} the issue body, or `null` when nothing is outdated anywhere + * @param {ReturnType} actions + * @returns {string | null} the issue body, or `null` when nothing is behind anywhere */ -export function buildIssueBody(installs) { +export function buildIssueBody(installs, actions = []) { const withPackages = installs.filter((i) => i.packages.length > 0); - if (withPackages.length === 0) return null; + if (withPackages.length === 0 && actions.length === 0) return null; const sections = withPackages.map(({ label, packages }) => { const rows = packages @@ -75,9 +179,18 @@ export function buildIssueBody(installs) { return `### \`${label}\`\n\n| Package | Current | Wanted | Latest |\n| --- | --- | --- | --- |\n${rows}`; }); + if (actions.length > 0) { + const rows = actions + .map((a) => `| \`${a.action}\` | ${a.current} | ${a.latest} |`) + .join("\n"); + sections.push( + `### GitHub Actions\n\n| Action | Current | Latest |\n| --- | --- | --- |\n${rows}`, + ); + } + return [ ISSUE_MARKER, - "Routine dependency refresh — `npm outdated` run against `v2/main` on a monthly schedule, replacing Dependabot version-update PRs (#2229).", + "Routine dependency refresh — `npm outdated` plus a workflow `uses:` check, run against `v2/main` on a monthly schedule. This sweep replaces Dependabot's version-update PRs (#2229, #2235); Dependabot security updates are a separate mechanism and remain enabled.", "", "This is a tracking issue, not a diff: pick what's worth bumping (`wanted` is the safe default; `latest` may cross a major and needs its own judgment call, especially for anything root-declared per [Dependency placement](https://github.com/modelcontextprotocol/inspector/blob/v2/main/AGENTS.md#dependency-placement)) and open a normal PR against `v2/main`.", "", @@ -88,9 +201,14 @@ export function buildIssueBody(installs) { } /** - * The body a still-open tracking issue is rewritten to once every install is - * current again. Without it the issue keeps its last package table forever and - * reads as live work that no longer exists (Copilot). + * The body a still-open tracking issue is rewritten to once every install AND + * every workflow action is current again. Without it the issue keeps its last + * table forever and reads as live work that no longer exists (Copilot). + * + * It has to speak for both halves of the sweep: once actions are in scope + * (#2235), npm-only wording here would assert a clean bill of health the sweep + * never checked, which is the same silent-all-clear shape the rest of this + * file guards against. * * The sweep rewrites rather than closes: it deliberately takes no board * actions (see the workflow header), and closing an issue whose card a @@ -103,11 +221,11 @@ export function buildIssueBody(installs) { export function buildClearedBody(isoDate) { return [ ISSUE_MARKER, - `Every install is up to date as of ${isoDate} — nothing is outdated at the root or in any client.`, + `Everything this sweep watches is current as of ${isoDate} — no npm package is outdated at the root or in any client, and no workflow \`uses:\` ref is behind its action's highest release.`, "", - "This issue was filed by an earlier run of the monthly sweep (#2229) and its package table is gone because the packages it listed are no longer behind. Either they were bumped or their ranges caught up; nothing here is outstanding.", + "This issue was filed by an earlier run of the monthly sweep (#2229, #2235) and its tables are gone because nothing they listed is behind any more. Either it was bumped or the ranges caught up; nothing here is outstanding.", "", - "Safe to close. A later sweep that finds something outdated will refile this body with a fresh table rather than open a duplicate.", + "Safe to close. A later sweep that finds something behind will refile this body with fresh tables rather than open a duplicate.", ].join("\n"); } @@ -132,6 +250,107 @@ function runOutdated(dir, spawn) { return result.stdout ?? ""; } +function collectActionRefs() { + return readdirSync(WORKFLOW_DIR) + .filter((file) => /\.ya?ml$/.test(file)) + .flatMap((file) => + parseActionRefs(readFileSync(join(WORKFLOW_DIR, file), "utf8")), + ); +} + +/** + * Is this failed release lookup the expected "publishes no releases" answer? + * + * `repos///releases/latest` 404s when an action has never cut a + * GitHub release, which is a legitimate state and must not fail the sweep. + * Every OTHER failure — a rate limit, an expired token, a transient 5xx — must, + * because treating it as "no release" is indistinguishable from "not stale": + * the sweep would exit green having silently checked nothing, and since this + * repo's actions are all on their latest major the empty section would look + * exactly like a healthy run (Copilot). + * + * @param {string} stderr stderr from a non-zero `gh api` call + * @returns {boolean} + */ +export function isMissingRelease(stderr) { + return /HTTP 404/.test(stderr); +} + +/** + * The highest parseable version among these tags, or `null` if none parse. + * + * Deliberately NOT `releases/latest`, which is GitHub's *designated* most + * recent release rather than the greatest version: an action that ships a + * maintenance release for an older major (a `v6.9.1` cut after `v8.0.0`) makes + * `releases/latest` report `v6.9.1`, and a workflow pinned to `v7` would then + * compare against v6 and read as current — silently missing a whole major + * upgrade, which is the one thing this check exists to catch (Copilot). + * + * @param {string[]} tags release tag names, in any order + * @returns {string | null} + */ +export function highestVersionTag(tags) { + let best = null; + let bestParts = null; + for (const tag of tags) { + const parts = parseVersionRef(tag); + if (parts === null) continue; + if (bestParts === null || comparePadded(parts, bestParts) > 0) { + best = tag; + bestParts = parts; + } + } + return best; +} + +/** Compare two version component arrays, padding the shorter with zeroes. */ +function comparePadded(a, b) { + for (let i = 0; i < Math.max(a.length, b.length); i++) { + const diff = (a[i] ?? 0) - (b[i] ?? 0); + if (diff !== 0) return diff; + } + return 0; +} + +/** + * The action's highest released version tag, or `null` when it has none. + * + * 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. + * + * @throws when the lookup fails for any reason other than a 404 + */ +function latestReleaseTag(action, spawn) { + // `owner/repo/subpath@ref` is a valid `uses:`; releases live on `owner/repo`. + const repo = action.split("/").slice(0, 2).join("/"); + const result = spawn( + "gh", + [ + "api", + `repos/${repo}/releases?per_page=100`, + "--jq", + '[.[] | select(.draft == false and .prerelease == false) | .tag_name] | join("\\n")', + ], + { encoding: "utf8" }, + ); + if (result.error) throw result.error; + if (result.status !== 0) { + const stderr = result.stderr ?? ""; + // A repo with no releases returns `[]`, not a 404 — but a renamed or + // deleted action really is gone, and that is not a reason to fail. + if (isMissingRelease(stderr)) return null; + throw new Error(`release lookup for ${repo} failed: ${stderr.trim()}`); + } + const tags = result.stdout + .split("\n") + .map((line) => line.trim()) + .filter(Boolean); + return highestVersionTag(tags); +} + function findExistingIssue(repo, spawn) { const result = spawn( "gh", @@ -193,15 +412,24 @@ export function main(repo = process.env.GITHUB_REPOSITORY, spawn = spawnSync) { packages: parseOutdated(runOutdated(dir, spawn)), })); + const refs = collectActionRefs(); + const latestByAction = Object.fromEntries( + [...new Set(refs.map((r) => r.action))].map((action) => [ + action, + latestReleaseTag(action, spawn), + ]), + ); + const actions = staleActions(refs, latestByAction); + // Look the existing issue up BEFORE branching on `body`: the nothing- - // outdated case still has to reach an open issue to clear it. + // behind case still has to reach an open issue to clear it. const existing = findExistingIssue(repo, spawn); - const body = buildIssueBody(installs); + const body = buildIssueBody(installs, actions); if (body === null) { if (!existing) { console.log( - "dependency-refresh: nothing outdated in any install — no-op", + "dependency-refresh: nothing outdated, no stale actions — no-op", ); return; } @@ -212,7 +440,7 @@ export function main(repo = process.env.GITHUB_REPOSITORY, spawn = spawnSync) { spawn, ); console.log( - `dependency-refresh: nothing outdated — cleared stale list on #${existing.number}`, + `dependency-refresh: nothing behind — cleared stale list on #${existing.number}`, ); return; } diff --git a/scripts/dependency-refresh.test.mjs b/scripts/dependency-refresh.test.mjs index f180b863c..4cf79226a 100644 --- a/scripts/dependency-refresh.test.mjs +++ b/scripts/dependency-refresh.test.mjs @@ -12,8 +12,14 @@ import assert from "node:assert/strict"; import { buildClearedBody, buildIssueBody, + highestVersionTag, + isActionStale, + isMissingRelease, main, + parseActionRefs, parseOutdated, + parseVersionRef, + staleActions, INSTALLS, ISSUE_MARKER, } from "./dependency-refresh.mjs"; @@ -26,12 +32,18 @@ import { * @param {number} [opts.outdatedStatus] exit status for every `npm outdated` * @param {Array<{number:number,body:string}>} [opts.existing] what `gh issue list` returns * @param {string|null} [opts.milestone] what the milestone lookup returns + * @param {string[]} [opts.releaseTags] tags every action's release list returns + * @param {number} [opts.releasesStatus] exit status for every release lookup + * @param {string} [opts.releasesStderr] stderr for a failing release lookup */ function fakeSpawn({ outdated = {}, outdatedStatus, existing = [], milestone = "v2.6.0", + releaseTags = [], + releasesStatus, + releasesStderr = "", } = {}) { const calls = []; const fn = (cmd, args, opts) => { @@ -48,6 +60,15 @@ function fakeSpawn({ } if (args[0] === "issue" && args[1] === "list") return { status: 0, stdout: JSON.stringify(existing), stderr: "" }; + // 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")) + return { + status: releasesStatus ?? 0, + stdout: releasesStatus ? "" : releaseTags.join("\n"), + stderr: releasesStderr, + }; if (args[0] === "api") return { status: 0, @@ -107,6 +128,112 @@ test("parseOutdated falls back when a field is missing", () => { ]); }); +test("parseActionRefs pulls owner/repo@ref out of a workflow, in file order", () => { + const yaml = [ + "jobs:", + " build:", + " steps:", + " - uses: actions/checkout@v7", + " - name: Setup", + " uses: actions/setup-node@v7", + ' - uses: "docker/login-action@v4"', + " - uses: docker/build-push-action@v7 # trailing comment", + ].join("\n"); + assert.deepEqual(parseActionRefs(yaml), [ + { action: "actions/checkout", ref: "v7" }, + { action: "actions/setup-node", ref: "v7" }, + { action: "docker/login-action", ref: "v4" }, + { action: "docker/build-push-action", ref: "v7" }, + ]); +}); + +test("parseActionRefs skips local, container and unpinned steps", () => { + const yaml = [ + " - uses: ./.github/actions/local", + " - uses: docker://alpine:3.20", + " - uses: actions/checkout", + " - uses: github/codeql-action/init@v3", + ].join("\n"); + assert.deepEqual(parseActionRefs(yaml), [ + { action: "github/codeql-action/init", ref: "v3" }, + ]); +}); + +test("parseVersionRef reads a numeric ref and rejects anything else", () => { + assert.deepEqual(parseVersionRef("v7"), [7]); + assert.deepEqual(parseVersionRef("7.0.1"), [7, 0, 1]); + assert.equal(parseVersionRef("main"), null); + assert.equal( + parseVersionRef("8f4b7f84864484a7bf31766abe9204da3cbe65b3"), + null, + ); +}); + +test("isActionStale compares only to the precision the ref specifies", () => { + // `v7` is a moving major tag, so a v7.x release does not make it stale. + assert.equal(isActionStale("v7", "v7.0.1"), false); + assert.equal(isActionStale("v7", "v8.0.0"), true); + // An exactly-pinned ref is behind its own patch release. + assert.equal(isActionStale("v7.0.0", "v7.0.1"), true); + assert.equal(isActionStale("v7.1", "v7.0.9"), false); +}); + +test("isActionStale reports nothing for a ref it cannot rank", () => { + // A SHA pin is deliberately immovable; a tag comparison says nothing about it. + assert.equal( + isActionStale("8f4b7f84864484a7bf31766abe9204da3cbe65b3", "v5"), + false, + ); + assert.equal(isActionStale("main", "v5"), false); + assert.equal(isActionStale("v5", "not-a-tag"), false); +}); + +test("highestVersionTag picks the greatest version, not the newest entry", () => { + // GitHub's `releases/latest` is the DESIGNATED latest, not the greatest + // version: a maintenance release cut for an older major after a newer one + // would make a workflow on v7 read as current (Copilot). + assert.equal(highestVersionTag(["v6.9.1", "v8.0.0", "v7.2.0"]), "v8.0.0"); + assert.equal(highestVersionTag(["v7", "v7.0.1"]), "v7.0.1"); + assert.equal(highestVersionTag(["v10.0.0", "v9.9.9"]), "v10.0.0"); +}); + +test("highestVersionTag ignores tags it cannot parse", () => { + assert.equal(highestVersionTag(["nightly", "v2.0.0", "latest"]), "v2.0.0"); + assert.equal(highestVersionTag(["nightly", "latest"]), null); + assert.equal(highestVersionTag([]), null); +}); + +test("staleActions dedupes, drops actions with no known release and sorts", () => { + const refs = [ + { action: "actions/checkout", ref: "v7" }, + { action: "actions/cache", ref: "v6" }, + { action: "actions/cache", ref: "v6" }, + { action: "some/unreleased", ref: "v1" }, + ]; + assert.deepEqual( + staleActions(refs, { + "actions/cache": "v7.0.0", + "actions/checkout": "v7.0.1", + "some/unreleased": null, + }), + [{ action: "actions/cache", current: "v6", latest: "v7.0.0" }], + ); +}); + +test("isMissingRelease suppresses only a 404, never a real API failure", () => { + // A 404 is the legitimate "this action cuts no GitHub releases" answer. + assert.equal(isMissingRelease("gh: Not Found (HTTP 404)"), true); + // Everything else must fail the sweep rather than read as "not stale" — an + // empty actions section is indistinguishable from a healthy run (Copilot). + assert.equal( + isMissingRelease("gh: API rate limit exceeded (HTTP 403)"), + false, + ); + assert.equal(isMissingRelease("gh: Bad credentials (HTTP 401)"), false); + assert.equal(isMissingRelease("gh: Server Error (HTTP 502)"), false); + assert.equal(isMissingRelease(""), false); +}); + test("buildIssueBody returns null when every install is up to date", () => { assert.equal( buildIssueBody([ @@ -266,3 +393,73 @@ test("main falls back to GITHUB_REPOSITORY when no repo is passed", () => { else process.env.GITHUB_REPOSITORY = saved; } }); + +test("buildIssueBody renders a GitHub Actions section after the npm ones", () => { + const body = buildIssueBody( + [{ label: "root", packages: [] }], + [{ action: "actions/cache", current: "v6", latest: "v7.0.0" }], + ); + assert.ok(body.startsWith(ISSUE_MARKER)); + assert.ok(body.includes("### GitHub Actions")); + assert.ok(body.includes("| `actions/cache` | v6 | v7.0.0 |")); + assert.ok(!body.includes("### `root`")); +}); + +test("buildIssueBody returns null only when npm and actions are both clean", () => { + assert.equal(buildIssueBody([{ label: "root", packages: [] }], []), null); + assert.notEqual( + buildIssueBody( + [], + [{ action: "actions/cache", current: "v6", latest: "v7.0.0" }], + ), + null, + ); +}); + +test("buildClearedBody speaks for both halves of the sweep", () => { + const body = buildClearedBody("2026-09-04"); + assert.ok(body.startsWith(ISSUE_MARKER)); + assert.ok(body.includes("2026-09-04")); + // npm-only wording here would assert an all-clear the sweep never checked. + assert.match(body, /npm package/); + assert.match(body, /uses:/); +}); + +test("main fails the sweep when a release lookup errors, rather than reporting no stale actions", () => { + // The silent-success shape one level down: a 403 read as "no release" is + // indistinguishable from "not stale", and every action here is already on + // its latest major, so the empty section would look like a healthy run. + const spawn = fakeSpawn({ + releasesStatus: 1, + releasesStderr: "gh: API rate limit exceeded (HTTP 403)", + }); + assert.throws( + () => captureLog(() => main("o/r", spawn)), + /release lookup for .* failed/, + ); +}); + +test("main treats a 404 release lookup as 'this action cuts no releases'", () => { + const spawn = fakeSpawn({ + releasesStatus: 1, + releasesStderr: "gh: Not Found (HTTP 404)", + }); + const log = captureLog(() => main("o/r", spawn)); + assert.match(log.join("\n"), /no-op/); +}); + +test("main asks for release lists, not the designated latest release", () => { + const spawn = fakeSpawn(); + captureLog(() => main("o/r", spawn)); + const lookups = spawn.calls.filter( + (c) => + c.cmd === "gh" && c.args[0] === "api" && c.args[1].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\?/); + } +}); From dde1736702864598bd7a3b848492921d083638e0 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Thu, 3 Sep 2026 23:19:58 -0400 Subject: [PATCH 2/4] chore(deps): treat every failed release-list response as fatal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses Copilot review round 4 on #2239. Both findings are regressions I introduced in round 3 and both are correct. Round 3 moved the lookup from `releases/latest` to the release LIST, to rank by version rather than by GitHub's designated latest — but kept the 404 suppression that only made sense for the old endpoint. The list endpoint answers "this action cuts no releases" with a successful empty array, so a 404 there does not mean that at all: it means the repository is missing or inaccessible, i.e. a `uses:` ref the sweep cannot check. Converting it to null silently dropped a broken or renamed action from the flow that replaced Dependabot — the same silent-success shape this PR has been closing everywhere else, reintroduced one layer down. Every non-zero status is now fatal and `isMissingRelease` is gone entirely; the benign no-releases case is the successful empty array, covered by its own test. `buildClearedBody` also overstated its all-clear. Refs pinned to a commit SHA or a branch return null from `parseVersionRef` and are never ranked, so "no workflow `uses:` ref is behind" asserted a check that had not happened for them. It now says version-pinned refs and names the exclusion explicitly. Both guards are mutation-checked: restoring the 404 suppression, and dropping the SHA/branch qualification, each fail exactly one test. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01NGtzPg3UxMLszysQXXfqax Signed-off-by: cliffhall --- scripts/dependency-refresh.mjs | 41 +++++++++++------------------ scripts/dependency-refresh.test.mjs | 33 ++++++++++++----------- 2 files changed, 32 insertions(+), 42 deletions(-) diff --git a/scripts/dependency-refresh.mjs b/scripts/dependency-refresh.mjs index 17b540e18..ab54ec1c3 100644 --- a/scripts/dependency-refresh.mjs +++ b/scripts/dependency-refresh.mjs @@ -29,8 +29,8 @@ // updates the existing open issue instead of filing a duplicate. // // `parseOutdated`, `parseActionRefs`, `parseVersionRef`, `isActionStale`, -// `staleActions`, `isMissingRelease`, `highestVersionTag`, `buildIssueBody` -// and `buildClearedBody` are pure. `main()` +// `staleActions`, `highestVersionTag`, `buildIssueBody` and +// `buildClearedBody` are pure. `main()` // shells out to `npm outdated` and `gh`, so it takes its spawn function as a // parameter (defaulting to the real one) and `dependency-refresh.test.mjs` // drives it with a fake — covering npm failure, create vs. edit, the milestone @@ -221,7 +221,9 @@ export function buildIssueBody(installs, actions = []) { export function buildClearedBody(isoDate) { return [ ISSUE_MARKER, - `Everything this sweep watches is current as of ${isoDate} — no npm package is outdated at the root or in any client, and no workflow \`uses:\` ref is behind its action's highest release.`, + `Everything this sweep can check is current as of ${isoDate} — no npm package is outdated at the root or in any client, and no version-pinned workflow \`uses:\` ref is behind its action's highest release.`, + "", + "Refs pinned to a commit SHA or a branch are deliberately **not** covered by that statement: neither can be ranked against a release tag, so this sweep says nothing about them either way.", "", "This issue was filed by an earlier run of the monthly sweep (#2229, #2235) and its tables are gone because nothing they listed is behind any more. Either it was bumped or the ranges caught up; nothing here is outstanding.", "", @@ -258,24 +260,6 @@ function collectActionRefs() { ); } -/** - * Is this failed release lookup the expected "publishes no releases" answer? - * - * `repos///releases/latest` 404s when an action has never cut a - * GitHub release, which is a legitimate state and must not fail the sweep. - * Every OTHER failure — a rate limit, an expired token, a transient 5xx — must, - * because treating it as "no release" is indistinguishable from "not stale": - * the sweep would exit green having silently checked nothing, and since this - * repo's actions are all on their latest major the empty section would look - * exactly like a healthy run (Copilot). - * - * @param {string} stderr stderr from a non-zero `gh api` call - * @returns {boolean} - */ -export function isMissingRelease(stderr) { - return /HTTP 404/.test(stderr); -} - /** * The highest parseable version among these tags, or `null` if none parse. * @@ -337,12 +321,17 @@ function latestReleaseTag(action, spawn) { { encoding: "utf8" }, ); if (result.error) throw result.error; + // EVERY non-zero status is fatal, 404 included. This is the release *list* + // endpoint, which answers "no releases" with a successful empty array — so a + // 404 here does not mean "this action cuts no releases", it means the + // repository is missing or inaccessible, i.e. a `uses:` ref this sweep + // cannot check at all. Converting that to `null` would silently drop a + // broken or renamed action from the sweep that replaced Dependabot + // (Copilot). The empty-array case is already handled by the parse below. if (result.status !== 0) { - const stderr = result.stderr ?? ""; - // A repo with no releases returns `[]`, not a 404 — but a renamed or - // deleted action really is gone, and that is not a reason to fail. - if (isMissingRelease(stderr)) return null; - throw new Error(`release lookup for ${repo} failed: ${stderr.trim()}`); + throw new Error( + `release lookup for ${repo} failed (exit ${result.status}): ${(result.stderr ?? "").trim()}`, + ); } const tags = result.stdout .split("\n") diff --git a/scripts/dependency-refresh.test.mjs b/scripts/dependency-refresh.test.mjs index 4cf79226a..55af8354a 100644 --- a/scripts/dependency-refresh.test.mjs +++ b/scripts/dependency-refresh.test.mjs @@ -14,7 +14,6 @@ import { buildIssueBody, highestVersionTag, isActionStale, - isMissingRelease, main, parseActionRefs, parseOutdated, @@ -220,20 +219,6 @@ test("staleActions dedupes, drops actions with no known release and sorts", () = ); }); -test("isMissingRelease suppresses only a 404, never a real API failure", () => { - // A 404 is the legitimate "this action cuts no GitHub releases" answer. - assert.equal(isMissingRelease("gh: Not Found (HTTP 404)"), true); - // Everything else must fail the sweep rather than read as "not stale" — an - // empty actions section is indistinguishable from a healthy run (Copilot). - assert.equal( - isMissingRelease("gh: API rate limit exceeded (HTTP 403)"), - false, - ); - assert.equal(isMissingRelease("gh: Bad credentials (HTTP 401)"), false); - assert.equal(isMissingRelease("gh: Server Error (HTTP 502)"), false); - assert.equal(isMissingRelease(""), false); -}); - test("buildIssueBody returns null when every install is up to date", () => { assert.equal( buildIssueBody([ @@ -423,6 +408,9 @@ test("buildClearedBody speaks for both halves of the sweep", () => { // npm-only wording here would assert an all-clear the sweep never checked. assert.match(body, /npm package/); assert.match(body, /uses:/); + // Must not claim every ref was verified: SHA- and branch-pinned refs are + // never ranked against a release (Copilot). + assert.match(body, /SHA or a branch/); }); test("main fails the sweep when a release lookup errors, rather than reporting no stale actions", () => { @@ -439,11 +427,24 @@ test("main fails the sweep when a release lookup errors, rather than reporting n ); }); -test("main treats a 404 release lookup as 'this action cuts no releases'", () => { +test("main fails on a 404 release lookup — a missing action repo is not 'no releases'", () => { + // The release LIST endpoint answers "no releases" with a successful empty + // array, so a 404 means the repository is missing or inaccessible. Treating + // it as benign would silently drop a broken or renamed action from the sweep + // that replaced Dependabot (Copilot). const spawn = fakeSpawn({ releasesStatus: 1, releasesStderr: "gh: Not Found (HTTP 404)", }); + assert.throws( + () => captureLog(() => main("o/r", spawn)), + /release lookup for .* failed/, + ); +}); + +test("main treats an empty release list as 'this action cuts no releases'", () => { + // The benign case: status 0 with no tags. The action is simply not ranked. + const spawn = fakeSpawn({ releaseTags: [] }); const log = captureLog(() => main("o/r", spawn)); assert.match(log.join("\n"), /no-op/); }); From 8414685ad9bef67ea357958ad2ce2989ebfe2583 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Thu, 3 Sep 2026 23:47:05 -0400 Subject: [PATCH 3/4] docs(scripts): correct the release-lookup contract left stale by round 4 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses Copilot review round 5 on #2239. Round 4 made every non-zero release-list response fatal and deleted `isMissingRelease`, but `latestReleaseTag`'s JSDoc still promised `@throws when the lookup fails for any reason other than a 404` — the exact behavior that change removed. The PR description carried the same stale claim. A contract that documents the opposite of the code is worse than none: the next reader reasonably trusts it, and here it would tell them a missing action repo is silently tolerated when it now fails the sweep. The contract now reserves `null` for a successful response carrying no usable release — an action that has never cut one, or whose tags are all unparseable — and states that a failed lookup is never `null`. Documentation only; no behavior change, and the 29 tests are untouched. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01NGtzPg3UxMLszysQXXfqax Signed-off-by: cliffhall --- scripts/dependency-refresh.mjs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/scripts/dependency-refresh.mjs b/scripts/dependency-refresh.mjs index ab54ec1c3..e0474f153 100644 --- a/scripts/dependency-refresh.mjs +++ b/scripts/dependency-refresh.mjs @@ -297,7 +297,7 @@ function comparePadded(a, b) { } /** - * The action's highest released version tag, or `null` when it has none. + * The action's highest released version tag. * * Reads the release *list* rather than `releases/latest`, for the reason on * `highestVersionTag`. Drafts and prereleases are excluded — neither is @@ -305,7 +305,10 @@ function comparePadded(a, b) { * 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. * - * @throws when the lookup fails for any reason other than a 404 + * @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 + * unparseable. A failed lookup is never `null`; see below. + * @throws on any non-zero status, 404 included */ function latestReleaseTag(action, spawn) { // `owner/repo/subpath@ref` is a valid `uses:`; releases live on `owner/repo`. From 0cb12a19ca2dde8273852db3b2ff317137099d79 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Fri, 4 Sep 2026 00:28:15 -0400 Subject: [PATCH 4/4] chore(deps): paginate the release list so the highest version is actual MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) Claude-Session: https://claude.ai/code/session_01NGtzPg3UxMLszysQXXfqax Signed-off-by: cliffhall --- scripts/dependency-refresh.mjs | 13 ++++++++++--- scripts/dependency-refresh.test.mjs | 17 +++++++++++++---- 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/scripts/dependency-refresh.mjs b/scripts/dependency-refresh.mjs index e0474f153..be87fba4e 100644 --- a/scripts/dependency-refresh.mjs +++ b/scripts/dependency-refresh.mjs @@ -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")', diff --git a/scripts/dependency-refresh.test.mjs b/scripts/dependency-refresh.test.mjs index 55af8354a..bbf407e88 100644 --- a/scripts/dependency-refresh.test.mjs +++ b/scripts/dependency-refresh.test.mjs @@ -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", + ); } });