diff --git a/.github/scripts/pin-bump.cjs b/.github/scripts/pin-bump.cjs new file mode 100644 index 00000000..b96385aa --- /dev/null +++ b/.github/scripts/pin-bump.cjs @@ -0,0 +1,79 @@ +#!/usr/bin/env node +// SPDX-License-Identifier: MIT +"use strict"; + +/** + * Move a family of exact dependency pins from one version to another, in the + * package.json SOURCE TEXT rather than in a parsed object. + * + * Shared by the two upgrade detectors — ast-grep's and Vale's — because the + * rewrite is the same operation on a different prefix, and because the count + * check below is the kind of guard that is worth having exactly one of. + * + * WHY TEXT AND NOT JSON. Parsing and re-serializing would reformat a file this + * repository formats with prettier, and it would do so in CI, where no + * `lint-staged` runs to normalize it back. The bump would then arrive as a + * whole-file diff with six version strings buried in it. A targeted replacement + * leaves every byte it did not have to touch, so the review is the versions. + * + * WHY THE COUNT IS RETURNED RATHER THAN ASSUMED. A pin the pattern fails to + * match is the failure that matters: it leaves a straggler at the old version, + * and because these platform packages are selected by optional dependency, a + * straggler is a DIFFERENT BINARY on one platform than on the others. Callers + * compare this count against the pins they independently enumerated and abort + * on a mismatch, so the two disagreeing is a failed run rather than a + * half-applied upgrade that looks fine in review. + */ + +/** Escape a version for literal use in a regular expression. */ +function escapeLiteral(text) { + return String(text).replaceAll(/[.*+?^${}()|[\]\\]/g, String.raw`\$&`); +} + +/** + * WHY THE CALLER PASSES A PATTERN RATHER THAN A PREFIX. + * + * This used to take a bare string prefix and match any key starting with it, + * while the `collectPins` it is paired with used a boundary-aware pattern + * (`/^@ast-grep\/cli(-|$)/`). Two functions that are supposed to agree on what + * counts as a pin disagreed on it, and only the `count !== pins.size` check + * downstream kept that from mattering. Relying on a guard to paper over a + * disagreement is not the same as not having one: the guard turns the + * disagreement into a failed run, which is better than a wrong bump and worse + * than the two agreeing in the first place. + * + * Taking the pattern means the caller hands BOTH functions the same constant, + * so they cannot drift apart at all. + * + * @param source the package.json text + * @param pattern anchored RegExp matching a package NAME, the same one the + * caller enumerates pins with + * @param from the exact version every matching pin currently holds + * @param to the exact version to write + */ +function bumpPins(source, { pattern, from, to }) { + // A /g regexp carries `lastIndex` between calls, so `.test()` would alternate + // true and false down the file and silently skip every other pin. Refusing it + // beats stripping the flag, because a caller passing /g also uses that same + // constant for its own enumeration, where the bug would be just as quiet. + if (pattern.global) { + throw new Error( + "the pin pattern must not be /g: a stateful lastIndex would skip pins" + ); + } + const matcher = new RegExp( + `("([^"]+)"\\s*:\\s*")${escapeLiteral(from)}(")`, + "g" + ); + let count = 0; + const bumped = source.replaceAll(matcher, (match, head, name, tail) => { + if (!pattern.test(name)) { + return match; + } + count += 1; + return `${head}${to}${tail}`; + }); + return { source: bumped, count }; +} + +module.exports = { bumpPins, escapeLiteral }; diff --git a/.github/scripts/pin-bump.test.cjs b/.github/scripts/pin-bump.test.cjs new file mode 100644 index 00000000..1302af40 --- /dev/null +++ b/.github/scripts/pin-bump.test.cjs @@ -0,0 +1,116 @@ +// SPDX-License-Identifier: MIT +"use strict"; + +/** + * Tests for pin-bump.cjs. + * + * Two properties matter here and neither is about the happy path. The rewrite + * must not touch a package it was not asked about, and it must agree with the + * caller's own enumeration about what counts as a pin — a disagreement there + * leaves a straggler at the old version, which for optional-dependency platform + * packages means a different binary on one platform than on the others. + */ + +const test = require("node:test"); +const assert = require("node:assert/strict"); + +const { bumpPins } = require("./pin-bump.cjs"); + +/** The boundary-aware pattern sg-detect.cjs enumerates its pins with. */ +const AST_GREP = /^@ast-grep\/cli(-|$)/; + +const sourcePinnedAt = (version) => + `${JSON.stringify( + { + name: "@taskless/cli", + dependencies: { "@ast-grep/cli": version, zod: "^4.0.0" }, + optionalDependencies: { + "@ast-grep/cli-darwin-arm64": version, + "@ast-grep/cli-linux-x64-gnu": version, + }, + }, + undefined, + 2 + )}\n`; + +test("every pin moves and nothing else does", () => { + const before = sourcePinnedAt("0.45.2"); + const { source, count } = bumpPins(before, { + pattern: AST_GREP, + from: "0.45.2", + to: "0.45.3", + }); + + assert.equal(count, 3); + assert.equal(source.match(/0\.45\.3/g).length, 3); + assert.doesNotMatch(source, /0\.45\.2/); + assert.match(source, /"zod": "\^4\.0\.0"/); + // The formatting is untouched, so the diff a reviewer reads is the versions. + assert.equal(before.split("\n").length, source.split("\n").length); +}); + +/** + * The dots in a version are regular-expression metacharacters. Unescaped, + * `0.45.2` also matches `0145.2`. + */ +test("the version is matched literally, not as a pattern", () => { + const { count } = bumpPins('{ "@ast-grep/cli": "0145.2" }', { + pattern: AST_GREP, + from: "0.45.2", + to: "0.45.3", + }); + assert.equal(count, 0); +}); + +test("the same version under an unrelated package is left alone", () => { + const { source, count } = bumpPins( + '{ "some-other-tool": "0.45.2", "@ast-grep/cli": "0.45.2" }', + { pattern: AST_GREP, from: "0.45.2", to: "0.45.3" } + ); + assert.equal(count, 1); + assert.match(source, /"some-other-tool": "0\.45\.2"/); +}); + +/** + * The reason the caller passes a pattern rather than a prefix. A bare prefix + * match treats `@ast-grep/clippy` as a pin because the string starts the same + * way, while the `collectPins` it is paired with — anchored on `(-|$)` — does + * not. The two would then disagree about what a pin is, and only the caller's + * count check would notice. + */ +test("a package that merely starts with the prefix is not a pin", () => { + const { source, count } = bumpPins( + '{ "@ast-grep/clippy": "0.45.2", "@ast-grep/cli-darwin-arm64": "0.45.2" }', + { pattern: AST_GREP, from: "0.45.2", to: "0.45.3" } + ); + assert.equal(count, 1); + assert.match(source, /"@ast-grep\/clippy": "0\.45\.2"/); + assert.match(source, /"@ast-grep\/cli-darwin-arm64": "0\.45\.3"/); +}); + +test("the Vale prefix carries its boundary in the pattern", () => { + const pattern = /^@taskless\/vale-/; + const { count } = bumpPins( + '{ "@taskless/vale-linux-x64": "3.20.0-20260907164938", "@taskless/valet": "3.20.0-20260907164938" }', + { pattern, from: "3.20.0-20260907164938", to: "3.21.0-20260914010203" } + ); + assert.equal(count, 1); +}); + +/** + * A /g regexp carries `lastIndex` between calls, so `.test()` alternates true + * and false down the file and silently skips every other pin. Refusing it beats + * stripping the flag: the caller uses that same constant for its own + * enumeration, where the bug would be just as quiet. + */ +test("a stateful /g pattern is refused rather than silently skipping pins", () => { + assert.throws( + () => + bumpPins(sourcePinnedAt("0.45.2"), { + pattern: /^@ast-grep\/cli(-|$)/g, + from: "0.45.2", + to: "0.45.3", + }), + /must not be \/g/ + ); +}); diff --git a/.github/scripts/release-notes.cjs b/.github/scripts/release-notes.cjs new file mode 100644 index 00000000..e7493486 --- /dev/null +++ b/.github/scripts/release-notes.cjs @@ -0,0 +1,187 @@ +#!/usr/bin/env node +// SPDX-License-Identifier: MIT +"use strict"; + +/** + * Upstream release notes, fetched and rendered for a pull request or issue body. + * + * WHY THIS EXISTS. Both detect scripts answer "is upstream ahead?" with a + * version number, and a version number is not enough to review a bump. A + * reviewer looking at `3.20.0 -> 3.21.0` and six changed digests has no way to + * tell a security fix from a docs release without leaving the pull request and + * going to find the changelog by hand. Carrying the notes into the body is the + * difference between reviewing a bump and approving one. + * + * THE NOTES ARE UNTRUSTED TEXT. They are Markdown written by a third party and + * they reach a body that a workflow composes. Nothing here interpolates them + * into a shell command, a `${{ }}` expression, or a $GITHUB_OUTPUT line — a + * release body is free to contain a heredoc terminator or an output delimiter, + * and either one is an injection if it meets a shell. `writeNotesFile` puts + * them on disk and the workflow passes that path to `--body-file`, so the bytes + * never pass through an interpreter. + * + * They are also FENCED when rendered. An upstream body that opens a fence and + * never closes it, or that ends mid-table, would otherwise swallow whatever the + * workflow appends after it. Quoting the whole block as Markdown blockquote + * lines keeps the surrounding body's structure independent of what upstream + * wrote, at the cost of one level of indentation in the rendering. + */ + +const { writeFileSync } = require("node:fs"); + +/** + * How much of an upstream body to carry, measured AFTER quoting. + * + * A GitHub pull request or issue body caps at 65536 characters, and a body that + * hits the cap fails the API call rather than truncating — so the whole + * proposal is lost to a talkative release. The limit is well under the cap + * because the workflow's own preamble and the truncation footer have to fit + * alongside it. + * + * Measured after quoting because the two differ by more than a rounding error: + * every line gains two characters of blockquote marker, so a release whose + * notes are a long list of short lines — which is exactly what ast-grep's + * generated changelog is — nearly doubles. Budgeting against the source length + * let a 40000-character body render as 80202, over the cap the limit exists to + * stay under. + */ +const NOTES_LIMIT = 40_000; + +/** + * Fetch a release from the GitHub API. + * + * `reference` is either `latest` or `tags/`, matching the two endpoint + * shapes. GITHUB_TOKEN, when present, is only for the rate limit; both + * endpoints are public. + * + * A missing release is NOT an error. Upstream may tag without publishing a + * release, or name its tags differently from its npm versions, and neither is a + * reason to fail a detect run whose actual job — comparing versions — already + * succeeded. The caller gets `undefined` and says so in the body. + */ +async function fetchRelease(repository, reference) { + const headers = { + accept: "application/vnd.github+json", + "user-agent": "taskless-release-notes", + }; + if (process.env.GITHUB_TOKEN) { + headers.authorization = `Bearer ${process.env.GITHUB_TOKEN}`; + } + const url = `https://api.github.com/repos/${repository}/releases/${reference}`; + const response = await fetch(url, { headers }); + if (response.status === 404) { + return undefined; + } + if (!response.ok) { + throw new Error(`GET ${url} responded ${response.status}`); + } + const release = await response.json(); + if (typeof release.tag_name !== "string") { + throw new TypeError(`${url} returned no tag_name`); + } + return { + tag: release.tag_name, + notes: typeof release.body === "string" ? release.body : "", + url: typeof release.html_url === "string" ? release.html_url : undefined, + }; +} + +/** The latest non-prerelease, non-draft release. */ +const fetchLatestRelease = (repository) => fetchRelease(repository, "latest"); + +/** A specific release by tag, for upstreams whose version we learn elsewhere. */ +const fetchReleaseByTag = (repository, tag) => + fetchRelease(repository, `tags/${encodeURIComponent(tag)}`); + +/** + * Render a release as a Markdown section, quoted so it cannot restructure the + * body around it. + * + * `release` may be undefined (no release found) and its `notes` may be empty (a + * release published with no body). Both render as a line saying so plus a link, + * rather than as an absent section — "upstream wrote no notes" and "we forgot + * to fetch them" look identical otherwise, and only one of them is fine. + */ +function formatReleaseNotes({ + repository, + version, + release, + limit = NOTES_LIMIT, +}) { + const link = release?.url ?? `https://github.com/${repository}/releases`; + const heading = `## Upstream release notes — ${version}\n\n${link}\n`; + + const body = release?.notes?.trim() ?? ""; + if (body.length === 0) { + return `${heading}\nUpstream published no release notes for this version.\n`; + } + + // Truncation happens on a line boundary, so the rendering cannot end + // mid-marker and produce a line that is not part of the quoted block. + const lines = body + .split("\n") + .map((line) => (line.length > 0 ? `> ${line}` : ">")); + const kept = []; + let length = 0; + for (const line of lines) { + if (length + line.length + 1 > limit) { + break; + } + kept.push(line); + length += line.length + 1; + } + let truncated = kept.length < lines.length; + // A first line longer than the whole budget keeps nothing, which would render + // a truncation footer under an empty quote. Cut inside that line instead: a + // release whose notes are one enormous paragraph still says something. This + // is also truncation, even when it drops no whole line. + if (kept.length === 0) { + kept.push(lines[0].slice(0, limit)); + truncated = true; + } + const quoted = kept.join("\n"); + + const footer = truncated + ? `\n\n_Truncated at ${limit} characters. Read the rest at ${link}._\n` + : "\n"; + return `${heading}\n${quoted}\n${footer}`; +} + +/** + * Read `--notes-out ` from an argv array, or undefined when absent. + * + * Lives here rather than in each detect script because all three parse the same + * flag for the same reason, and a change to how it is parsed — accepting + * `--notes-out=path`, say — should not be a change three files have to make in + * agreement. A partial fix would leave one workflow silently writing nothing. + * + * A following value that looks like another flag is an error rather than a + * path. `--notes-out --write` is a caller that forgot the argument, and taking + * `--write` as a filename would write release notes to a file named `--write` + * and drop the flag that was meant to do the work. + */ +function readNotesOut(argv) { + const at = argv.indexOf("--notes-out"); + if (at === -1) { + return undefined; + } + const path = argv[at + 1]; + if (!path || path.startsWith("--")) { + throw new Error("--notes-out needs a path"); + } + return path; +} + +/** Write a rendered section to disk for a workflow to pass to `--body-file`. */ +function writeNotesFile(path, section) { + writeFileSync(path, section.endsWith("\n") ? section : `${section}\n`); +} + +module.exports = { + NOTES_LIMIT, + fetchLatestRelease, + fetchReleaseByTag, + formatReleaseNotes, + readNotesOut, + writeNotesFile, +}; diff --git a/.github/scripts/release-notes.test.cjs b/.github/scripts/release-notes.test.cjs new file mode 100644 index 00000000..af19af13 --- /dev/null +++ b/.github/scripts/release-notes.test.cjs @@ -0,0 +1,196 @@ +// SPDX-License-Identifier: MIT +"use strict"; + +/** + * Tests for release-notes.cjs. + * + * The rendering is what is worth pinning down. These notes are third-party + * Markdown pasted into a body that a workflow composes, so the two failures + * that matter are structural: upstream restructuring the body around it, and + * upstream being long enough to push the body past GitHub's 65536-character + * cap, which fails the API call outright rather than truncating. + */ + +const test = require("node:test"); +const assert = require("node:assert/strict"); +const { mkdtempSync, readFileSync, rmSync } = require("node:fs"); +const { tmpdir } = require("node:os"); +const { join } = require("node:path"); + +const { + NOTES_LIMIT, + fetchLatestRelease, + formatReleaseNotes, + readNotesOut, + writeNotesFile, +} = require("./release-notes.cjs"); + +const release = (notes) => ({ + tag: "v3.21.0", + notes, + url: "https://github.com/errata-ai/vale/releases/tag/v3.21.0", +}); + +test("notes: the section names the version and links the release", () => { + const section = formatReleaseNotes({ + repository: "errata-ai/vale", + version: "3.21.0", + release: release("Some change."), + }); + assert.match(section, /^## Upstream release notes — 3\.21\.0$/m); + assert.match( + section, + /^https:\/\/github\.com\/errata-ai\/vale\/releases\/tag\/v3\.21\.0$/m + ); + assert.match(section, /^> Some change\.$/m); +}); + +/** + * The reason every line is quoted. An upstream body that opens a fence and + * never closes it would otherwise swallow whatever the workflow appends after + * it — in the Vale case, nothing, but in the ast-grep case the body is composed + * the other way round and a runaway fence eats the preamble. + */ +test("notes: an unterminated fence cannot escape the quoted block", () => { + const section = formatReleaseNotes({ + repository: "errata-ai/vale", + version: "3.21.0", + release: release("```yaml\nscope: sentence"), + }); + for (const line of section.split("\n").slice(4)) { + if (line.length > 0) { + assert.match(line, /^>/, `unquoted line escaped the block: ${line}`); + } + } +}); + +test("notes: blank lines stay blank rather than becoming trailing spaces", () => { + const section = formatReleaseNotes({ + repository: "errata-ai/vale", + version: "3.21.0", + release: release("First.\n\nSecond."), + }); + assert.match(section, /^>$/m); + assert.doesNotMatch(section, /> $/m); +}); + +test("notes: a long body is truncated with a pointer to the rest", () => { + const section = formatReleaseNotes({ + repository: "errata-ai/vale", + version: "3.21.0", + release: release(`${"line\n".repeat(200)}`), + limit: 100, + }); + assert.match(section, /Truncated at 100 characters/); + assert.match(section, /Read the rest at https:\/\/github\.com/); + assert.ok( + section.length < 400, + `expected the kept text to be bounded, got ${section.length} characters` + ); +}); + +test("notes: a single line over budget is cut rather than dropped", () => { + const section = formatReleaseNotes({ + repository: "errata-ai/vale", + version: "3.21.0", + release: release("x".repeat(500)), + limit: 100, + }); + assert.match(section, /^> x{98}$/m); + assert.match(section, /Truncated at 100 characters/); +}); + +test("notes: the default limit leaves room under GitHub's body cap", () => { + const section = formatReleaseNotes({ + repository: "errata-ai/vale", + version: "3.21.0", + // Worst case for the quoting: every line is one character, so every line + // also carries two characters of blockquote marker. + release: release(`${"x\n".repeat(NOTES_LIMIT)}`), + }); + assert.ok( + section.length < 65_536, + `a body of ${section.length} characters would be rejected by the API` + ); +}); + +/** + * Both cases render a section rather than nothing. "Upstream wrote no notes" + * and "we failed to fetch them" look identical when the section is simply + * absent, and only one of those is fine. + */ +test("notes: a release with an empty body says so", () => { + const section = formatReleaseNotes({ + repository: "errata-ai/vale", + version: "3.21.0", + release: release(" \n "), + }); + assert.match(section, /Upstream published no release notes/); +}); + +test("notes: no release at all falls back to the releases page", () => { + const section = formatReleaseNotes({ + repository: "ast-grep/ast-grep", + version: "0.45.3", + release: undefined, + }); + assert.match(section, /Upstream published no release notes/); + assert.match( + section, + /^https:\/\/github\.com\/ast-grep\/ast-grep\/releases$/m + ); +}); + +test("notes: a 404 is not an error, because the version comparison stands", async () => { + const previous = globalThis.fetch; + globalThis.fetch = async () => new Response("", { status: 404 }); + try { + assert.equal(await fetchLatestRelease("ast-grep/ast-grep"), undefined); + } finally { + globalThis.fetch = previous; + } +}); + +test("notes: any other API failure is an error", async () => { + const previous = globalThis.fetch; + globalThis.fetch = async () => new Response("", { status: 500 }); + try { + await assert.rejects( + fetchLatestRelease("ast-grep/ast-grep"), + /responded 500/ + ); + } finally { + globalThis.fetch = previous; + } +}); + +test("notes: the written file always ends in a newline", () => { + const directory = mkdtempSync(join(tmpdir(), "release-notes-test-")); + try { + const path = join(directory, "notes.md"); + writeNotesFile(path, "no trailing newline"); + assert.equal(readFileSync(path, "utf8"), "no trailing newline\n"); + } finally { + rmSync(directory, { recursive: true, force: true }); + } +}); + +/** + * Parsed here rather than in each detect script. All three parse the same flag + * for the same reason, and a partial fix — one script taught a new form, two + * not — would leave a workflow silently writing no notes. + */ +test("notes: --notes-out yields its path, and its absence yields nothing", () => { + assert.equal( + readNotesOut(["--write", "--notes-out", "/tmp/n.md"]), + "/tmp/n.md" + ); + assert.equal(readNotesOut(["--write"]), undefined); +}); + +test("notes: a flag where the path should be is a mistake, not a filename", () => { + // `--notes-out --write` would otherwise write to a file named `--write` and + // drop the flag meant to do the work. + assert.throws(() => readNotesOut(["--notes-out", "--write"]), /needs a path/); + assert.throws(() => readNotesOut(["--notes-out"]), /needs a path/); +}); diff --git a/.github/scripts/sg-detect.cjs b/.github/scripts/sg-detect.cjs index 2047daba..784914b4 100644 --- a/.github/scripts/sg-detect.cjs +++ b/.github/scripts/sg-detect.cjs @@ -35,12 +35,45 @@ * edit able to break this badge. * * Usage: - * node .github/scripts/sg-detect.cjs [--json] + * node .github/scripts/sg-detect.cjs [--json] [--write] [--notes-out ] * - * --json print `{ pinned, upstream, ahead }` and nothing else. This script - * never writes anything in either mode; unlike Vale there is no - * manifest to rewrite, and bumping eight dependency pins is a - * lockfile-touching change that belongs to a human. + * --json print `{ pinned, upstream, ahead }` and nothing else, writing + * nothing. This is what update-badges.cjs calls. + * + * --write rewrite every `@ast-grep/cli*` pin in packages/cli/package.json to + * the upstream version. `ast-grep-upgrade.yml` then regenerates the + * lockfile and opens a pull request. + * + * THIS REVERSES AN EARLIER DECISION, deliberately. This script used + * to refuse to write on the grounds that a lockfile-touching bump + * belongs to a human. The part of that which was right — nobody + * should merge a machine's dependency bump unread — is unchanged; + * the pull request is reviewed like any other. The part which was + * wrong is that refusing to WRITE does not make anyone review + * anything. It made the answer "upstream is ahead" reach only a + * README badge, so ast-grep 0.45.3 sat unpinned with nothing + * reporting it. A reviewed pull request is more review than no + * pull request, not less. + * + * What keeps it honest is that the write is mechanical and bounded: + * `bumpPins` replaces a version string in pins it can already + * enumerate, and refuses if the number it rewrote is not the number + * it found. It cannot add a dependency, reorder the file, or reflow + * it, so the diff a reviewer reads is N version strings and a + * lockfile. + * + * --notes-out + * write upstream's release notes for the newer version to , + * rendered as a Markdown section. `ast-grep-upgrade.yml` appends that + * file to the pull request body, so the bump arrives with what it + * actually contains rather than as eight changed version strings a + * reviewer has to go look up. Written only when upstream is ahead. + * + * The version comes from npm and the notes come from GitHub, which + * is the one place those two records can disagree: a tag missing + * from GitHub renders as a link to the releases page rather than + * failing the run, because the comparison this script exists to make + * has already succeeded by then. * * Outputs (appended to $GITHUB_OUTPUT when set): * update "true" when upstream is ahead @@ -48,9 +81,17 @@ * pinned_version the version currently pinned in packages/cli/package.json */ -const { appendFileSync, readFileSync } = require("node:fs"); +const { appendFileSync, readFileSync, writeFileSync } = require("node:fs"); const { join } = require("node:path"); +const { bumpPins } = require("./pin-bump.cjs"); +const { + fetchReleaseByTag, + formatReleaseNotes, + readNotesOut, + writeNotesFile, +} = require("./release-notes.cjs"); + const PACKAGE_JSON_PATH = join( __dirname, "..", @@ -67,6 +108,14 @@ const VERSION_PATTERN = /^(\d+)\.(\d+)\.(\d+)$/; const REGISTRY = "https://registry.npmjs.org"; +/** + * Where the release notes live. ast-grep tags its releases with the bare npm + * version (`0.45.3`, no `v`), so the version read from the registry is also the + * tag — an assumption this script does not have to be right about, since a tag + * it cannot find degrades to a link rather than to an error. + */ +const NOTES_REPOSITORY = "ast-grep/ast-grep"; + function setOutput(key, value) { const file = process.env.GITHUB_OUTPUT; if (file) { @@ -113,7 +162,7 @@ function isAhead(pinned, upstream) { * different ast-grep on one platform than on the others — and a badge that * smoothed it over would hide exactly the drift it was added to expose. */ -function collectPinnedVersion(packageJson) { +function collectPins(packageJson) { const pins = new Map(); for (const field of [ "dependencies", @@ -126,6 +175,11 @@ function collectPinnedVersion(packageJson) { } } } + return pins; +} + +function collectPinnedVersion(packageJson) { + const pins = collectPins(packageJson); if (pins.size === 0) { throw new Error( @@ -185,9 +239,19 @@ async function fetchLatestVersion(packageName) { async function main({ argv = process.argv.slice(2), latestVersion = fetchLatestVersion, - packageJson = JSON.parse(readFileSync(PACKAGE_JSON_PATH, "utf8")), + releaseFor = fetchReleaseByTag, + packageJsonPath = PACKAGE_JSON_PATH, + packageJson = JSON.parse(readFileSync(packageJsonPath, "utf8")), } = {}) { const json = argv.includes("--json"); + const write = argv.includes("--write"); + const notesOut = readNotesOut(argv); + // --json is what the badge run calls, and a badge run proposes nothing. + if (json && (notesOut || write)) { + throw new Error( + "--json prints the comparison and nothing else; it cannot be combined with --notes-out or --write" + ); + } const log = json ? () => {} : (line) => console.log(line); const pinned = collectPinnedVersion(packageJson); @@ -206,6 +270,48 @@ async function main({ console.log(JSON.stringify(comparison)); } + // Only the ahead path has anything to write. The pins are rewritten before + // the notes are fetched so that a network failure on the (optional) changelog + // cannot leave a half-done bump: by the time anything can throw below, the + // file on disk is either fully bumped or untouched. + if (write && ahead) { + const pins = collectPins(packageJson); + const source = readFileSync(packageJsonPath, "utf8"); + // The same constant collectPins enumerates with, so the two cannot + // disagree about what counts as a pin. + const { source: bumped, count } = bumpPins(source, { + pattern: PIN_PATTERN, + from: pinned, + to: upstream, + }); + // A straggler left at the old version is a different ast-grep on one + // platform than on the others, which is the exact state collectPinnedVersion + // refuses to report on. Better to fail here than to open that as a PR. + if (count !== pins.size) { + throw new Error( + `expected to rewrite ${pins.size} @ast-grep/cli* pins, rewrote ${count}` + ); + } + writeFileSync(packageJsonPath, bumped); + log(`Rewrote ${count} pins in ${packageJsonPath} to ${upstream}.`); + } + + // Only the ahead path has a bump to describe. A second request, unlike Vale's + // — that one reads its version out of a GitHub release and gets the notes in + // the same response, while this one learns the version from npm. + if (notesOut && ahead) { + const release = await releaseFor(NOTES_REPOSITORY, upstream); + writeNotesFile( + notesOut, + formatReleaseNotes({ + repository: NOTES_REPOSITORY, + version: upstream, + release, + }) + ); + log(`Wrote the upstream release notes to ${notesOut}.`); + } + setOutput("update", String(ahead)); setOutput("sg_version", upstream); setOutput("pinned_version", pinned); @@ -215,7 +321,7 @@ async function main({ // main() both prints and RETURNS the comparison, so update-badges.cjs can call // it in-process and read the answer as data. Nothing should ever parse the // human line above to recover a version that this return value already holds. -module.exports = { collectPinnedVersion, isAhead, main }; +module.exports = { collectPinnedVersion, collectPins, isAhead, main }; if (require.main === module) { main().catch((error) => { diff --git a/.github/scripts/sg-detect.test.cjs b/.github/scripts/sg-detect.test.cjs index 383d8794..7f3b4d6c 100644 --- a/.github/scripts/sg-detect.test.cjs +++ b/.github/scripts/sg-detect.test.cjs @@ -13,7 +13,7 @@ const test = require("node:test"); const assert = require("node:assert/strict"); -const { mkdtempSync, readFileSync, rmSync } = require("node:fs"); +const { mkdtempSync, readFileSync, rmSync, writeFileSync } = require("node:fs"); const { tmpdir } = require("node:os"); const { join } = require("node:path"); @@ -27,15 +27,35 @@ const CLI_PACKAGE_JSON = JSON.parse( ); /** Run main() with the registry stubbed and $GITHUB_OUTPUT captured. */ -async function runDetect({ upstream, packageJson, argv = [] }) { +async function runDetect({ + upstream, + packageJson, + packageJsonSource, + argv = [], + release, + wantNotes = false, +}) { const directory = mkdtempSync(join(tmpdir(), "sg-detect-test-")); const outputPath = join(directory, "github-output"); + const notesPath = join(directory, "release-notes.md"); + // --write rewrites the file on disk, so a test that exercises it needs a + // package.json of its own. The committed one is never written to here. + const packageJsonPath = join(directory, "package.json"); + if (packageJsonSource !== undefined) { + writeFileSync(packageJsonPath, packageJsonSource); + } const previous = process.env.GITHUB_OUTPUT; + const releasesFetched = []; process.env.GITHUB_OUTPUT = outputPath; try { const comparison = await main({ - argv, + argv: wantNotes ? [...argv, "--notes-out", notesPath] : argv, latestVersion: async () => upstream, + releaseFor: async (repository, tag) => { + releasesFetched.push(`${repository}@${tag}`); + return release; + }, + packageJsonPath, packageJson, }); const outputs = Object.fromEntries( @@ -47,7 +67,27 @@ async function runDetect({ upstream, packageJson, argv = [] }) { return [line.slice(0, at), line.slice(at + 1)]; }) ); - return { comparison, outputs }; + // Read before the finally below removes the directory. `undefined` means + // the script wrote nothing, which is a distinct answer from an empty file. + let notesWritten; + if (wantNotes) { + try { + notesWritten = readFileSync(notesPath, "utf8"); + } catch { + notesWritten = undefined; + } + } + const packageJsonWritten = + packageJsonSource === undefined + ? undefined + : readFileSync(packageJsonPath, "utf8"); + return { + comparison, + outputs, + notesWritten, + releasesFetched, + packageJsonWritten, + }; } finally { if (previous === undefined) { delete process.env.GITHUB_OUTPUT; @@ -169,3 +209,138 @@ test("sg-detect: ordering is numeric, not lexical", () => { assert.equal(isAhead("0.10.0", "0.9.0"), false); assert.equal(isAhead("1.0.0", "0.99.99"), false); }); + +test("sg-detect: --notes-out describes the bump upstream is proposing", async () => { + const { notesWritten, releasesFetched } = await runDetect({ + upstream: "0.45.3", + packageJson: pinnedAt("0.45.2"), + release: { + tag: "0.45.3", + notes: "- feat: add min-severity cli", + url: "https://github.com/ast-grep/ast-grep/releases/tag/0.45.3", + }, + wantNotes: true, + }); + + // The tag is the bare npm version, not a `v`-prefixed one. Getting that wrong + // degrades to a link rather than an error, so nothing else would catch it. + assert.deepEqual(releasesFetched, ["ast-grep/ast-grep@0.45.3"]); + assert.match(notesWritten, /^## Upstream release notes — 0\.45\.3$/m); + assert.match(notesWritten, /^> - feat: add min-severity cli$/m); +}); + +test("sg-detect: --notes-out writes nothing, and asks nothing, when the pin is current", async () => { + const { notesWritten, releasesFetched } = await runDetect({ + upstream: "0.45.2", + packageJson: pinnedAt("0.45.2"), + wantNotes: true, + }); + + assert.equal(notesWritten, undefined); + assert.deepEqual(releasesFetched, []); +}); + +/** + * The version comes from npm and the notes come from GitHub, so a tag npm has + * and GitHub does not is a real state. The comparison has already succeeded by + * then, and failing the run would throw that answer away. + */ +test("sg-detect: a version with no GitHub release still reports the bump", async () => { + const { notesWritten, outputs } = await runDetect({ + upstream: "0.45.3", + packageJson: pinnedAt("0.45.2"), + release: undefined, + wantNotes: true, + }); + + assert.equal(outputs.update, "true"); + assert.match(notesWritten, /Upstream published no release notes/); + assert.match( + notesWritten, + /^https:\/\/github\.com\/ast-grep\/ast-grep\/releases$/m + ); +}); + +test("sg-detect: --json refuses to be combined with --notes-out", async () => { + await assert.rejects( + runDetect({ + upstream: "0.45.3", + packageJson: pinnedAt("0.45.2"), + argv: ["--json"], + wantNotes: true, + }), + /cannot be combined with --notes-out/ + ); +}); + +/** A package.json shaped like the real one: prettier-formatted, mixed fields. */ +const sourcePinnedAt = (version) => + `${JSON.stringify( + { + name: "@taskless/cli", + dependencies: { "@ast-grep/cli": version, zod: "^4.0.0" }, + optionalDependencies: { + "@ast-grep/cli-darwin-arm64": version, + "@ast-grep/cli-linux-x64-gnu": version, + }, + }, + undefined, + 2 + )}\n`; + +test("sg-detect: --write bumps every pin in the file on disk", async () => { + const { packageJsonWritten, outputs } = await runDetect({ + upstream: "0.45.3", + packageJson: pinnedAt("0.45.2"), + packageJsonSource: sourcePinnedAt("0.45.2"), + argv: ["--write"], + }); + + assert.equal(outputs.update, "true"); + assert.doesNotMatch(packageJsonWritten, /0\.45\.2/); + assert.equal(packageJsonWritten.match(/0\.45\.3/g).length, 3); +}); + +test("sg-detect: --write leaves the file alone when the pin is current", async () => { + const before = sourcePinnedAt("0.45.2"); + const { packageJsonWritten } = await runDetect({ + upstream: "0.45.2", + packageJson: pinnedAt("0.45.2"), + packageJsonSource: before, + argv: ["--write"], + }); + + assert.equal(packageJsonWritten, before); +}); + +/** + * The half-applied bump this guard exists for. `packageJson` says there are + * three pins; the source text on disk only spells two of them at the old + * version, so a rewrite would leave a straggler behind — a different ast-grep + * on one platform than on the others. + */ +test("sg-detect: a pin the rewrite cannot reach fails the run", async () => { + await assert.rejects( + runDetect({ + upstream: "0.45.3", + packageJson: pinnedAt("0.45.2"), + packageJsonSource: sourcePinnedAt("0.45.2").replace( + '"@ast-grep/cli-linux-x64-gnu": "0.45.2"', + '"@ast-grep/cli-linux-x64-gnu": "0.45.1"' + ), + argv: ["--write"], + }), + /expected to rewrite 3 @ast-grep\/cli\* pins, rewrote 2/ + ); +}); + +test("sg-detect: --json refuses to be combined with --write", async () => { + await assert.rejects( + runDetect({ + upstream: "0.45.3", + packageJson: pinnedAt("0.45.2"), + argv: ["--json", "--write"], + }), + /cannot be combined with/ + ); +}); diff --git a/.github/scripts/vale-detect.cjs b/.github/scripts/vale-detect.cjs index 60a2ba60..72f76f0d 100644 --- a/.github/scripts/vale-detect.cjs +++ b/.github/scripts/vale-detect.cjs @@ -38,6 +38,18 @@ * pin, and skipping the checksums fetch is not a shortcut but the * point (nothing is being verified here). * + * --notes-out + * write upstream's release notes for the proposed version to , + * rendered as a Markdown section. The detect workflow appends that + * file to the pull request body, so a reviewer can see what the bump + * contains without leaving the pull request. Written only when + * upstream is ahead; there is nothing to describe otherwise. + * + * A path rather than a step output on purpose: release notes are + * third-party Markdown, and a $GITHUB_OUTPUT line is delimited text + * that a body containing the delimiter can break out of. A file + * passed to `--body-file` never meets an interpreter. + * * Outputs (appended to $GITHUB_OUTPUT when set): * update "true" when upstream is ahead * vale_version the upstream version @@ -47,6 +59,12 @@ const { appendFileSync, readFileSync, writeFileSync } = require("node:fs"); const { join } = require("node:path"); +const { + fetchLatestRelease, + formatReleaseNotes, + readNotesOut, + writeNotesFile, +} = require("./release-notes.cjs"); const { applyTemplate, assertManifest, @@ -65,31 +83,6 @@ function setOutput(key, value) { } } -/** - * GitHub's `releases/latest` deliberately excludes prereleases and drafts, so a - * Vale release candidate never trips detection. `GITHUB_TOKEN`, when present, - * is only for the API rate limit; the endpoint is public. - */ -async function fetchLatestTag(repository) { - const headers = { - accept: "application/vnd.github+json", - "user-agent": "taskless-skills-vale-detect", - }; - if (process.env.GITHUB_TOKEN) { - headers.authorization = `Bearer ${process.env.GITHUB_TOKEN}`; - } - const url = `https://api.github.com/repos/${repository}/releases/latest`; - const response = await fetch(url, { headers }); - if (!response.ok) { - throw new Error(`GET ${url} responded ${response.status}`); - } - const release = await response.json(); - if (typeof release.tag_name !== "string") { - throw new TypeError(`${url} returned no tag_name`); - } - return release.tag_name; -} - async function fetchText(url) { const response = await fetch(url, { redirect: "follow" }); if (!response.ok) { @@ -100,17 +93,24 @@ async function fetchText(url) { async function main({ argv = process.argv.slice(2), - latestTag = fetchLatestTag, + latestRelease = fetchLatestRelease, text = fetchText, } = {}) { const json = argv.includes("--json"); const write = argv.includes("--write"); + const notesOut = readNotesOut(argv); // --json is a reporting mode and --write is a writing one. Refusing the // combination beats silently dropping whichever flag loses, since the caller // that passed both is wrong about what it is asking for. if (json && write) { throw new Error("--json is read-only; it cannot be combined with --write"); } + // Same reasoning as above: --json reports a comparison and writes nothing. + if (json && notesOut) { + throw new Error( + "--json is read-only; it cannot be combined with --notes-out" + ); + } // Everything a human wants to read is noise on stdout when a caller is // reading structured output from it. const log = json ? () => {} : (line) => console.log(line); @@ -118,7 +118,16 @@ async function main({ JSON.parse(readFileSync(MANIFEST_PATH, "utf8")) ); - const upstreamTag = await latestTag(manifest.upstream.repository); + // GitHub's `releases/latest` deliberately excludes prereleases and drafts, so + // a Vale release candidate never trips detection. The same call carries the + // release notes, so describing the bump costs no extra request. + const release = await latestRelease(manifest.upstream.repository); + if (!release) { + throw new Error( + `${manifest.upstream.repository} has no published releases to compare against` + ); + } + const upstreamTag = release.tag; log(`pinned: ${manifest.valeVersion} upstream latest: ${upstreamTag}`); // Decide whether to go on with the two pure predicates directly, rather than @@ -177,6 +186,20 @@ async function main({ console.log("\nPass --write to update the manifest."); } + // What the bump actually contains, for whoever reviews the digests. Written + // only on the ahead path: the other paths propose nothing to describe. + if (notesOut) { + writeNotesFile( + notesOut, + formatReleaseNotes({ + repository: manifest.upstream.repository, + version: plan.upstreamVersion, + release, + }) + ); + console.log(`Wrote the upstream release notes to ${notesOut}.`); + } + setOutput("update", "true"); setOutput("vale_version", plan.upstreamVersion); setOutput("pinned_version", plan.pinnedVersion); diff --git a/.github/scripts/vale-detect.test.cjs b/.github/scripts/vale-detect.test.cjs index d0a6c070..5f34357c 100644 --- a/.github/scripts/vale-detect.test.cjs +++ b/.github/scripts/vale-detect.test.cjs @@ -49,16 +49,28 @@ function checksumsFor(version) { * Run main() with both fetches stubbed and $GITHUB_OUTPUT pointed at a temp * file, then return the parsed step outputs plus which URLs were fetched. */ -async function runDetect({ upstreamTag, checksums, argv = [] }) { +async function runDetect({ + upstreamTag, + notes = "", + checksums, + argv = [], + wantNotes = false, +}) { const directory = mkdtempSync(join(tmpdir(), "vale-detect-test-")); const outputPath = join(directory, "github-output"); + const notesPath = join(directory, "release-notes.md"); + const fullArgv = wantNotes ? [...argv, "--notes-out", notesPath] : argv; const previous = process.env.GITHUB_OUTPUT; const fetched = []; process.env.GITHUB_OUTPUT = outputPath; try { const comparison = await main({ - argv, - latestTag: async () => upstreamTag, + argv: fullArgv, + latestRelease: async () => ({ + tag: upstreamTag, + notes, + url: `https://github.com/${MANIFEST.upstream.repository}/releases/tag/${upstreamTag}`, + }), text: async (url) => { fetched.push(url); return checksums; @@ -73,7 +85,17 @@ async function runDetect({ upstreamTag, checksums, argv = [] }) { return [line.slice(0, at), line.slice(at + 1)]; }) ); - return { comparison, outputs, fetched }; + // Read before the finally below removes the directory. `undefined` means + // the script wrote nothing, which is a distinct answer from an empty file. + let notesWritten; + if (wantNotes) { + try { + notesWritten = readFileSync(notesPath, "utf8"); + } catch { + notesWritten = undefined; + } + } + return { comparison, outputs, fetched, notesWritten }; } finally { if (previous === undefined) { delete process.env.GITHUB_OUTPUT; @@ -176,3 +198,70 @@ test("detect: a checksums file missing a platform aborts", async () => { /publishes no asset named/ ); }); + +test("detect: --notes-out carries upstream's release notes for the proposal", async () => { + const { notesWritten } = await runDetect({ + upstreamTag: "v3.99.0", + notes: "## Fixed\n\nA thing that was broken.", + checksums: checksumsFor("3.99.0"), + wantNotes: true, + }); + + assert.match(notesWritten, /^## Upstream release notes — 3\.99\.0$/m); + assert.match(notesWritten, /^> A thing that was broken\.$/m); +}); + +/** + * The no-op path proposes nothing, so there is nothing to describe. Writing a + * section anyway would leave the previous run's notes on disk for a workflow + * step that only checks whether the file exists. + */ +test("detect: --notes-out writes nothing when upstream is not ahead", async () => { + const { notesWritten } = await runDetect({ + upstreamTag: `v${MANIFEST.valeVersion}`, + notes: "Should not be written.", + checksums: "", + wantNotes: true, + }); + + assert.equal(notesWritten, undefined); +}); + +test("detect: --json refuses to be combined with --notes-out", async () => { + await assert.rejects( + runDetect({ + upstreamTag: "v3.99.0", + checksums: "", + argv: ["--json"], + wantNotes: true, + }), + /--json is read-only/ + ); +}); + +test('detect: --notes-out without a path aborts rather than writing to "--write"', async () => { + await assert.rejects( + runDetect({ + upstreamTag: "v3.99.0", + checksums: checksumsFor("3.99.0"), + argv: ["--notes-out", "--write"], + }), + /--notes-out needs a path/ + ); +}); + +/** + * A repository with no published GitHub release at all. The comparison has + * nothing to compare against, so failing loudly is the only honest answer — + * reporting "not ahead" would read as "we are current" forever. + */ +test("detect: a repository with no releases aborts rather than reporting current", async () => { + await assert.rejects( + main({ + argv: [], + latestRelease: async () => undefined, + text: async () => "", + }), + /has no published releases/ + ); +}); diff --git a/.github/scripts/vale-upgrade-detect.cjs b/.github/scripts/vale-upgrade-detect.cjs new file mode 100644 index 00000000..b0049ca8 --- /dev/null +++ b/.github/scripts/vale-upgrade-detect.cjs @@ -0,0 +1,279 @@ +#!/usr/bin/env node +// SPDX-License-Identifier: MIT +"use strict"; + +/** + * Vale — the CONSUMER half of the vendoring pipeline. + * + * THE TWO STAGES, and why this one was missing. `vale-detect.cjs` watches + * upstream Vale and proposes a new manifest; merging that republishes the six + * `@taskless/vale-*` platform packages at `-`. That is the + * PRODUCER stage, and release-vale.yml says plainly what it does not do: + * "publishing a platform package changes no consumer, because the CLI pins each + * one exactly and a new version reaches a user only when someone reviews a bump + * to that pin." + * + * That review has happened — by hand, every time. `git log` on the pins shows + * 3.18.0, 3.19.0 and 3.20.0 each moved in a separate manual commit. So the + * stage is not missing so much as unautomated, and what it costs is not a stale + * pin but a dependency on somebody remembering: nothing detects that a publish + * has landed and the pins are now behind, and nothing puts upstream's changelog + * in front of whoever bumps them. + * + * This script is that detection. It asks whether a newer platform set exists on + * npm and, with `--write`, moves every pin to it. + * + * WHY npm AND NOT THE MANIFEST. The manifest records what we intend to publish; + * npm records what was actually published. Between the two sits a publish job + * that can fail, and a pin bumped to a version npm does not serve is a broken + * install rather than a stale one. The honest question for a consumer-side + * upgrade is "what can be installed", so the registry is the source. + * + * WHY ALL SIX ARE READ, not one as a representative. The publish loop attempts + * every package and reports failures at the end precisely because a partial set + * is the one state the CLI's exact pins cannot tolerate — some platforms + * resolvable, others not. Reading one package would upgrade the pins into that + * state without noticing. Reading six costs six cheap requests and turns it + * into a failed run. + * + * WHY THE COMPARISON IS vale-release.cjs's. Unlike sg-detect.cjs — which keeps + * its own comparator so an ast-grep oddity cannot surface as an error message + * about Vale — these versions really are Vale stamped versions, so + * `compareStampedVersions` is both correct and the one place that already knows + * a stamp's ordering rules. + * + * Usage: + * node .github/scripts/vale-upgrade-detect.cjs [--json] [--write] [--notes-out ] + * + * Outputs (appended to $GITHUB_OUTPUT when set): + * update "true" when a newer published set exists + * vale_version the newer stamped version + * pinned_version the stamped version currently pinned + * base_version the plain Vale version inside the newer stamp + */ + +const { appendFileSync, readFileSync, writeFileSync } = require("node:fs"); +const { join } = require("node:path"); + +const { bumpPins } = require("./pin-bump.cjs"); +const { + fetchReleaseByTag, + formatReleaseNotes, + readNotesOut, + writeNotesFile, +} = require("./release-notes.cjs"); +const { + assertManifest, + assertStampedVersion, + compareStampedVersions, +} = require("./vale-release.cjs"); + +const PACKAGE_JSON_PATH = join( + __dirname, + "..", + "..", + "packages", + "cli", + "package.json" +); + +const MANIFEST_PATH = join(__dirname, "vale-manifest.json"); + +const PIN_PREFIX = "@taskless/vale-"; + +/** + * What counts as a platform pin. Used BOTH to enumerate the pins and to rewrite + * them, so the two cannot drift apart — the trailing hyphen is the boundary + * here, since there is no bare `@taskless/vale` package. + */ +const PIN_PATTERN = /^@taskless\/vale-/; + +const REGISTRY = "https://registry.npmjs.org"; + +function setOutput(key, value) { + const file = process.env.GITHUB_OUTPUT; + if (file) { + appendFileSync(file, `${key}=${value}\n`); + } +} + +/** + * The one stamped version every `@taskless/vale-*` pin holds. + * + * Disagreement is an error for the same reason it is in sg-detect.cjs: the + * platform packages are selected by optional dependency, so pins that disagree + * are a different Vale on one platform than on the others. There is no version + * this script could honestly report for that state, and picking the highest + * would paper over exactly the drift worth surfacing. + */ +function collectPins(packageJson) { + const pins = new Map(); + for (const field of [ + "dependencies", + "devDependencies", + "optionalDependencies", + ]) { + for (const [name, range] of Object.entries(packageJson[field] ?? {})) { + if (PIN_PATTERN.test(name)) { + pins.set(name, range); + } + } + } + if (pins.size === 0) { + throw new Error( + `packages/cli/package.json declares no ${PIN_PREFIX}* dependency` + ); + } + const versions = new Set(pins.values()); + if (versions.size > 1) { + const detail = [...pins] + .map(([name, range]) => `${name}@${range}`) + .sort() + .join(", "); + throw new Error( + `${PIN_PREFIX}* pins disagree, so there is no single version to upgrade from: ${detail}` + ); + } + const [version] = versions; + assertStampedVersion(version); + return { pins, version }; +} + +/** What `npm install ` would resolve to today. */ +async function fetchLatestVersion(packageName) { + const url = `${REGISTRY}/${packageName.replace("/", "%2F")}`; + const response = await fetch(url, { + headers: { + accept: "application/vnd.npm.install-v1+json", + "user-agent": "taskless-vale-upgrade-detect", + }, + }); + if (!response.ok) { + throw new Error(`GET ${url} responded ${response.status}`); + } + const packument = await response.json(); + const latest = packument["dist-tags"]?.latest; + if (typeof latest !== "string") { + throw new TypeError(`${url} returned no dist-tags.latest`); + } + return latest; +} + +/** + * The version the whole set is published at. + * + * A set that does not agree is a half-finished publish, and upgrading into it + * would pin some platforms to a version npm cannot serve. release-vale.yml's + * publish loop is built to make this rare and re-runnable; this refuses to + * build on it while it is true. + */ +async function resolvePublishedVersion(names, latestVersion) { + const published = new Map( + await Promise.all( + names.map(async (name) => [name, await latestVersion(name)]) + ) + ); + const versions = new Set(published.values()); + if (versions.size > 1) { + const detail = [...published] + .map(([name, version]) => `${name}@${version}`) + .sort() + .join(", "); + throw new Error( + `the published ${PIN_PREFIX}* set is not at one version, so it is mid-publish or partially failed: ${detail}` + ); + } + const [version] = versions; + assertStampedVersion(version); + return version; +} + +/** `3.21.0-20260914012345` -> `3.21.0`, the release upstream actually tagged. */ +function baseVersion(stamped) { + return assertStampedVersion(stamped).split("-")[0]; +} + +async function main({ + argv = process.argv.slice(2), + latestVersion = fetchLatestVersion, + releaseFor = fetchReleaseByTag, + packageJsonPath = PACKAGE_JSON_PATH, + packageJson = JSON.parse(readFileSync(packageJsonPath, "utf8")), + manifest = assertManifest(JSON.parse(readFileSync(MANIFEST_PATH, "utf8"))), +} = {}) { + const json = argv.includes("--json"); + const write = argv.includes("--write"); + const notesOut = readNotesOut(argv); + if (json && (write || notesOut)) { + throw new Error( + "--json prints the comparison and nothing else; it cannot be combined with --write or --notes-out" + ); + } + const log = json ? () => {} : (line) => console.log(line); + + const { pins, version: pinned } = collectPins(packageJson); + const upstream = await resolvePublishedVersion( + [...pins.keys()].sort(), + latestVersion + ); + const ahead = compareStampedVersions(upstream, pinned) > 0; + + log(`pinned: ${pinned} published latest: ${upstream}`); + log( + ahead + ? `A newer platform set is published. Move all ${pins.size} pins together.` + : "The pins are current with what is published. Nothing to do." + ); + + const comparison = { pinned, upstream, ahead }; + if (json) { + console.log(JSON.stringify(comparison)); + } + + if (write && ahead) { + const source = readFileSync(packageJsonPath, "utf8"); + const { source: bumped, count } = bumpPins(source, { + pattern: PIN_PATTERN, + from: pinned, + to: upstream, + }); + if (count !== pins.size) { + throw new Error( + `expected to rewrite ${pins.size} ${PIN_PREFIX}* pins, rewrote ${count}` + ); + } + writeFileSync(packageJsonPath, bumped); + log(`Rewrote ${count} pins in ${packageJsonPath} to ${upstream}.`); + } + + // The changelog a reviewer wants is UPSTREAM's, not ours. Our stamp says when + // the package was built; `v` is the release whose behaviour changes. + if (notesOut && ahead) { + const base = baseVersion(upstream); + const release = await releaseFor(manifest.upstream.repository, `v${base}`); + writeNotesFile( + notesOut, + formatReleaseNotes({ + repository: manifest.upstream.repository, + version: base, + release, + }) + ); + log(`Wrote the upstream release notes to ${notesOut}.`); + } + + setOutput("update", String(ahead)); + setOutput("vale_version", upstream); + setOutput("pinned_version", pinned); + setOutput("base_version", baseVersion(upstream)); + return comparison; +} + +module.exports = { baseVersion, collectPins, main, resolvePublishedVersion }; + +if (require.main === module) { + main().catch((error) => { + console.error(`\nvale-upgrade-detect failed: ${error.message}`); + process.exitCode = 1; + }); +} diff --git a/.github/scripts/vale-upgrade-detect.test.cjs b/.github/scripts/vale-upgrade-detect.test.cjs new file mode 100644 index 00000000..94e4b261 --- /dev/null +++ b/.github/scripts/vale-upgrade-detect.test.cjs @@ -0,0 +1,227 @@ +// SPDX-License-Identifier: MIT +"use strict"; + +/** + * Tests for vale-upgrade-detect.cjs — the consumer half of the Vale pipeline. + * + * The state worth guarding against here is not "we missed a release". It is + * "we upgraded into a half-published set", which produces pins that resolve on + * some platforms and 404 on others, and which no amount of local testing on one + * machine would reveal. + */ + +const test = require("node:test"); +const assert = require("node:assert/strict"); +const { mkdtempSync, readFileSync, rmSync, writeFileSync } = require("node:fs"); +const { tmpdir } = require("node:os"); +const { join } = require("node:path"); + +const { baseVersion, collectPins, main } = require("./vale-upgrade-detect.cjs"); + +const MANIFEST = { upstream: { repository: "errata-ai/vale" } }; + +const PLATFORMS = [ + "@taskless/vale-darwin-arm64", + "@taskless/vale-darwin-x64", + "@taskless/vale-linux-arm64", + "@taskless/vale-linux-x64", + "@taskless/vale-win32-arm64", + "@taskless/vale-win32-x64", +]; + +const pinnedAt = (version) => ({ + optionalDependencies: Object.fromEntries( + PLATFORMS.map((name) => [name, version]) + ), +}); + +const sourcePinnedAt = (version) => + `${JSON.stringify( + { + name: "@taskless/cli", + ...pinnedAt(version), + dependencies: { zod: "^4" }, + }, + undefined, + 2 + )}\n`; + +async function run({ + packageJson, + packageJsonSource, + published, + argv = [], + release, + wantNotes = false, +}) { + const directory = mkdtempSync(join(tmpdir(), "vale-upgrade-test-")); + const outputPath = join(directory, "github-output"); + const notesPath = join(directory, "release-notes.md"); + const packageJsonPath = join(directory, "package.json"); + if (packageJsonSource !== undefined) { + writeFileSync(packageJsonPath, packageJsonSource); + } + const previous = process.env.GITHUB_OUTPUT; + const tagsFetched = []; + process.env.GITHUB_OUTPUT = outputPath; + try { + const comparison = await main({ + argv: wantNotes ? [...argv, "--notes-out", notesPath] : argv, + latestVersion: async (name) => + typeof published === "string" ? published : published[name], + releaseFor: async (repository, tag) => { + tagsFetched.push(`${repository}@${tag}`); + return release; + }, + packageJsonPath, + packageJson, + manifest: MANIFEST, + }); + const outputs = Object.fromEntries( + readFileSync(outputPath, "utf8") + .split("\n") + .filter(Boolean) + .map((line) => { + const at = line.indexOf("="); + return [line.slice(0, at), line.slice(at + 1)]; + }) + ); + let notesWritten; + if (wantNotes) { + try { + notesWritten = readFileSync(notesPath, "utf8"); + } catch { + notesWritten = undefined; + } + } + const written = + packageJsonSource === undefined + ? undefined + : readFileSync(packageJsonPath, "utf8"); + return { comparison, outputs, notesWritten, tagsFetched, written }; + } finally { + if (previous === undefined) { + delete process.env.GITHUB_OUTPUT; + } else { + process.env.GITHUB_OUTPUT = previous; + } + rmSync(directory, { recursive: true, force: true }); + } +} + +test("a newer published set is an upgrade", async () => { + const { outputs } = await run({ + packageJson: pinnedAt("3.20.0-20260907164938"), + published: "3.21.0-20260914010203", + }); + + assert.equal(outputs.update, "true"); + assert.equal(outputs.vale_version, "3.21.0-20260914010203"); + assert.equal(outputs.pinned_version, "3.20.0-20260907164938"); + // The plain version is what the changelog and the release note talk about. + assert.equal(outputs.base_version, "3.21.0"); +}); + +test("the pins already matching what is published is a no-op", async () => { + const { outputs } = await run({ + packageJson: pinnedAt("3.20.0-20260907164938"), + published: "3.20.0-20260907164938", + }); + assert.equal(outputs.update, "false"); +}); + +/** + * Two stamps of the SAME upstream Vale version. Ordering by the base version + * alone would call this equal and never ship a republish — which is a real + * event here, since a repackaging fix keeps the upstream version and moves only + * the stamp. + */ +test("a newer stamp of the same Vale version is still an upgrade", async () => { + const { outputs } = await run({ + packageJson: pinnedAt("3.20.0-20260907164938"), + published: "3.20.0-20260908000000", + }); + assert.equal(outputs.update, "true"); + assert.equal(outputs.base_version, "3.20.0"); +}); + +/** + * The failure this script exists to refuse. release-vale.yml publishes six + * packages in a loop and can leave the set split across two versions if a + * publish fails partway; upgrading into that pins some platforms to something + * npm does not serve. + */ +test("a half-published set aborts rather than upgrading into it", async () => { + await assert.rejects( + run({ + packageJson: pinnedAt("3.20.0-20260907164938"), + published: Object.fromEntries( + PLATFORMS.map((name, index) => [ + name, + index === 0 ? "3.21.0-20260914010203" : "3.20.0-20260907164938", + ]) + ), + }), + /not at one version, so it is mid-publish or partially failed/ + ); +}); + +test("pins that disagree with each other abort", () => { + assert.throws( + () => + collectPins({ + optionalDependencies: { + "@taskless/vale-darwin-arm64": "3.20.0-20260907164938", + "@taskless/vale-linux-x64": "3.19.0-20260901000817", + }, + }), + /pins disagree/ + ); +}); + +test("an unstamped pin aborts, because it could never have been published", () => { + assert.throws( + () => + collectPins({ + optionalDependencies: { "@taskless/vale-linux-x64": "3.20.0" }, + }), + /not a stamped version/ + ); +}); + +test("--write moves every pin and nothing else", async () => { + const { written } = await run({ + packageJson: pinnedAt("3.20.0-20260907164938"), + packageJsonSource: sourcePinnedAt("3.20.0-20260907164938"), + published: "3.21.0-20260914010203", + argv: ["--write"], + }); + + assert.equal(written.match(/3\.21\.0-20260914010203/g).length, 6); + assert.doesNotMatch(written, /3\.20\.0-20260907164938/); + assert.match(written, /"zod": "\^4"/); +}); + +test("the changelog is upstream's, fetched by the BASE version's tag", async () => { + const { notesWritten, tagsFetched } = await run({ + packageJson: pinnedAt("3.20.0-20260907164938"), + published: "3.21.0-20260914010203", + release: { + tag: "v3.21.0", + notes: "## `doc(...)` selections", + url: "https://github.com/errata-ai/vale/releases/tag/v3.21.0", + }, + wantNotes: true, + }); + + // Not the stamped version, which upstream has never heard of, and not a bare + // `3.21.0`, which is not how Vale tags. + assert.deepEqual(tagsFetched, ["errata-ai/vale@v3.21.0"]); + assert.match(notesWritten, /^## Upstream release notes — 3\.21\.0$/m); + assert.match(notesWritten, /> ## `doc\(\.\.\.\)` selections/); +}); + +test("base version strips the stamp", () => { + assert.equal(baseVersion("3.21.0-20260914010203"), "3.21.0"); + assert.throws(() => baseVersion("3.21.0"), /not a stamped version/); +}); diff --git a/.github/scripts/vendor-pr.cjs b/.github/scripts/vendor-pr.cjs new file mode 100644 index 00000000..a69f115c --- /dev/null +++ b/.github/scripts/vendor-pr.cjs @@ -0,0 +1,289 @@ +#!/usr/bin/env node +// SPDX-License-Identifier: MIT +"use strict"; + +/** + * Maintain the rolling pull request for a vendored toolchain bump. + * + * Both detect workflows — Vale's and ast-grep's — arrive here with the same + * shape: a working tree that already carries the proposed bump, a title, and a + * body. What happens next is identical for both, so it lives in one place + * rather than as two copies of the same shell that drift. + * + * ONE STATIC BRANCH PER ENGINE, `vendor/`. The alternative, a branch + * per upstream version, means a second release while the first is unreviewed + * opens a SECOND pull request proposing a conflicting edit to the same lines, + * and someone has to notice and close the stale one. A rolling branch instead + * carries whatever upstream's current answer is: the proposal is rewritten, the + * title and body are rewritten with it, and there is exactly one thing to + * review. It also means a reviewer who comes back after a week is not looking + * at a bump that upstream has already superseded. + * + * THE BRANCH IS REBUILT FROM `main`, NOT APPENDED TO (`checkout -B`). The + * proposal is "main plus this bump", and it has to stay that as `main` moves. + * Appending would accumulate one commit per upstream release and slowly turn + * the diff into a history of versions nobody pinned. + * + * WHICH IS WHY FORCE-PUSHING NEEDS TWO GUARDS, because rebuilding a branch + * somebody may be reviewing is the obvious way this hurts someone: + * + * - Nothing is pushed when the rebuilt branch matches what is already on the + * remote. A scheduled run that finds the same upstream version must be + * inert, or every run rewrites the branch, and every rewrite dismisses + * approvals and re-triggers CI on an unchanged proposal. + * + * - Nothing is pushed when the remote branch carries a commit this workflow + * did not write. A reviewer who pushes a fixup onto the branch has done the + * most reasonable thing available to them, and a force-push would silently + * delete it. The run warns and stops instead, which costs a stale proposal + * until someone looks — strictly better than costing someone's work. + * + * Usage: + * node .github/scripts/vendor-pr.cjs \ + * --branch vendor/vale \ + * --title "chore(vale): pin Vale 3.21.0" \ + * --body-file /path/to/body.md \ + * --message "chore(vale): pin Vale 3.21.0" \ + * [--label skip-changeset] \ + * -- packages/cli/package.json pnpm-lock.yaml + * + * Everything after `--` is the set of paths to stage. They are passed to git as + * an argv array, never through a shell, so a path is a path even if it contains + * something a shell would find interesting. + */ + +const { execFileSync } = require("node:child_process"); + +/** Commits with this author are ours to overwrite. Anything else is not. */ +const BOT_AUTHOR = "github-actions[bot]"; + +const BOT_EMAIL = "41898282+github-actions[bot]@users.noreply.github.com"; + +function parseArgs(argv) { + const separator = argv.indexOf("--"); + const flags = separator === -1 ? argv : argv.slice(0, separator); + const paths = separator === -1 ? [] : argv.slice(separator + 1); + + const read = (name) => { + const at = flags.indexOf(name); + if (at === -1) { + return undefined; + } + const value = flags[at + 1]; + if (!value || value.startsWith("--")) { + throw new Error(`${name} needs a value`); + } + return value; + }; + + const options = { + branch: read("--branch"), + title: read("--title"), + bodyFile: read("--body-file"), + message: read("--message"), + label: read("--label"), + paths, + }; + + for (const required of ["branch", "title", "bodyFile", "message"]) { + if (!options[required]) { + throw new Error( + `--${required.replace(/[A-Z]/g, "-$&").toLowerCase()} is required` + ); + } + } + if (paths.length === 0) { + throw new Error("no paths to stage were given after `--`"); + } + return options; +} + +/** + * Whether anyone but this workflow wrote what is on the branch. + * + * Takes author lines rather than running git itself, because this is the + * decision worth testing and it is a decision about a list of strings. An empty + * list — no branch yet — is not foreign. + */ +function hasForeignCommits(authorLines) { + return authorLines + .map((line) => line.trim()) + .filter((line) => line.length > 0) + .some((author) => author !== BOT_AUTHOR); +} + +const runner = + (command) => + (args, { allowFailure = false } = {}) => { + try { + return execFileSync(command, args, { encoding: "utf8" }).trim(); + } catch (error) { + if (allowFailure) { + return undefined; + } + const detail = error.stderr?.toString().trim() ?? error.message; + throw new Error(`${command} ${args.join(" ")} failed: ${detail}`); + } + }; + +async function main({ + argv = process.argv.slice(2), + git = runner("git"), + gh = runner("gh"), + log = (line) => console.log(line), +} = {}) { + const { branch, title, bodyFile, message, label, paths } = parseArgs(argv); + + git(["config", "user.name", BOT_AUTHOR]); + git(["config", "user.email", BOT_EMAIL]); + + // -B rather than -b: the branch is rebuilt from wherever HEAD is (the freshly + // checked-out `main`), so the proposal is always "main plus this bump". + git(["checkout", "-B", branch]); + git(["add", "--", ...paths]); + + // `diff --cached --quiet` exits non-zero when something IS staged, so a + // successful run here means the working tree carried no bump after all. + const nothingStaged = + git(["diff", "--cached", "--quiet"], { allowFailure: true }) !== undefined; + if (nothingStaged) { + log("Nothing staged; there is no bump to propose."); + return { action: "none", reason: "nothing-staged" }; + } + git(["commit", "-m", message]); + + const remoteExists = + git(["ls-remote", "--exit-code", "--heads", "origin", branch], { + allowFailure: true, + }) !== undefined; + + // The SHA the guards below are reasoning about, kept so the push can lease + // against it. See the push itself for why that matters. + let fetchedTip; + + if (remoteExists) { + git(["fetch", "--quiet", "origin", branch]); + fetchedTip = git(["rev-parse", "FETCH_HEAD"]); + + // The TIP author, not every commit since `main`. Asking "which commits are + // on the branch and not on main" needs a merge base, and `actions/checkout` + // clones at depth 1, so there is none — the question would answer wrongly + // or not at all depending on clone depth, which is the worst property a + // safety guard can have. The tip is sufficient here because this script is + // the only thing that ever writes the branch, and it does so by force-push: + // there is no path by which a bot commit lands ON TOP of a human's. + const tipAuthor = git(["log", "-1", "--format=%an", "FETCH_HEAD"]) ?? ""; + if (hasForeignCommits([tipAuthor])) { + log( + `::warning::${branch} was last written by ${tipAuthor}, not this workflow; refusing to force-push over that.` + ); + return { action: "none", reason: "foreign-commits" }; + } + + // By content, and only the content this run proposes. + // + // Comparing SHAs would always say "changed", because the rebuild reparents + // onto whatever `main` is now. Comparing whole TREES is subtler and was + // wrong in the same direction: an unrelated commit on `main` makes the + // trees differ, so an upstream that had not moved still force-pushed the + // branch and rewrote the pull request — dismissing approvals and + // restarting CI over somebody else's commit to a different file. + // + // The question that matters is "does the branch already propose exactly + // this bump", so the diff is scoped to the paths being proposed. Falling + // behind `main` is a real thing that happens to this branch, but it is + // branch protection's business and one click to resolve, not a reason to + // rewrite a proposal nobody changed. + const identical = + git(["diff", "--quiet", "FETCH_HEAD", "HEAD", "--", ...paths], { + allowFailure: true, + }) !== undefined; + if (identical) { + log(`${branch} already proposes exactly this; leaving it alone.`); + return { action: "none", reason: "unchanged" }; + } + } + + // --force-with-lease, not --force, and the distinction is the whole point of + // the guards above. + // + // Reading the tip author and then force-pushing is a check and an action with + // a gap between them. A reviewer who pushes a fixup inside that gap has their + // commit destroyed silently — precisely the outcome the ownership guard + // exists to prevent, arrived at through timing rather than through logic. A + // guard that a race defeats is not a guard. + // + // The lease closes it by making the push itself assert what the guards + // assumed: the branch is still the commit we inspected. If it is not, the + // push is REJECTED and the run fails loudly, which is a report rather than a + // loss. The SHA is explicit rather than implied by a remote-tracking ref, + // which also sidesteps the `stale info` failure a shallow clone produces + // (see the shallow-clone note in CLAUDE.md) — `actions/checkout` clones at + // depth 1, so there may be no tracking ref to lease against. + // + // A branch that does not exist yet has nothing to lease and nothing to + // overwrite, so it takes an ordinary push. Forcing there would be asserting + // a claim about a ref that is not there. + git( + remoteExists + ? ["push", `--force-with-lease=${branch}:${fetchedTip}`, "origin", branch] + : ["push", "origin", branch] + ); + + const existing = gh([ + "pr", + "list", + "--head", + branch, + "--state", + "open", + "--json", + "number", + "--jq", + ".[].number", + ]); + + if (existing) { + // `gh pr edit` goes through GraphQL, which this repository has had broken + // out from under it by the Projects (classic) deprecation. REST does not + // depend on it. `-F body=@file` reads the file rather than passing its + // contents as an argument, so third-party release notes never become argv. + gh([ + "api", + "-X", + "PATCH", + `repos/{owner}/{repo}/pulls/${existing}`, + "-f", + `title=${title}`, + "-F", + `body=@${bodyFile}`, + ]); + log(`Updated #${existing}: ${title}`); + return { action: "updated", pr: Number(existing) }; + } + + gh([ + "pr", + "create", + "--base", + "main", + "--head", + branch, + "--title", + title, + "--body-file", + bodyFile, + ...(label ? ["--label", label] : []), + ]); + log(`Opened a pull request on ${branch}: ${title}`); + return { action: "created" }; +} + +module.exports = { BOT_AUTHOR, hasForeignCommits, main, parseArgs }; + +if (require.main === module) { + main().catch((error) => { + console.error(`\nvendor-pr failed: ${error.message}`); + process.exitCode = 1; + }); +} diff --git a/.github/scripts/vendor-pr.test.cjs b/.github/scripts/vendor-pr.test.cjs new file mode 100644 index 00000000..b9215aa5 --- /dev/null +++ b/.github/scripts/vendor-pr.test.cjs @@ -0,0 +1,293 @@ +// SPDX-License-Identifier: MIT +"use strict"; + +/** + * Tests for vendor-pr.cjs. + * + * The argument parsing and the force-push guard are pure and tested directly. + * Everything else is tested by driving main() with `git` and `gh` replaced by + * recorders, which is the only way to reach the ORDER of operations — and the + * order is where the damage lives. Pushing before checking who owns the branch, + * or creating a second pull request instead of updating the open one, are both + * states every individual step would report as healthy. + */ + +const test = require("node:test"); +const assert = require("node:assert/strict"); + +const { + BOT_AUTHOR, + hasForeignCommits, + main, + parseArgs, +} = require("./vendor-pr.cjs"); + +const ARGV = [ + "--branch", + "vendor/vale", + "--title", + "chore(vale): pin Vale 3.21.0", + "--body-file", + "/tmp/body.md", + "--message", + "chore(vale): pin Vale 3.21.0", + "--", + ".github/scripts/vale-manifest.json", +]; + +/** + * Drive main() with both commands recorded. + * + * `git` answers are keyed by the subcommand plus enough of its arguments to be + * unambiguous. `undefined` from the runner means the command failed, which is + * how the real one reports a non-zero exit under `allowFailure`. + */ +function harness({ gitAnswers = {}, ghAnswers = {} } = {}) { + const calls = []; + const answer = (table, args, fallback) => { + for (const [key, value] of Object.entries(table)) { + if (args.join(" ").startsWith(key)) { + return value; + } + } + return fallback; + }; + return { + calls, + git: (args) => { + calls.push(["git", ...args]); + return answer(gitAnswers, args, ""); + }, + gh: (args) => { + calls.push(["gh", ...args]); + return answer(ghAnswers, args, ""); + }, + log: () => {}, + }; +} + +const ran = (calls, command, ...prefix) => + calls.some( + ([name, ...args]) => + name === command && prefix.every((part, index) => args[index] === part) + ); + +test("args: paths come after `--` and flags before it", () => { + const options = parseArgs(ARGV); + assert.equal(options.branch, "vendor/vale"); + assert.deepEqual(options.paths, [".github/scripts/vale-manifest.json"]); + assert.equal(options.label, undefined); +}); + +test("args: a missing required flag aborts", () => { + assert.throws(() => parseArgs(ARGV.slice(2)), /--branch is required/); +}); + +test("args: no paths at all aborts rather than committing nothing", () => { + assert.throws( + () => parseArgs(ARGV.slice(0, ARGV.indexOf("--"))), + /no paths to stage/ + ); +}); + +test("guard: the workflow's own commits are not foreign", () => { + assert.equal(hasForeignCommits([BOT_AUTHOR]), false); + assert.equal(hasForeignCommits([]), false); + assert.equal(hasForeignCommits([""]), false); +}); + +test("guard: anyone else's commit is foreign", () => { + assert.equal(hasForeignCommits(["A Reviewer"]), true); +}); + +test("no remote branch yet: pushes and opens a pull request", async () => { + // `diff --cached --quiet` fails when something is staged, which is the + // healthy path; `ls-remote --exit-code` fails when the branch is absent. + const h = harness({ + gitAnswers: { "diff --cached": undefined, "ls-remote": undefined }, + }); + const result = await main({ argv: ARGV, ...h }); + + assert.deepEqual(result, { action: "created" }); + assert.ok(ran(h.calls, "git", "checkout", "-B", "vendor/vale")); + // A branch that does not exist has nothing to overwrite and nothing to lease + // against, so it takes an ordinary push. + assert.ok(ran(h.calls, "git", "push", "origin", "vendor/vale")); + assert.ok(ran(h.calls, "gh", "pr", "create")); + assert.ok( + !ran(h.calls, "git", "fetch"), + "fetched a branch that does not exist" + ); +}); + +test("an open pull request is retitled and rewritten, not duplicated", async () => { + const h = harness({ + gitAnswers: { + "diff --cached": undefined, + "ls-remote": "abc123\trefs/heads/vendor/vale", + "rev-parse": "abc123", + "log -1": BOT_AUTHOR, + "diff --quiet FETCH_HEAD": undefined, // content differs + }, + ghAnswers: { "pr list": "73" }, + }); + const result = await main({ argv: ARGV, ...h }); + + assert.deepEqual(result, { action: "updated", pr: 73 }); + assert.ok(!ran(h.calls, "gh", "pr", "create"), "opened a duplicate PR"); + // The title has to move with the body: a rolling branch whose PR still names + // the previous version is worse than no automation, because it reads as + // current. + const patch = h.calls.find( + ([name, ...args]) => name === "gh" && args[0] === "api" + ); + assert.ok(patch, "did not PATCH the open pull request"); + assert.ok(patch.includes("title=chore(vale): pin Vale 3.21.0")); + assert.ok(patch.includes("body=@/tmp/body.md")); +}); + +/** + * The guard that makes a static branch safe. A scheduled run that finds the + * same upstream version must be completely inert — otherwise every run rewrites + * the branch, and every rewrite dismisses approvals and restarts CI on a + * proposal that has not changed. + */ +test("an unchanged proposal is not re-pushed", async () => { + const h = harness({ + gitAnswers: { + "diff --cached": undefined, + "ls-remote": "abc123\trefs/heads/vendor/vale", + "log -1": BOT_AUTHOR, + "diff --quiet FETCH_HEAD": "", // identical content + }, + }); + const result = await main({ argv: ARGV, ...h }); + + assert.deepEqual(result, { action: "none", reason: "unchanged" }); + assert.ok(!ran(h.calls, "git", "push"), "force-pushed an unchanged branch"); + assert.ok(!ran(h.calls, "gh", "pr"), "touched the pull request anyway"); +}); + +/** + * The guard that protects a reviewer. Pushing a fixup onto the branch is the + * most reasonable thing a reviewer can do, and a force-push would delete it + * with nothing reporting the loss. + */ +test("a branch last written by someone else is left alone", async () => { + const h = harness({ + gitAnswers: { + "diff --cached": undefined, + "ls-remote": "abc123\trefs/heads/vendor/vale", + "log -1": "A Reviewer", + }, + }); + const result = await main({ argv: ARGV, ...h }); + + assert.deepEqual(result, { action: "none", reason: "foreign-commits" }); + assert.ok(!ran(h.calls, "git", "push"), "force-pushed over a reviewer"); +}); + +test("the ownership check happens before the push, not after", async () => { + const h = harness({ + gitAnswers: { + "diff --cached": undefined, + "ls-remote": "abc123\trefs/heads/vendor/vale", + "log -1": BOT_AUTHOR, + "diff --quiet FETCH_HEAD": undefined, + }, + ghAnswers: { "pr list": "" }, + }); + await main({ argv: ARGV, ...h }); + + const authorAt = h.calls.findIndex( + ([name, ...args]) => name === "git" && args[0] === "log" + ); + const pushAt = h.calls.findIndex( + ([name, ...args]) => name === "git" && args[0] === "push" + ); + assert.ok(authorAt !== -1 && pushAt !== -1); + assert.ok(authorAt < pushAt, "pushed before checking who owns the branch"); +}); + +test("a working tree with no bump in it proposes nothing", async () => { + const h = harness({ gitAnswers: { "diff --cached": "" } }); + const result = await main({ argv: ARGV, ...h }); + + assert.deepEqual(result, { action: "none", reason: "nothing-staged" }); + assert.ok(!ran(h.calls, "git", "commit"), "committed an empty change"); + assert.ok(!ran(h.calls, "git", "push")); +}); + +test("a label is passed on only when one was asked for", async () => { + const h = harness({ + gitAnswers: { "diff --cached": undefined, "ls-remote": undefined }, + }); + await main({ + argv: [...ARGV.slice(0, 8), "--label", "skip-changeset", ...ARGV.slice(8)], + ...h, + }); + + const create = h.calls.find( + ([name, ...args]) => + name === "gh" && args[0] === "pr" && args[1] === "create" + ); + assert.ok(create.includes("skip-changeset")); +}); + +/** + * The narrower half of the unchanged guard, and the one that was wrong first. + * Comparing whole trees made an unrelated commit on `main` look like a changed + * proposal, so a quiet upstream still force-pushed the branch and dismissed the + * pull request's approvals over somebody else's edit to a different file. + */ +test("the unchanged check looks only at the paths being proposed", async () => { + const h = harness({ + gitAnswers: { + "diff --cached": undefined, + "ls-remote": "abc123\trefs/heads/vendor/vale", + "log -1": BOT_AUTHOR, + "diff --quiet FETCH_HEAD": "", + }, + }); + await main({ argv: ARGV, ...h }); + + const compare = h.calls.find( + ([name, ...args]) => + name === "git" && args[0] === "diff" && args[2] === "FETCH_HEAD" + ); + assert.ok(compare.includes("--"), "compared whole trees, not the proposal"); + assert.ok(compare.includes(".github/scripts/vale-manifest.json")); +}); + +/** + * The ownership guard reads the tip author and the push happens afterwards, so + * on its own it is a check with a gap after it: a reviewer pushing a fixup + * inside that gap loses the commit silently, which is exactly the outcome the + * guard exists to prevent — reached by timing rather than by logic. + * + * The lease makes the push assert what the guard assumed. The SHA is explicit + * rather than implied by a remote-tracking ref, because `actions/checkout` + * clones at depth 1 and there may be no tracking ref to lease against. + */ +test("the push leases against the exact commit the guards inspected", async () => { + const h = harness({ + gitAnswers: { + "diff --cached": undefined, + "ls-remote": "abc123\trefs/heads/vendor/vale", + "rev-parse": "deadbeef", + "log -1": BOT_AUTHOR, + "diff --quiet FETCH_HEAD": undefined, + }, + ghAnswers: { "pr list": "" }, + }); + await main({ argv: ARGV, ...h }); + + const push = h.calls.find( + ([name, ...args]) => name === "git" && args[0] === "push" + ); + assert.ok( + push.includes("--force-with-lease=vendor/vale:deadbeef"), + `expected a lease against the fetched tip, got: ${push.join(" ")}` + ); + assert.ok(!push.includes("--force"), "an unconditional force survived"); +}); diff --git a/.github/workflows/ast-grep-upgrade.yml b/.github/workflows/ast-grep-upgrade.yml new file mode 100644 index 00000000..bc661f56 --- /dev/null +++ b/.github/workflows/ast-grep-upgrade.yml @@ -0,0 +1,185 @@ +# SPDX-License-Identifier: MIT +# ast-grep — notice upstream releases, and open the pin bump with the changelog in it. +# +# WHY THIS EXISTS. Vale's upstream releases have been watched by a machine since +# the platform packages were built. ast-grep had `sg-detect.cjs` but nowhere for +# its answer to go: the only consumer was the README badge, which renders a +# version and cannot render a changelog. So "ast-grep 0.45.3 is out" was +# discoverable only by someone noticing a number on a badge and then going to +# find the release notes by hand — and 0.45.3 duly sat unpinned with nothing +# reporting it. +# +# WHY IT OPENS A PULL REQUEST, having previously argued for not writing at all. +# `sg-detect.cjs` used to refuse to write on the grounds that a lockfile-touching +# bump belongs to a human. Half of that was right and is unchanged: nobody +# merges this unread, and `Validate` runs on it like any other pull request. +# The other half was wrong. Refusing to write does not cause anyone to review +# anything — it only meant nothing was ever proposed. A reviewed pull request is +# more review than no pull request. +# +# HOW THIS DIFFERS FROM release-vale.yml's detect, which it otherwise mirrors: +# +# - There is no trust boundary here, so there is no second phase. Vale's +# detect proposes DIGESTS that authorize a later job to download and publish +# third-party bytes, which is why reviewing it is load-bearing and why the +# publish is split into a separate credential-free fetch. This proposes a +# version string. npm verifies its own integrity through the lockfile, and +# nothing here publishes anything. +# +# - The lockfile is regenerated rather than hand-edited. `pnpm install +# --lockfile-only` resolves the eight platform packages at the new version +# and rewrites `pnpm-lock.yaml`. It fails if upstream published the CLI +# without one of its platform siblings, which is the failure worth catching +# before a reviewer sees it, not after. +# +# - A changeset is written, where Vale's detect labels `skip-changeset`. The +# Vale packages are stamped by their own pipeline and reach a consumer only +# when a pin is bumped; THIS is that bump, and it changes which ast-grep a +# `@taskless/cli` user runs. `patch`, because the package is pre-1.0 — see +# the bump guidance in CLAUDE.md before reaching for `minor`. +# +# ONE ROLLING BRANCH, `vendor/ast-grep/upgrade`. `vendor-pr.cjs` owns that +# lifecycle for every vendor workflow — rebuild from `main`, retitle and rewrite +# as upstream moves, and force-push only when the proposal changed and only when +# this workflow wrote what is already on the branch. +# +# THE `/upgrade` SUFFIX IS NOT DECORATION, even though ast-grep has only the one +# stage. Git refs are paths, so a branch named `vendor/ast-grep` is a FILE at +# that path and makes `vendor/ast-grep/` impossible to create ever +# after. Vale needed exactly that split — `vendor/vale/republish` for the +# packages we build and `vendor/vale/upgrade` for the pins a user gets — and +# reserving the namespace here costs one word. That Vale has a `republish` and +# ast-grep does not is the real difference between them: nothing in this +# repository repackages ast-grep, so there is no set of our own to publish +# before the pins can move. +# +# Action refs are pinned to commit SHAs; the trailing comment records the tag. + +name: Upgrade ast-grep + +on: + # Same cadence and the same reasoning as the Vale detect: upstream's release + # rhythm, not ours. The dispatch covers "upstream just released, do not wait + # until Monday". + schedule: + - cron: "41 7 * * 1" + + workflow_dispatch: + +permissions: {} + +concurrency: ast-grep-upgrade + +jobs: + detect: + name: Upgrade the pinned ast-grep + runs-on: ubuntu-latest + permissions: + contents: write # push the vendor/ast-grep/upgrade branch + pull-requests: write # open the bump PR + steps: + # Credentials persist because this job pushes a branch. It holds no npm + # identity and no id-token. + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + + - uses: pnpm/action-setup@ea17c68df8912ef543352723c149a84f56e3d413 # v6.1.0 + - uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0 + with: + node-version-file: .nvmrc + + # No dependency install: sg-detect.cjs is zero-dependency CommonJS, and + # the lockfile step below needs pnpm rather than node_modules. + # + # `--notes-out` writes upstream's release notes to a FILE rather than to a + # step output, because they are third-party Markdown and a $GITHUB_OUTPUT + # line is delimited text that a release body containing the delimiter can + # break out of. It reaches the body through `--body-file` and never meets + # a shell or a `${{ }}` expression. + - id: detect + run: | + node .github/scripts/sg-detect.cjs \ + --write --notes-out "${RUNNER_TEMP}/sg-release-notes.md" + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + + # Resolution, not editing. `--lockfile-only` skips linking, so nothing is + # downloaded into node_modules and no lifecycle script runs; the job's + # only product is a rewritten pnpm-lock.yaml. It fails if the eight + # platform packages are not all published at the new version. + - name: Regenerate the lockfile + if: steps.detect.outputs.update == 'true' + run: pnpm install --lockfile-only --ignore-scripts + + # Every value reaching the shell goes through `env:` rather than `${{ }}` + # interpolation into the script body. The branch and pull request + # lifecycle is `vendor-pr.cjs`, shared with release-vale.yml. + - name: Propose the bump + if: steps.detect.outputs.update == 'true' + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + SG_VERSION: ${{ steps.detect.outputs.sg_version }} + PINNED_VERSION: ${{ steps.detect.outputs.pinned_version }} + run: | + set -euo pipefail + + # A changeset, because this bump is what reaches a consumer — unlike + # the Vale manifest, whose packages are stamped by their own pipeline + # and reach nobody until a pin like this one moves. `patch` is correct + # while @taskless/cli is pre-1.0 even though the diff adds no surface + # of our own; see the bump guidance in CLAUDE.md before reaching for + # `minor`. The filename carries the version so that two different + # bumps never share a release note. Accumulation is not what stops + # here — `vendor-pr.cjs` rebuilds the branch from `main` every run, + # so only ever one changeset file exists on it regardless of the + # name. + changeset=".changeset/ast-grep-${SG_VERSION//./-}.md" + cat > "$changeset" < "$body" <> "$body" + cat "$notes" >> "$body" + else + echo "::warning::no release notes file at ${notes}" + fi + + node .github/scripts/vendor-pr.cjs \ + --branch vendor/ast-grep/upgrade \ + --title "chore(ast-grep): pin ast-grep ${SG_VERSION}" \ + --message "chore(ast-grep): pin ast-grep ${SG_VERSION}" \ + --body-file "$body" \ + -- packages/cli/package.json pnpm-lock.yaml "$changeset" diff --git a/.github/workflows/release-vale.yml b/.github/workflows/release-vale.yml index 4b8364f2..aab988b5 100644 --- a/.github/workflows/release-vale.yml +++ b/.github/workflows/release-vale.yml @@ -13,8 +13,21 @@ # compares the latest upstream Vale release against the version # pinned in .github/scripts/vale-manifest.json. When upstream is # ahead it opens a pull request that updates the pinned version and -# all six SHA256 digests, taken from upstream's own checksums file. -# It publishes nothing. +# all six SHA256 digests, taken from upstream's own checksums file, +# and carries upstream's release notes so a reviewer can see what the +# digests are for. It publishes nothing. +# +# That pull request lives on ONE ROLLING BRANCH, +# `vendor/vale/republish`, rebuilt and retitled as upstream moves +# rather than accumulating a branch per release. `vendor-pr.cjs` owns +# that lifecycle and its two force-push guards, and every vendor +# workflow uses it, so they behave identically. +# +# THIS IS THE PRODUCER HALF ONLY. Publishing a platform package +# changes nothing for a consumer (see APPROVAL POLICY below); moving +# the CLI's pins onto the published set is `vale-upgrade.yml`, on +# `vendor/vale/upgrade`. Do not add that here — it needs no npm +# credential and belongs nowhere near one. # # publish Runs on the push to main that merges that pull request — i.e. once # a human has reviewed the digests. Split further into `gate`, @@ -142,7 +155,7 @@ jobs: (github.event_name == 'workflow_dispatch' && inputs.phase == 'detect') runs-on: ubuntu-latest permissions: - contents: write # push the vale/update- branch + contents: write # push the vendor/vale/republish branch pull-requests: write # open the manifest update PR steps: # Credentials persist here because this job pushes a branch. It holds no @@ -154,8 +167,19 @@ jobs: # No install step: the script is zero-dependency CommonJS. GITHUB_TOKEN is # passed only to raise the GitHub API rate limit; the endpoints are public. + # + # `--notes-out` writes upstream's release notes for the proposed version + # to a file, which the next step appends to the pull request body. A + # reviewer's job here is to decide whether these bytes should be + # publishable, and a bare version number with six digests does not say + # whether the release is a security fix or a docs pass. The notes arrive + # as a FILE rather than a step output because they are third-party + # Markdown: a $GITHUB_OUTPUT line is delimited text that a release body + # containing the delimiter can break out of. - id: detect - run: node .github/scripts/vale-detect.cjs --write + run: | + node .github/scripts/vale-detect.cjs \ + --write --notes-out "${RUNNER_TEMP}/vale-release-notes.md" env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} @@ -163,37 +187,26 @@ jobs: # `${{ }}` interpolation into the script body. The version has already been # validated as major.minor.patch by the script, but the pattern is the # rule regardless of the value. - - name: Open the manifest update PR + # + # The branch and pull request lifecycle is `vendor-pr.cjs`, shared with + # every vendor workflow: one rolling `vendor/vale/republish` branch, + # rebuilt from `main`, retitled and rewritten as upstream moves, and + # force-pushed only when the proposal actually changed and only when this + # workflow wrote what is already there. + - name: Propose the bump if: steps.detect.outputs.update == 'true' env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} VALE_VERSION: ${{ steps.detect.outputs.vale_version }} PINNED_VERSION: ${{ steps.detect.outputs.pinned_version }} run: | - branch="vale/update-${VALE_VERSION}" - git config user.name "github-actions[bot]" - git config user.email "41898282+github-actions[bot]@users.noreply.github.com" - - if git diff --quiet -- .github/scripts/vale-manifest.json; then - echo "Manifest already pins ${VALE_VERSION}; nothing to propose." - exit 0 - fi - - git checkout -b "$branch" - git add .github/scripts/vale-manifest.json - git commit -m "chore(vale): pin Vale ${VALE_VERSION}" - git push --force origin "$branch" + set -euo pipefail - if [ -n "$(gh pr list --head "$branch" --state open --json number --jq '.[].number')" ]; then - echo "A pull request for $branch is already open; it now carries the current digests." - exit 0 - fi - - gh pr create \ - --base main \ - --head "$branch" \ - --title "chore(vale): pin Vale ${VALE_VERSION}" \ - --body "$(cat <<'BODY' + # Composed as a FILE, not as an argument. The preamble is ours and the + # notes appended below it are upstream's, and the quoted heredoc plus + # `cat` means neither the shell nor gh ever interprets the latter. + body="${RUNNER_TEMP}/vale-pr-body.md" + cat > "$body" <<'BODY' Upstream Vale is ahead of the version this repository packages. This updates `.github/scripts/vale-manifest.json` to the new upstream @@ -210,9 +223,31 @@ jobs: package `-` and publishes the set. That publish is inert on its own: `@taskless/cli` pins exact versions, so nothing reaches a consumer until that pin is deliberately bumped. + + This pull request rolls: if upstream releases again before it merges, + the branch, title, and body are rewritten to the newer version rather + than a second pull request being opened. Push a commit to the branch + and that stops — the workflow will not force-push over a commit it did + not write. BODY - )" \ - --label skip-changeset + + # Absent only if the detect step reported ahead and then wrote no + # notes, which it cannot — so warn rather than skip silently. + notes="${RUNNER_TEMP}/vale-release-notes.md" + if [ -f "$notes" ]; then + printf '\n---\n\n' >> "$body" + cat "$notes" >> "$body" + else + echo "::warning::no release notes file at ${notes}" + fi + + node .github/scripts/vendor-pr.cjs \ + --branch vendor/vale/republish \ + --title "chore(vale): pin Vale ${VALE_VERSION}" \ + --message "chore(vale): pin Vale ${VALE_VERSION}" \ + --body-file "$body" \ + --label skip-changeset \ + -- .github/scripts/vale-manifest.json # Is there anything to publish? The push trigger fires on ANY edit to # vale-manifest.json — a reworded comment, a reformat, a digest correction — diff --git a/.github/workflows/vale-upgrade.yml b/.github/workflows/vale-upgrade.yml new file mode 100644 index 00000000..01bb915e --- /dev/null +++ b/.github/workflows/vale-upgrade.yml @@ -0,0 +1,164 @@ +# SPDX-License-Identifier: MIT +# Vale — ship the platform packages we already published. +# +# THE STAGE THAT WAS MISSING. Vendoring Vale has two halves, and until now only +# one of them was automated: +# +# republish release-vale.yml. Watches upstream Vale, proposes a manifest +# with new digests on `vendor/vale/republish`, and on merge +# publishes the six @taskless/vale-* packages at +# -. Changes nothing for a consumer. +# +# upgrade this workflow. Moves the six pins in packages/cli/package.json +# to the published set on `vendor/vale/upgrade`. THIS is what +# reaches a user. +# +# release-vale.yml states the dependency in its own header: "a new version +# reaches a user only when someone reviews a bump to that pin". Somebody always +# did — `git log` on the pins shows 3.18.0, 3.19.0 and 3.20.0 each moved in its +# own hand-written commit, and the pins are current as this is written. So this +# automates a step that was working, which is worth being honest about: what it +# removes is the dependency on remembering, and what it adds is the changelog +# next to the diff. The failure it prevents is quiet — a publish lands, nobody +# notices the pins are behind, and the release simply does not ship. +# +# ast-grep has only the upgrade half, because nothing here repackages it; there +# is no `vendor/ast-grep/republish` and its absence is the point. Beyond that +# the two upgrades are the same operation, and they share the same scripts: +# `pin-bump.cjs` rewrites the pins, `release-notes.cjs` renders the changelog, +# and `vendor-pr.cjs` owns the rolling branch and its force-push guards. What +# differs is which registry answers "what is published" and which tag carries +# the release notes. +# +# THE CHANGELOG HERE IS UPSTREAM'S, NOT OURS. Our stamp records when a package +# was built, which tells a reviewer nothing about what changed. The base version +# inside the stamp is the release upstream actually tagged, and that is what +# gets fetched. +# +# Action refs are pinned to commit SHAs; the trailing comment records the tag. + +name: Upgrade Vale + +on: + # Deliberately DAILY, where the republish detect is weekly. That one waits on + # upstream; this one waits on our own publish job, which fires whenever a + # manifest pull request merges. A weekly schedule here would mean a Vale + # release sat packaged-but-unshipped for up to a week after we published it, + # which is the exact failure this workflow exists to end. + schedule: + - cron: "52 7 * * *" + + workflow_dispatch: + +permissions: {} + +concurrency: vale-upgrade + +jobs: + upgrade: + name: Upgrade the pinned Vale + runs-on: ubuntu-latest + permissions: + contents: write # push the vendor/vale/upgrade branch + pull-requests: write # open the upgrade PR + steps: + # Credentials persist because this job pushes a branch. It holds no npm + # identity and no id-token. + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + + - uses: pnpm/action-setup@ea17c68df8912ef543352723c149a84f56e3d413 # v6.1.0 + - uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0 + with: + node-version-file: .nvmrc + + # No dependency install: the script is zero-dependency CommonJS, and the + # lockfile step below needs pnpm rather than node_modules. GITHUB_TOKEN is + # only for the rate limit on the release-notes lookup. + # + # `--notes-out` writes third-party Markdown to a FILE rather than a step + # output, because a $GITHUB_OUTPUT line is delimited text that a release + # body containing the delimiter can break out of. + - id: detect + run: | + node .github/scripts/vale-upgrade-detect.cjs \ + --write --notes-out "${RUNNER_TEMP}/vale-release-notes.md" + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + + # Resolution, not editing. `--lockfile-only` skips linking, so nothing is + # downloaded and no lifecycle script runs. It fails if the six packages + # are not all resolvable at the new version — a second, independent check + # on the half-published set the detect step already refuses. + - name: Regenerate the lockfile + if: steps.detect.outputs.update == 'true' + run: pnpm install --lockfile-only --ignore-scripts + + - name: Propose the upgrade + if: steps.detect.outputs.update == 'true' + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + VALE_VERSION: ${{ steps.detect.outputs.vale_version }} + BASE_VERSION: ${{ steps.detect.outputs.base_version }} + PINNED_VERSION: ${{ steps.detect.outputs.pinned_version }} + run: | + set -euo pipefail + + # A changeset, because this is the bump a consumer experiences — the + # republish that produced the package did not change anyone's install. + # `patch` is correct while @taskless/cli is pre-1.0 even for a new Vale + # minor; see the bump guidance in CLAUDE.md before reaching for + # `minor`. Named for the BASE version, not the stamped one: a + # republish mints a fresh timestamp for the same upstream Vale, so a + # stamped filename would churn on every republish while the release + # note it contains says the same thing. The base version is also what + # the title, the body, and the note itself say. + changeset=".changeset/vale-${BASE_VERSION//./-}.md" + cat > "$changeset" < "$body" <> "$body" + cat "$notes" >> "$body" + else + echo "::warning::no release notes file at ${notes}" + fi + + node .github/scripts/vendor-pr.cjs \ + --branch vendor/vale/upgrade \ + --title "chore(vale): upgrade to Vale ${BASE_VERSION}" \ + --message "chore(vale): upgrade to Vale ${BASE_VERSION}" \ + --body-file "$body" \ + -- packages/cli/package.json pnpm-lock.yaml "$changeset"