From 4dfea038d65022bef2a839bd03a851cfc547c66e Mon Sep 17 00:00:00 2001 From: Jakob Heuser Date: Mon, 21 Sep 2026 16:35:34 -0700 Subject: [PATCH 1/5] chore(ci): move the engine version constant with the pins in vendor upgrades sg-detect.cjs --write and vale-upgrade-detect.cjs --write now also rewrite AST_GREP_VERSION / VALE_VERSION in packages/cli/src/rules/capabilities.ts, via a new bumpVersionConstant in pin-bump.cjs that anchors on the exact declaration line, fails unless it is found exactly once, and leaves every other byte alone. Vale's constant takes the base version the binary reports, not the stamped npm version. engine-version-consistency.test.ts holds the constant to the pins, so a bot commit that moved only the pins failed Validate by construction (taskless/cli#368). The test stays; the bot commit becomes consistent. --- .github/scripts/pin-bump.cjs | 57 ++++++++++++- .github/scripts/pin-bump.test.cjs | 85 +++++++++++++++++++- .github/scripts/sg-detect.cjs | 41 +++++++++- .github/scripts/sg-detect.test.cjs | 53 +++++++++++- .github/scripts/vale-upgrade-detect.cjs | 39 ++++++++- .github/scripts/vale-upgrade-detect.test.cjs | 81 ++++++++++++++++++- 6 files changed, 345 insertions(+), 11 deletions(-) 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..3c3476c0 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"); @@ -294,6 +316,19 @@ async function main({ } writeFileSync(packageJsonPath, bumped); log(`Rewrote ${count} pins in ${packageJsonPath} to ${upstream}.`); + + // 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 pushed, rather than + // failing Validate later. + const { source: constants, from } = bumpVersionConstant( + readFileSync(capabilitiesPath, "utf8"), + { name: VERSION_CONSTANT, to: upstream } + ); + writeFileSync(capabilitiesPath, constants); + 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..fc445362 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,25 @@ 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/ + ); }); /** diff --git a/.github/scripts/vale-upgrade-detect.cjs b/.github/scripts/vale-upgrade-detect.cjs index b0049ca8..fd971c93 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"); @@ -244,6 +265,20 @@ async function main({ } writeFileSync(packageJsonPath, bumped); log(`Rewrote ${count} pins in ${packageJsonPath} to ${upstream}.`); + + // 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 pushed, rather than failing Validate later. + const base = baseVersion(upstream); + const { source: constants, from } = bumpVersionConstant( + readFileSync(capabilitiesPath, "utf8"), + { name: VERSION_CONSTANT, to: base } + ); + writeFileSync(capabilitiesPath, constants); + log( + `Rewrote ${VERSION_CONSTANT} in ${capabilitiesPath}: ${from} -> ${base}.` + ); } // The changelog a reviewer wants is UPSTREAM's, not ours. Our stamp says when diff --git a/.github/scripts/vale-upgrade-detect.test.cjs b/.github/scripts/vale-upgrade-detect.test.cjs index 94e4b261..f36bd009 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,56 @@ 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/ + ); +}); + 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"), From 5ccd2809f84d96f5dd97aeec9278ff9d205c7cc2 Mon Sep 17 00:00:00 2001 From: Jakob Heuser Date: Mon, 21 Sep 2026 16:41:36 -0700 Subject: [PATCH 2/5] chore(cli): drop the timestamp from the ast-grep rule schema's provenance comment ast-grep-upgrade.yml is about to refetch the schema on every scheduled run and hand it to vendor-pr.cjs, which force-pushes the rolling branch only when the proposed files differ. A wall-clock stamp in $comment made the artifact differ on every run, so an unchanged upstream would still rewrite the branch. The version and URL are the provenance; git has the date. Regenerated: the only byte that changed is that comment. --- packages/cli/scripts/fetch-ast-grep-schema.ts | 10 ++++++++-- packages/cli/src/generated/ast-grep-rule-schema.json | 2 +- 2 files changed, 9 insertions(+), 3 deletions(-) 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)" } From 657f4aede3fd44c4eeb23f3f3f59806958dc35b0 Mon Sep 17 00:00:00 2001 From: Jakob Heuser Date: Mon, 21 Sep 2026 16:41:39 -0700 Subject: [PATCH 3/5] chore(ci): regenerate and verify the vendor upgrade commit before pushing it Both upgrade workflows now install after the pin bump, regenerate the engine artifact from the new version (the Vale vocabulary from the new binary; the ast-grep rule schema from upstream's tag), run engine-version-consistency against the result, and only then hand the files to vendor-pr.cjs. A failing check fails the run and proposes nothing. The contract tests (vale-schema-contract, vale-vendor-contract, ast-grep-vendor-contract) are deliberately not part of that gate: they record what the binary does, and a new engine that changes a behaviour should surface as a red Validate on a pull request carrying upstream's notes, not as a scheduled run failing with nothing to read. That edit, the ledger, any migration and the changeset prose stay in the child. Measured locally by driving the exact steps against Vale 3.22.0 (ahead on npm today) and ast-grep 0.45.2 (rewound), see the workflow comments. --- .github/workflows/ast-grep-upgrade.yml | 85 ++++++++++++++++++++++-- .github/workflows/vale-upgrade.yml | 89 ++++++++++++++++++++++++-- 2 files changed, 163 insertions(+), 11 deletions(-) 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 From 49b83a5631a452e87be043015fac2172fda53c30 Mon Sep 17 00:00:00 2001 From: Jakob Heuser Date: Mon, 21 Sep 2026 16:42:56 -0700 Subject: [PATCH 4/5] test(cli): assert the reconcile marker against the engine constants, not pin literals reconcile-marker.test.ts pinned { sg: "0.45.3", vale: "3.21.0" } as literals. The test is about the marker carrying the engine versions, not their values, and engine-version-consistency already holds the constants to the pins; the literal was one more thing a bot pin bump broke by construction. The other version literals under test/ are measured facts ("changed at 0.45.3", a "Vale 3.21.0" block) and stay. engine-version-consistency's docblock now says the bot bumps satisfy it by construction and what the check is still for. --- .../cli/test/engine-version-consistency.test.ts | 9 +++++++++ packages/cli/test/reconcile-marker.test.ts | 13 +++++++++++-- 2 files changed, 20 insertions(+), 2 deletions(-) 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 () => { From 46c7468395ec1c0fea38737b27e9006d55cba785 Mon Sep 17 00:00:00 2001 From: Jakob Heuser Date: Mon, 21 Sep 2026 17:22:59 -0700 Subject: [PATCH 5/5] chore(ci): write the pins and the version constant together, or neither sg-detect.cjs and vale-upgrade-detect.cjs computed the package.json rewrite, wrote it, and only then computed the capabilities.ts rewrite, so a constant declaration that had moved left package.json bumped on disk without its constant, which is exactly the half-done bump the surrounding comment promised could not happen. Both rewrites are now computed before either file is written; a refused run leaves a clean tree. One test per script pins that. vale-upgrade-detect.cjs also computes baseVersion(upstream) once, in main(), instead of at three call sites. --- .github/scripts/sg-detect.cjs | 20 +++++++----- .github/scripts/sg-detect.test.cjs | 30 +++++++++++++++++ .github/scripts/vale-upgrade-detect.cjs | 19 +++++++---- .github/scripts/vale-upgrade-detect.test.cjs | 34 ++++++++++++++++++++ 4 files changed, 89 insertions(+), 14 deletions(-) diff --git a/.github/scripts/sg-detect.cjs b/.github/scripts/sg-detect.cjs index 3c3476c0..91f01d43 100644 --- a/.github/scripts/sg-detect.cjs +++ b/.github/scripts/sg-detect.cjs @@ -292,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"); @@ -314,18 +317,19 @@ async function main({ `expected to rewrite ${pins.size} @ast-grep/cli* pins, rewrote ${count}` ); } - writeFileSync(packageJsonPath, bumped); - log(`Rewrote ${count} pins in ${packageJsonPath} to ${upstream}.`); // 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 pushed, rather than - // failing Validate later. + // 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}.` ); diff --git a/.github/scripts/sg-detect.test.cjs b/.github/scripts/sg-detect.test.cjs index fc445362..07de2590 100644 --- a/.github/scripts/sg-detect.test.cjs +++ b/.github/scripts/sg-detect.test.cjs @@ -360,6 +360,36 @@ test("sg-detect: --write fails when AST_GREP_VERSION is not declared once", asyn ); }); +/** + * 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 }); + } +}); + /** * 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 diff --git a/.github/scripts/vale-upgrade-detect.cjs b/.github/scripts/vale-upgrade-detect.cjs index fd971c93..13c67fff 100644 --- a/.github/scripts/vale-upgrade-detect.cjs +++ b/.github/scripts/vale-upgrade-detect.cjs @@ -238,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( @@ -251,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, { @@ -263,19 +271,19 @@ async function main({ `expected to rewrite ${pins.size} ${PIN_PREFIX}* pins, rewrote ${count}` ); } - writeFileSync(packageJsonPath, bumped); - log(`Rewrote ${count} pins in ${packageJsonPath} to ${upstream}.`); // 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 pushed, rather than failing Validate later. - const base = baseVersion(upstream); + // 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}.` ); @@ -284,7 +292,6 @@ async function main({ // 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, @@ -300,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 f36bd009..3edcd4a2 100644 --- a/.github/scripts/vale-upgrade-detect.test.cjs +++ b/.github/scripts/vale-upgrade-detect.test.cjs @@ -281,6 +281,40 @@ test("--write fails when VALE_VERSION is not declared exactly once", async () => ); }); +/** + * 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"),