diff --git a/.github/scripts/pin-bump.cjs b/.github/scripts/pin-bump.cjs index b96385aa..b5e0fff9 100644 --- a/.github/scripts/pin-bump.cjs +++ b/.github/scripts/pin-bump.cjs @@ -76,4 +76,59 @@ function bumpPins(source, { pattern, from, to }) { return { source: bumped, count }; } -module.exports = { bumpPins, escapeLiteral }; +/** + * Move one `export const NAME = "…";` declaration to a new version, in the + * TypeScript source text of `packages/cli/src/rules/capabilities.ts`. + * + * WHY THIS IS PART OF THE BUMP. `capabilities.ts` publishes `AST_GREP_VERSION` + * and `VALE_VERSION` by hand, and `test/engine-version-consistency.test.ts` + * asserts they agree with the pins. That test is deliberate and the constant + * is deliberately NOT derived from package.json (its docblock says why), so a + * bot commit that moves the pins and not the constant fails Validate BY + * CONSTRUCTION. That is what happened on taskless/cli#368: the base of a stack + * was red before anyone had looked at it, and the child (#372) had to carry + * the constant along with the real work. The bot moves both now, so the + * consistency test is what it was meant to be — a check on humans, not a + * scheduled failure. + * + * WHY TEXT AGAIN. The same reason as `bumpPins`: the file is prettier-formatted + * and the diff a reviewer reads should be one string. The match is anchored on + * the whole declaration line, so a mention of the name in a docblock or a + * `{@link VALE_VERSION}` is not a candidate. + * + * WHY EXACTLY ONE. Zero means the declaration moved or was renamed and the + * workflow would otherwise push a commit the consistency test rejects — the + * failure this exists to remove. Two means the anchor is no longer specific, + * and rewriting both would be a guess. Either is a failed run. + * + * @param source the capabilities.ts text + * @param name the exported constant, e.g. `VALE_VERSION` + * @param to the version to write. For Vale this is the BASE version the + * binary reports (`3.22.0`), not the stamped npm version. + * @returns the rewritten source and the version the declaration held before + */ +function bumpVersionConstant(source, { name, to }) { + if (!/^[A-Z][A-Z0-9_]*$/.test(String(name))) { + throw new Error( + `the constant name must be an UPPER_SNAKE identifier, got ${JSON.stringify(name)}` + ); + } + const matcher = new RegExp( + `^(export const ${name} = ")([^"\\n]*)(";)$`, + "gm" + ); + const matches = [...source.matchAll(matcher)]; + if (matches.length !== 1) { + throw new Error( + `expected exactly one \`export const ${name} = "…";\` declaration, found ${matches.length}` + ); + } + const [match] = matches; + const [whole, head, from, tail] = match; + const at = match.index; + const bumped = + source.slice(0, at) + head + to + tail + source.slice(at + whole.length); + return { source: bumped, from }; +} + +module.exports = { bumpPins, bumpVersionConstant, escapeLiteral }; diff --git a/.github/scripts/pin-bump.test.cjs b/.github/scripts/pin-bump.test.cjs index 1302af40..6995c101 100644 --- a/.github/scripts/pin-bump.test.cjs +++ b/.github/scripts/pin-bump.test.cjs @@ -14,7 +14,7 @@ const test = require("node:test"); const assert = require("node:assert/strict"); -const { bumpPins } = require("./pin-bump.cjs"); +const { bumpPins, bumpVersionConstant } = require("./pin-bump.cjs"); /** The boundary-aware pattern sg-detect.cjs enumerates its pins with. */ const AST_GREP = /^@ast-grep\/cli(-|$)/; @@ -114,3 +114,86 @@ test("a stateful /g pattern is refused rather than silently skipping pins", () = /must not be \/g/ ); }); + +/** + * The constant half of the bump. `capabilities.ts` declares `AST_GREP_VERSION` + * and `VALE_VERSION` by hand and `engine-version-consistency.test.ts` holds + * them to the pins, so a bot commit that moves only the pins is red before + * anyone reads it (taskless/cli#368). These pin the rewrite that closes that. + */ +const CAPABILITIES = [ + "/**", + " * Pinned against the binary by `test/ast-grep-vendor-contract.test.ts`.", + " */", + 'export const AST_GREP_VERSION = "0.45.3";', + "", + "/**", + " * The Vale release, measured against {@link VALE_VERSION}'s binary.", + ' * A literal stamp like "3.21.0-20260915061224" lives in package.json.', + " */", + 'export const VALE_VERSION = "3.21.0";', + "", + 'export const OTHER = "3.21.0";', + "", +].join("\n"); + +test("the named constant moves to the base version and reports what it held", () => { + const { source, from } = bumpVersionConstant(CAPABILITIES, { + name: "VALE_VERSION", + to: "3.22.0", + }); + assert.equal(from, "3.21.0"); + assert.match(source, /^export const VALE_VERSION = "3\.22\.0";$/m); + // Every other byte survives: the docblock's mention of the old version, the + // `{@link}`, and the unrelated constant at the same value. + assert.match(source, /"3\.21\.0-20260915061224"/); + assert.match(source, /\{@link VALE_VERSION\}/); + assert.match(source, /^export const OTHER = "3\.21\.0";$/m); + assert.match(source, /^export const AST_GREP_VERSION = "0\.45\.3";$/m); + assert.equal(source.split("\n").length, CAPABILITIES.split("\n").length); +}); + +test("the ast-grep constant is reached by the same anchor", () => { + const { source, from } = bumpVersionConstant(CAPABILITIES, { + name: "AST_GREP_VERSION", + to: "0.46.0", + }); + assert.equal(from, "0.45.3"); + assert.match(source, /^export const AST_GREP_VERSION = "0\.46\.0";$/m); + assert.match(source, /^export const VALE_VERSION = "3\.21\.0";$/m); +}); + +test("a constant already at the target is rewritten to itself", () => { + const { source, from } = bumpVersionConstant(CAPABILITIES, { + name: "VALE_VERSION", + to: "3.21.0", + }); + assert.equal(from, "3.21.0"); + assert.equal(source, CAPABILITIES); +}); + +test("a missing declaration fails the run rather than writing nothing", () => { + assert.throws( + () => + bumpVersionConstant(CAPABILITIES, { + name: "RUFF_VERSION", + to: "1.0.0", + }), + /expected exactly one `export const RUFF_VERSION = "…";` declaration, found 0/ + ); +}); + +test("a declaration that appears twice is ambiguous and refused", () => { + const doubled = `${CAPABILITIES}export const VALE_VERSION = "3.20.0";\n`; + assert.throws( + () => bumpVersionConstant(doubled, { name: "VALE_VERSION", to: "3.22.0" }), + /found 2/ + ); +}); + +test("a name that is not an identifier cannot become a pattern", () => { + assert.throws( + () => bumpVersionConstant(CAPABILITIES, { name: ".*", to: "3.22.0" }), + /UPPER_SNAKE identifier/ + ); +}); diff --git a/.github/scripts/sg-detect.cjs b/.github/scripts/sg-detect.cjs index 784914b4..91f01d43 100644 --- a/.github/scripts/sg-detect.cjs +++ b/.github/scripts/sg-detect.cjs @@ -41,8 +41,10 @@ * 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. + * the upstream version, and `AST_GREP_VERSION` in + * packages/cli/src/rules/capabilities.ts with it. + * `ast-grep-upgrade.yml` then regenerates the lockfile and the rule + * schema, checks the result, 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 @@ -84,7 +86,7 @@ const { appendFileSync, readFileSync, writeFileSync } = require("node:fs"); const { join } = require("node:path"); -const { bumpPins } = require("./pin-bump.cjs"); +const { bumpPins, bumpVersionConstant } = require("./pin-bump.cjs"); const { fetchReleaseByTag, formatReleaseNotes, @@ -101,6 +103,25 @@ const PACKAGE_JSON_PATH = join( "package.json" ); +/** + * Where `AST_GREP_VERSION` is declared by hand. `--write` moves it with the + * pins, because `test/engine-version-consistency.test.ts` holds the two + * together and a bump that moved only the pins failed Validate before anyone + * had looked at it (taskless/cli#368, the Vale instance of the same shape). + */ +const CAPABILITIES_PATH = join( + __dirname, + "..", + "..", + "packages", + "cli", + "src", + "rules", + "capabilities.ts" +); + +const VERSION_CONSTANT = "AST_GREP_VERSION"; + /** `@ast-grep/cli` itself and its per-platform siblings. */ const PIN_PATTERN = /^@ast-grep\/cli(-|$)/; @@ -242,6 +263,7 @@ async function main({ releaseFor = fetchReleaseByTag, packageJsonPath = PACKAGE_JSON_PATH, packageJson = JSON.parse(readFileSync(packageJsonPath, "utf8")), + capabilitiesPath = CAPABILITIES_PATH, } = {}) { const json = argv.includes("--json"); const write = argv.includes("--write"); @@ -270,10 +292,13 @@ 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. + // Only the ahead path has anything to write. BOTH rewrites are computed + // before EITHER file is written, and the writes come before the notes are + // fetched, so that neither a refused rewrite nor a network failure on the + // (optional) changelog can leave a half-done bump: by the time anything can + // throw below, the two files on disk are either both bumped or both + // untouched. A `--write` run by hand that fails therefore leaves a clean + // tree, not a package.json that moved without its constant. if (write && ahead) { const pins = collectPins(packageJson); const source = readFileSync(packageJsonPath, "utf8"); @@ -292,8 +317,22 @@ async function main({ `expected to rewrite ${pins.size} @ast-grep/cli* pins, rewrote ${count}` ); } + + // Upstream's own packages, so the constant is the pin verbatim. Rewritten + // in the same run as the pins so the bot commit is self-consistent: a + // missing declaration throws here, before anything is written, rather + // than failing Validate later. + const { source: constants, from } = bumpVersionConstant( + readFileSync(capabilitiesPath, "utf8"), + { name: VERSION_CONSTANT, to: upstream } + ); + writeFileSync(packageJsonPath, bumped); + writeFileSync(capabilitiesPath, constants); log(`Rewrote ${count} pins in ${packageJsonPath} to ${upstream}.`); + log( + `Rewrote ${VERSION_CONSTANT} in ${capabilitiesPath}: ${from} -> ${upstream}.` + ); } // Only the ahead path has a bump to describe. A second request, unlike Vale's diff --git a/.github/scripts/sg-detect.test.cjs b/.github/scripts/sg-detect.test.cjs index 7f3b4d6c..07de2590 100644 --- a/.github/scripts/sg-detect.test.cjs +++ b/.github/scripts/sg-detect.test.cjs @@ -26,6 +26,20 @@ const CLI_PACKAGE_JSON = JSON.parse( ) ); +/** + * A capabilities.ts with the declaration `--write` rewrites, plus the kind of + * neighbour it must not touch: the Vale constant, and a docblock naming the + * old version. + */ +const CAPABILITIES_AT = (version) => + [ + "/** Measured at 0.45.2; see {@link AST_GREP_VERSION}. */", + `export const AST_GREP_VERSION = "${version}";`, + "", + 'export const VALE_VERSION = "3.21.0";', + "", + ].join("\n"); + /** Run main() with the registry stubbed and $GITHUB_OUTPUT captured. */ async function runDetect({ upstream, @@ -34,6 +48,7 @@ async function runDetect({ argv = [], release, wantNotes = false, + capabilitiesSource = CAPABILITIES_AT("0.45.2"), }) { const directory = mkdtempSync(join(tmpdir(), "sg-detect-test-")); const outputPath = join(directory, "github-output"); @@ -44,6 +59,11 @@ async function runDetect({ if (packageJsonSource !== undefined) { writeFileSync(packageJsonPath, packageJsonSource); } + // --write also rewrites AST_GREP_VERSION, so the fixture carries a + // capabilities.ts of its own for the same reason. The committed one is + // never written to here either. + const capabilitiesPath = join(directory, "capabilities.ts"); + writeFileSync(capabilitiesPath, capabilitiesSource); const previous = process.env.GITHUB_OUTPUT; const releasesFetched = []; process.env.GITHUB_OUTPUT = outputPath; @@ -57,6 +77,7 @@ async function runDetect({ }, packageJsonPath, packageJson, + capabilitiesPath, }); const outputs = Object.fromEntries( readFileSync(outputPath, "utf8") @@ -81,12 +102,14 @@ async function runDetect({ packageJsonSource === undefined ? undefined : readFileSync(packageJsonPath, "utf8"); + const capabilitiesWritten = readFileSync(capabilitiesPath, "utf8"); return { comparison, outputs, notesWritten, releasesFetched, packageJsonWritten, + capabilitiesWritten, }; } finally { if (previous === undefined) { @@ -289,7 +312,7 @@ const sourcePinnedAt = (version) => )}\n`; test("sg-detect: --write bumps every pin in the file on disk", async () => { - const { packageJsonWritten, outputs } = await runDetect({ + const { packageJsonWritten, capabilitiesWritten, outputs } = await runDetect({ upstream: "0.45.3", packageJson: pinnedAt("0.45.2"), packageJsonSource: sourcePinnedAt("0.45.2"), @@ -299,11 +322,16 @@ test("sg-detect: --write bumps every pin in the file on disk", async () => { assert.equal(outputs.update, "true"); assert.doesNotMatch(packageJsonWritten, /0\.45\.2/); assert.equal(packageJsonWritten.match(/0\.45\.3/g).length, 3); + // The constant moves with the pins, and nothing around it does. This is + // what keeps the bot commit green under engine-version-consistency.test.ts. + assert.equal(capabilitiesWritten, CAPABILITIES_AT("0.45.3")); + assert.match(capabilitiesWritten, /Measured at 0\.45\.2/); + assert.match(capabilitiesWritten, /VALE_VERSION = "3\.21\.0"/); }); -test("sg-detect: --write leaves the file alone when the pin is current", async () => { +test("sg-detect: --write leaves the files alone when the pin is current", async () => { const before = sourcePinnedAt("0.45.2"); - const { packageJsonWritten } = await runDetect({ + const { packageJsonWritten, capabilitiesWritten } = await runDetect({ upstream: "0.45.2", packageJson: pinnedAt("0.45.2"), packageJsonSource: before, @@ -311,6 +339,55 @@ test("sg-detect: --write leaves the file alone when the pin is current", async ( }); assert.equal(packageJsonWritten, before); + assert.equal(capabilitiesWritten, CAPABILITIES_AT("0.45.2")); +}); + +/** + * The declaration the workflow depends on has moved or been renamed. Failing + * here is the point: pushing the pins without the constant is exactly the + * red-by-construction pull request this rewrite exists to end. + */ +test("sg-detect: --write fails when AST_GREP_VERSION is not declared once", async () => { + await assert.rejects( + runDetect({ + upstream: "0.45.3", + packageJson: pinnedAt("0.45.2"), + packageJsonSource: sourcePinnedAt("0.45.2"), + argv: ["--write"], + capabilitiesSource: 'export const VALE_VERSION = "3.21.0";\n', + }), + /expected exactly one `export const AST_GREP_VERSION = "…";` declaration, found 0/ + ); +}); + +/** + * The half of that failure that matters to a tree: when the constant cannot + * be rewritten, the pins must not have been written either. Both rewrites are + * computed before either file is touched, so a refused run leaves package.json + * exactly as it found it rather than bumped without its constant. + */ +test("sg-detect: a refused constant rewrite leaves package.json untouched", async () => { + const before = sourcePinnedAt("0.45.2"); + const directory = mkdtempSync(join(tmpdir(), "sg-detect-atomic-")); + const packageJsonPath = join(directory, "package.json"); + const capabilitiesPath = join(directory, "capabilities.ts"); + writeFileSync(packageJsonPath, before); + writeFileSync(capabilitiesPath, 'export const VALE_VERSION = "3.21.0";\n'); + try { + await assert.rejects( + main({ + argv: ["--write"], + latestVersion: async () => "0.45.3", + packageJsonPath, + packageJson: pinnedAt("0.45.2"), + capabilitiesPath, + }), + /found 0/ + ); + assert.equal(readFileSync(packageJsonPath, "utf8"), before); + } finally { + rmSync(directory, { recursive: true, force: true }); + } }); /** diff --git a/.github/scripts/vale-upgrade-detect.cjs b/.github/scripts/vale-upgrade-detect.cjs index b0049ca8..13c67fff 100644 --- a/.github/scripts/vale-upgrade-detect.cjs +++ b/.github/scripts/vale-upgrade-detect.cjs @@ -21,7 +21,8 @@ * 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. + * npm and, with `--write`, moves every pin to it and `VALE_VERSION` in + * packages/cli/src/rules/capabilities.ts to the base version inside the stamp. * * 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 @@ -55,7 +56,7 @@ const { appendFileSync, readFileSync, writeFileSync } = require("node:fs"); const { join } = require("node:path"); -const { bumpPins } = require("./pin-bump.cjs"); +const { bumpPins, bumpVersionConstant } = require("./pin-bump.cjs"); const { fetchReleaseByTag, formatReleaseNotes, @@ -79,6 +80,25 @@ const PACKAGE_JSON_PATH = join( const MANIFEST_PATH = join(__dirname, "vale-manifest.json"); +/** + * Where `VALE_VERSION` is declared by hand. `--write` moves it with the pins, + * to the BASE version, because `test/engine-version-consistency.test.ts` holds + * the two together and a bump that moved only the pins failed Validate before + * anyone had looked at it (taskless/cli#368). + */ +const CAPABILITIES_PATH = join( + __dirname, + "..", + "..", + "packages", + "cli", + "src", + "rules", + "capabilities.ts" +); + +const VERSION_CONSTANT = "VALE_VERSION"; + const PIN_PREFIX = "@taskless/vale-"; /** @@ -199,6 +219,7 @@ async function main({ releaseFor = fetchReleaseByTag, packageJsonPath = PACKAGE_JSON_PATH, packageJson = JSON.parse(readFileSync(packageJsonPath, "utf8")), + capabilitiesPath = CAPABILITIES_PATH, manifest = assertManifest(JSON.parse(readFileSync(MANIFEST_PATH, "utf8"))), } = {}) { const json = argv.includes("--json"); @@ -217,6 +238,9 @@ async function main({ latestVersion ); const ahead = compareStampedVersions(upstream, pinned) > 0; + // The release upstream actually tagged: what the constant records, what the + // changelog is fetched by, and what the workflow names the pull request. + const base = baseVersion(upstream); log(`pinned: ${pinned} published latest: ${upstream}`); log( @@ -230,6 +254,11 @@ async function main({ console.log(JSON.stringify(comparison)); } + // BOTH rewrites are computed before EITHER file is written, so a refused + // rewrite (a pin the pattern cannot reach, a constant declaration that has + // moved) leaves the two files on disk either both bumped or both untouched. + // A `--write` run by hand that fails therefore leaves a clean tree, not a + // package.json that moved without its constant. if (write && ahead) { const source = readFileSync(packageJsonPath, "utf8"); const { source: bumped, count } = bumpPins(source, { @@ -242,14 +271,27 @@ async function main({ `expected to rewrite ${pins.size} ${PIN_PREFIX}* pins, rewrote ${count}` ); } + + // The constant carries the version the BINARY reports, which is the base + // version, not the stamped one npm serves. Rewritten in the same run as the + // pins so the bot commit is self-consistent: a missing declaration throws + // here, before anything is written, rather than failing Validate later. + const { source: constants, from } = bumpVersionConstant( + readFileSync(capabilitiesPath, "utf8"), + { name: VERSION_CONSTANT, to: base } + ); + writeFileSync(packageJsonPath, bumped); + writeFileSync(capabilitiesPath, constants); log(`Rewrote ${count} pins in ${packageJsonPath} to ${upstream}.`); + log( + `Rewrote ${VERSION_CONSTANT} in ${capabilitiesPath}: ${from} -> ${base}.` + ); } // 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, @@ -265,7 +307,7 @@ async function main({ setOutput("update", String(ahead)); setOutput("vale_version", upstream); setOutput("pinned_version", pinned); - setOutput("base_version", baseVersion(upstream)); + setOutput("base_version", base); return comparison; } diff --git a/.github/scripts/vale-upgrade-detect.test.cjs b/.github/scripts/vale-upgrade-detect.test.cjs index 94e4b261..3edcd4a2 100644 --- a/.github/scripts/vale-upgrade-detect.test.cjs +++ b/.github/scripts/vale-upgrade-detect.test.cjs @@ -46,6 +46,21 @@ const sourcePinnedAt = (version) => 2 )}\n`; +/** + * A capabilities.ts with the declaration `--write` rewrites, plus the kind of + * neighbour it must not touch: the ast-grep constant, and a docblock naming + * the old version. The constant carries the BASE version; the stamp stays in + * package.json. + */ +const CAPABILITIES_AT = (version) => + [ + "/** The tier table is a property of {@link VALE_VERSION}'s binary. */", + `export const VALE_VERSION = "${version}";`, + "", + 'export const AST_GREP_VERSION = "0.45.3";', + "", + ].join("\n"); + async function run({ packageJson, packageJsonSource, @@ -53,6 +68,7 @@ async function run({ argv = [], release, wantNotes = false, + capabilitiesSource = CAPABILITIES_AT("3.20.0"), }) { const directory = mkdtempSync(join(tmpdir(), "vale-upgrade-test-")); const outputPath = join(directory, "github-output"); @@ -61,6 +77,10 @@ async function run({ if (packageJsonSource !== undefined) { writeFileSync(packageJsonPath, packageJsonSource); } + // --write also rewrites VALE_VERSION, so the fixture carries a + // capabilities.ts of its own. The committed one is never written to here. + const capabilitiesPath = join(directory, "capabilities.ts"); + writeFileSync(capabilitiesPath, capabilitiesSource); const previous = process.env.GITHUB_OUTPUT; const tagsFetched = []; process.env.GITHUB_OUTPUT = outputPath; @@ -75,6 +95,7 @@ async function run({ }, packageJsonPath, packageJson, + capabilitiesPath, manifest: MANIFEST, }); const outputs = Object.fromEntries( @@ -98,7 +119,15 @@ async function run({ packageJsonSource === undefined ? undefined : readFileSync(packageJsonPath, "utf8"); - return { comparison, outputs, notesWritten, tagsFetched, written }; + const capabilitiesWritten = readFileSync(capabilitiesPath, "utf8"); + return { + comparison, + outputs, + notesWritten, + tagsFetched, + written, + capabilitiesWritten, + }; } finally { if (previous === undefined) { delete process.env.GITHUB_OUTPUT; @@ -202,6 +231,90 @@ test("--write moves every pin and nothing else", async () => { assert.match(written, /"zod": "\^4"/); }); +/** + * The constant is what `test/engine-version-consistency.test.ts` holds to the + * pins, so it moves in the same write. It takes the BASE version — the one the + * binary reports — not the stamped one npm serves, and nothing else in the + * file moves with it. + */ +test("--write moves VALE_VERSION to the base version, not the stamp", async () => { + const { capabilitiesWritten } = await run({ + packageJson: pinnedAt("3.20.0-20260907164938"), + packageJsonSource: sourcePinnedAt("3.20.0-20260907164938"), + published: "3.21.0-20260914010203", + argv: ["--write"], + }); + + assert.equal(capabilitiesWritten, CAPABILITIES_AT("3.21.0")); + assert.doesNotMatch(capabilitiesWritten, /20260914010203/); + assert.match(capabilitiesWritten, /AST_GREP_VERSION = "0\.45\.3"/); +}); + +test("--write leaves both files alone when the pins are current", async () => { + const before = sourcePinnedAt("3.20.0-20260907164938"); + const { written, capabilitiesWritten } = await run({ + packageJson: pinnedAt("3.20.0-20260907164938"), + packageJsonSource: before, + published: "3.20.0-20260907164938", + argv: ["--write"], + }); + + assert.equal(written, before); + assert.equal(capabilitiesWritten, CAPABILITIES_AT("3.20.0")); +}); + +/** + * The declaration the workflow depends on has moved or been renamed. Failing + * here is the point: pushing the pins without the constant is exactly the + * red-by-construction pull request (taskless/cli#368) this rewrite ends. + */ +test("--write fails when VALE_VERSION is not declared exactly once", async () => { + await assert.rejects( + run({ + packageJson: pinnedAt("3.20.0-20260907164938"), + packageJsonSource: sourcePinnedAt("3.20.0-20260907164938"), + published: "3.21.0-20260914010203", + argv: ["--write"], + capabilitiesSource: 'export const AST_GREP_VERSION = "0.45.3";\n', + }), + /expected exactly one `export const VALE_VERSION = "…";` declaration, found 0/ + ); +}); + +/** + * The half of that failure that matters to a tree: when the constant cannot + * be rewritten, the pins must not have been written either. Both rewrites are + * computed before either file is touched, so a refused run leaves package.json + * exactly as it found it rather than bumped without its constant. + */ +test("a refused constant rewrite leaves package.json untouched", async () => { + const before = sourcePinnedAt("3.20.0-20260907164938"); + const directory = mkdtempSync(join(tmpdir(), "vale-upgrade-atomic-")); + const packageJsonPath = join(directory, "package.json"); + const capabilitiesPath = join(directory, "capabilities.ts"); + writeFileSync(packageJsonPath, before); + writeFileSync( + capabilitiesPath, + 'export const AST_GREP_VERSION = "0.45.3";\n' + ); + try { + await assert.rejects( + main({ + argv: ["--write"], + latestVersion: async () => "3.21.0-20260914010203", + packageJsonPath, + packageJson: pinnedAt("3.20.0-20260907164938"), + capabilitiesPath, + manifest: MANIFEST, + }), + /found 0/ + ); + assert.equal(readFileSync(packageJsonPath, "utf8"), before); + } finally { + rmSync(directory, { recursive: true, force: true }); + } +}); + 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"), diff --git a/.github/workflows/ast-grep-upgrade.yml b/.github/workflows/ast-grep-upgrade.yml index cff36922..55419cac 100644 --- a/.github/workflows/ast-grep-upgrade.yml +++ b/.github/workflows/ast-grep-upgrade.yml @@ -38,6 +38,22 @@ # `@taskless/cli` user runs. `patch`, because the package is pre-1.0 — see # the bump guidance in CLAUDE.md before reaching for `minor`. # +# THE BOT COMMIT IS SELF-CONSISTENT, AND CHECKED BEFORE IT IS PUSHED. +# `AST_GREP_VERSION` in packages/cli/src/rules/capabilities.ts is maintained by +# hand and test/engine-version-consistency.test.ts holds it to the pins +# (deliberately — read that test's docblock before deriving the constant from +# package.json), and src/generated/ast-grep-rule-schema.json is fetched from +# upstream by tag. A commit that moved only the pins failed Validate BY +# CONSTRUCTION; vale-upgrade.yml measured it on #368/#372, and this workflow +# had the same shape. So `--write` moves the constant with the pins, the rule +# schema is refetched for the new tag, and the consistency check runs here, +# before `vendor-pr.cjs` pushes. If it fails, the run fails and nothing is +# proposed: pushing an inconsistent commit is the thing being removed. What +# stays for a human child pull request is judgment, not bookkeeping: the +# measured additions to ast-grep-vendor-contract.test.ts, the `update` ledger, +# and the changeset prose. That contract test is NOT gated here, on purpose: +# see the check step. +# # 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 @@ -87,8 +103,13 @@ jobs: 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. + # No dependency install yet: sg-detect.cjs is zero-dependency CommonJS, + # and the install below has to come AFTER the bump so it resolves the + # new pins. + # + # `--write` moves the eight pins and `AST_GREP_VERSION` in + # capabilities.ts together, and fails the run if the declaration is not + # found exactly once. # # `--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 @@ -110,6 +131,48 @@ jobs: if: steps.detect.outputs.update == 'true' run: pnpm install --lockfile-only --ignore-scripts + # A real install this time, because the schema refetch and the check + # below run under the package's toolchain, and the check spawns the NEW + # ast-grep. Under CI pnpm applies `--frozen-lockfile` on its own, and the + # lockfile rewritten one step ago satisfies it — a third check that + # resolution matched the pins. Still a registry-only install under the + # default GITHUB_TOKEN, with no NODE_AUTH_TOKEN and no npm identity; the + # root `postinstall` skips itself under CI. + - name: Install + if: steps.detect.outputs.update == 'true' + run: pnpm install + + # The rule schema is upstream's, fetched by the tag the new pin names + # (raw.githubusercontent.com, the same network this job already uses for + # the release notes). The generator reads the version from the bumped + # package.json, and writes no timestamp, so a rerun against an unchanged + # upstream leaves the tree unchanged and `vendor-pr.cjs` stays inert. + - name: Regenerate the ast-grep rule schema + if: steps.detect.outputs.update == 'true' + run: pnpm --filter @taskless/cli generate:ast-grep-schema + + # The consistency check Validate would run, before anything is pushed. It + # imports source directly and needs no build: it holds the constant to + # the pins and spawns the pinned binary to confirm it reports that + # version. A failure here is a bookkeeping defect in this job's own + # output, so the run fails and proposes nothing. + # + # Deliberately ONLY that test. ast-grep-vendor-contract.test.ts holds the + # language list and a set of measured behaviours to the binary, and a new + # ast-grep that adds a language or changes a behaviour fails it — + # correctly, and the fix is a measured edit that belongs in the child + # pull request. Gated here, that signal would become a scheduled run + # failing every Monday with no pull request and no changelog to read; + # left to Validate, it is a red check on a pull request that already + # carries upstream's notes. (Measured: rewinding the pin to 0.45.2 through + # these exact steps passed engine-version-consistency and failed four + # behaviours the contract recorded at 0.45.3.) + - name: Check the bump is self-consistent + if: steps.detect.outputs.update == 'true' + run: >- + pnpm --filter @taskless/cli test --project cli + engine-version-consistency + # "upgrade to", the same verb vale-upgrade.yml uses, because it is the # same operation: moving the pins a user resolves. ast-grep has no # "accept upstream" counterpart, since nothing here repackages it. @@ -159,10 +222,16 @@ jobs: the old version is a different ast-grep on one platform than on the others. \`sg-detect.cjs\` fails the run rather than open a partial bump. - Nothing else is edited. The rewrite replaces a version string in pins - it can already enumerate and refuses if the count does not match, so - it cannot add a dependency or reformat the file — the diff below is - eight version strings, a resolved lockfile, and a changeset. + The rewrite replaces a version string in pins it can already + enumerate and refuses if the count does not match, so it cannot add a + dependency or reformat the file. It also moves \`AST_GREP_VERSION\` in + \`capabilities.ts\` and refetches \`src/generated/ast-grep-rule-schema.json\` + for the ${SG_VERSION} tag, and the workflow ran + \`engine-version-consistency\` against the result before pushing. What + this does **not** carry is judgment: if \`ast-grep-vendor-contract\` + is red on this pull request, ast-grep ${SG_VERSION} added a language + or changed a measured behaviour, and that edit, the \`update\` ledger, + and the release note's prose belong in a child pull request. This pull request rolls: if upstream releases again before it merges, the branch, title, and body are rewritten to the newer version rather @@ -186,4 +255,6 @@ jobs: --title "chore(ast-grep): upgrade to ast-grep ${SG_VERSION}" \ --message "chore(ast-grep): upgrade to ast-grep ${SG_VERSION}" \ --body-file "$body" \ - -- packages/cli/package.json pnpm-lock.yaml "$changeset" + -- packages/cli/package.json pnpm-lock.yaml "$changeset" \ + packages/cli/src/rules/capabilities.ts \ + packages/cli/src/generated/ast-grep-rule-schema.json diff --git a/.github/workflows/vale-upgrade.yml b/.github/workflows/vale-upgrade.yml index b1dcae3f..fb226898 100644 --- a/.github/workflows/vale-upgrade.yml +++ b/.github/workflows/vale-upgrade.yml @@ -35,6 +35,29 @@ # inside the stamp is the release upstream actually tagged, and that is what # gets fetched. # +# THE BOT COMMIT IS SELF-CONSISTENT, AND CHECKED BEFORE IT IS PUSHED. Moving the +# pins alone is not a green commit: `VALE_VERSION` in +# packages/cli/src/rules/capabilities.ts is maintained by hand and +# test/engine-version-consistency.test.ts holds it to the pins (deliberately — +# read that test's docblock before deriving the constant from package.json), +# and src/generated/vale-vocabulary.ts is derived from the binary and +# typechecked against the constant. A commit that moved only the pins therefore +# failed Validate BY CONSTRUCTION. Measured on #368: the bump was red before +# anyone read it, the child #372 had to carry the constant, the regenerated +# vocabulary and a test literal along with its real work, and the stack could +# not use GitHub's landing because its base was red. +# +# So this job now writes everything the consistency check compares — the pins, +# the constant, the lockfile, and the vocabulary regenerated from the new +# binary — and runs that check itself before `vendor-pr.cjs` pushes. If it +# fails, the run fails and nothing is proposed, which is the correct outcome: +# pushing an inconsistent commit is the thing being removed. What stays for a +# human child pull request is judgment, not bookkeeping: the vendor contract +# (the measured `Vale ` block in vale-vendor-contract.test.ts and the +# verdict corpus in vale-schema-contract.test.ts), the `update` ledger, any +# migration, and the changeset prose. Those tests are NOT gated here, on +# purpose: see the check step. +# # Action refs are pinned to commit SHAs; the trailing comment records the tag. name: Upgrade Vale @@ -198,9 +221,14 @@ jobs: done echo "::warning::no probe in five minutes after the publish succeeded reported a newer set than the pins; each attempt is logged above with its result. The daily schedule will retry." - # 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. + # No dependency install yet: the script is zero-dependency CommonJS, and + # the install below has to come AFTER the bump so it resolves the new + # pins. GITHUB_TOKEN is only for the rate limit on the release-notes + # lookup. + # + # `--write` moves the six pins and `VALE_VERSION` in capabilities.ts + # together, the latter to the base version the binary reports. It fails + # the run if the declaration is not found exactly once. # # `--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 @@ -220,6 +248,46 @@ jobs: if: steps.detect.outputs.update == 'true' run: pnpm install --lockfile-only --ignore-scripts + # A real install this time, because the next step spawns the NEW Vale + # and the check after it imports the package's test toolchain. Under CI + # pnpm applies `--frozen-lockfile` on its own, and the lockfile rewritten + # one step ago satisfies it — a third check that resolution matched the + # pins. This is still a registry-only install under the default + # GITHUB_TOKEN, with no NODE_AUTH_TOKEN and no npm identity: the platform + # packages carry no lifecycle script, and the root `postinstall` skips + # itself under CI. + - name: Install + if: steps.detect.outputs.update == 'true' + run: pnpm install + + # The vocabulary is derived by running the vendored binary, and its + # generator refuses a binary that does not report VALE_VERSION — which is + # why the constant had to move first. The output is deterministic for a + # given binary (no timestamp), so a rerun against an unchanged upstream + # leaves the tree unchanged and `vendor-pr.cjs` stays inert. + - name: Regenerate the Vale vocabulary + if: steps.detect.outputs.update == 'true' + run: pnpm --filter @taskless/cli generate:vale-schema + + # The consistency check Validate would run, before anything is pushed. It + # imports source directly and needs no build: it holds the constant to + # the pins and spawns the pinned binary to confirm it reports that + # version. A failure here is a bookkeeping defect in this job's own + # output, so the run fails and proposes nothing. + # + # Deliberately ONLY that test. vale-schema-contract.test.ts and + # vale-vendor-contract.test.ts hold a recorded corpus of what the binary + # DOES, and a new Vale that changes a verdict fails them — correctly, and + # the fix is a measured edit that belongs in the child pull request. Gated + # here, that signal would become a scheduled run failing every morning + # with no pull request and no changelog to read; left to Validate, it is + # a red check on a pull request that already carries upstream's notes. + - name: Check the bump is self-consistent + if: steps.detect.outputs.update == 'true' + run: >- + pnpm --filter @taskless/cli test --project cli + engine-version-consistency + - name: Propose the upgrade if: steps.detect.outputs.update == 'true' env: @@ -262,6 +330,16 @@ jobs: packages are selected by optional dependency, so a straggler left at the old version is a different Vale on one platform than on the others. + It also moves \`VALE_VERSION\` in \`capabilities.ts\` to + \`${BASE_VERSION}\` and regenerates \`src/generated/vale-vocabulary.ts\` + from the new binary, and the workflow ran + \`engine-version-consistency\` against the result before pushing. What + this does **not** carry is judgment: if \`vale-schema-contract\` or + \`vale-vendor-contract\` is red on this pull request, Vale + ${BASE_VERSION} changed a measured behaviour, and that edit, the + \`update\` ledger, any migration, and the release note's prose belong + in a child pull request. + **This is the half that reaches a user.** Republishing the platform packages changes nobody's install, because the CLI pins each one exactly; merging this is what ships the new Vale. @@ -288,4 +366,7 @@ jobs: --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" + -- packages/cli/package.json pnpm-lock.yaml "$changeset" \ + packages/cli/src/rules/capabilities.ts \ + packages/cli/src/generated/vale-vocabulary.ts \ + packages/cli/src/generated/vale-vocabulary-report.md diff --git a/packages/cli/scripts/fetch-ast-grep-schema.ts b/packages/cli/scripts/fetch-ast-grep-schema.ts index d32e9e12..72416a57 100644 --- a/packages/cli/scripts/fetch-ast-grep-schema.ts +++ b/packages/cli/scripts/fetch-ast-grep-schema.ts @@ -49,9 +49,15 @@ if (!response.ok) { const schema = (await response.json()) as Record; -// Add metadata comment +// Provenance, with NO timestamp. `ast-grep-upgrade.yml` runs this on every +// scheduled pass and hands the result to `vendor-pr.cjs`, which force-pushes +// the rolling branch only when the proposed files differ from what is already +// there. A wall-clock stamp made the artifact differ on every run, so an +// upstream that had not moved would still rewrite the branch daily, dismissing +// approvals and restarting CI over nothing. The version and URL are the whole +// provenance; `git log` has the date. schema["$comment"] = - `Generated by fetch-ast-grep-schema.ts at ${new Date().toISOString()} from ast-grep v${version} (${sourceUrl})`; + `Generated by fetch-ast-grep-schema.ts from ast-grep v${version} (${sourceUrl})`; await writeJsonArtifact(OUTPUT_PATH, schema); console.log(` Written to: ${OUTPUT_PATH}`); diff --git a/packages/cli/src/generated/ast-grep-rule-schema.json b/packages/cli/src/generated/ast-grep-rule-schema.json index 30159355..fe162bdb 100644 --- a/packages/cli/src/generated/ast-grep-rule-schema.json +++ b/packages/cli/src/generated/ast-grep-rule-schema.json @@ -743,5 +743,5 @@ "additionalProperties": true } }, - "$comment": "Generated by fetch-ast-grep-schema.ts at 2026-09-15T23:43:40.361Z from ast-grep v0.45.3 (https://raw.githubusercontent.com/ast-grep/ast-grep/0.45.3/schemas/rule.json)" + "$comment": "Generated by fetch-ast-grep-schema.ts from ast-grep v0.45.3 (https://raw.githubusercontent.com/ast-grep/ast-grep/0.45.3/schemas/rule.json)" } diff --git a/packages/cli/test/engine-version-consistency.test.ts b/packages/cli/test/engine-version-consistency.test.ts index 45832546..25f920e2 100644 --- a/packages/cli/test/engine-version-consistency.test.ts +++ b/packages/cli/test/engine-version-consistency.test.ts @@ -35,6 +35,15 @@ import { resolvePlatformBinary } from "../src/rules/platform-binary"; * disagreed with both. Together they are what makes the published constant a * fact rather than a claim. * + * The bot pin bumps (`vale-upgrade.yml`, `ast-grep-upgrade.yml`) move the + * constant with the pins and run this file before they push, so a bot commit + * satisfies link 1 by construction. It used not to: a bump that moved only + * the pins was red here before anyone had read it (taskless/cli#368), and the + * child (#372) had to carry the constant along with its real work. The check + * is unchanged by that. It is what holds a HUMAN bump to the same standard, + * and what would catch the bot's rewrite if the declaration it anchors on + * ever moved. + * * This is deliberately NOT derived from `optionalDependencies` at build time. * The generator team asked for that, reasoning that a hand-maintained constant * is a second copy of a fact we already hold. The reasoning is right and the diff --git a/packages/cli/test/reconcile-marker.test.ts b/packages/cli/test/reconcile-marker.test.ts index db50fd48..d34b3229 100644 --- a/packages/cli/test/reconcile-marker.test.ts +++ b/packages/cli/test/reconcile-marker.test.ts @@ -5,6 +5,8 @@ import { join, resolve } from "node:path"; import { promisify } from "node:util"; import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { AST_GREP_VERSION, VALE_VERSION } from "../src/rules/capabilities"; + const execFileAsync = promisify(execFile); const binPath = resolve(import.meta.dirname, "../dist/index.js"); @@ -85,8 +87,15 @@ describe("recording a rules reconciliation", () => { const rules = await readRules(); expect(rules?.reconciledTo).toBe(version); // Engine versions are the input a later differential needs. Recorded here - // and nowhere else, so an upgrade cannot silently refresh them. - expect(rules?.engines).toEqual({ sg: "0.45.3", vale: "3.22.0" }); + // and nowhere else, so an upgrade cannot silently refresh them. Asserted + // against the constants rather than as literals: this test is about the + // marker CARRYING the engine versions, and engine-version-consistency + // already holds the constants to the pins. A literal here was one more + // thing a bot pin bump broke by construction (taskless/cli#368). + expect(rules?.engines).toEqual({ + sg: AST_GREP_VERSION, + vale: VALE_VERSION, + }); }); it("reports the marker through info", async () => {