From 5937436f945b8db041bc3ba9faf12fa5212a6989 Mon Sep 17 00:00:00 2001 From: Jakob Heuser Date: Sun, 9 Aug 2026 21:18:45 -0700 Subject: [PATCH 1/2] fix(vale): gate publishing on the pinned Vale version, not the manifest file The publish path fires on a push to main touching vale-manifest.json, and a `paths:` filter cannot see WHY the file changed. A reworded comment, a reformat, or a digest correction is indistinguishable from a version bump, and each one published six packages at a fresh -. Nothing downstream absorbs that: every stamp is novel by construction, so there was no second line of defense. Add a credential-free `gate` job ahead of `prepare` that asks whether the pinned Vale version is already published, and skips when it is. It runs before prepare downloads ~60 MB, so a skip is cheap. design.md D5 argued an already-published check cannot work here. That is true of the STAMPED version and false of the BASE version, and the difference is the whole design: "is 3.17.1-20260810000724 published?" is always no, while "has anything been published for Vale 3.17.1?" is answered by a published 3.17.1 or any 3.17.1-* stamp. D5 is amended to draw that line, so the gate does not read as contradicting it. Two properties kept deliberately: - An explicit workflow_dispatch passes --force and is never suppressed. A human asking for a publish gets one. - A skip requires ALL six packages to carry the pinned version. Checking one would silently skip a half-published set, so the gate doubles as partial-release repair. Blocks #91: that PR archives this change, so its design.md must carry the amended D5 before it lands. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Jwc9FFroR3mTZ4hLiSkkX3 --- .github/scripts/vale-gate.cjs | 159 ++++++++++++++++++ .github/scripts/vale-gate.test.cjs | 133 +++++++++++++++ .github/workflows/vale-binaries.yml | 37 +++- .../add-vale-binary-packages/design.md | 4 +- 4 files changed, 329 insertions(+), 4 deletions(-) create mode 100644 .github/scripts/vale-gate.cjs create mode 100644 .github/scripts/vale-gate.test.cjs diff --git a/.github/scripts/vale-gate.cjs b/.github/scripts/vale-gate.cjs new file mode 100644 index 0000000..8ce8d76 --- /dev/null +++ b/.github/scripts/vale-gate.cjs @@ -0,0 +1,159 @@ +// SPDX-License-Identifier: MIT +/** + * Publish gate — decide whether the pinned Vale version still needs publishing. + * + * WHY THIS EXISTS. The publish path fires on a push to main that touches + * vale-manifest.json (plus explicit dispatch). That `paths:` filter cannot see + * WHY the file changed: correcting a typo in its comment block, reformatting + * it, or fixing a digest all look identical to a version bump, and each one + * would publish six fresh packages. Publishing is not idempotent here — every + * run stamps -, a version npm has never seen — so + * nothing downstream can absorb the mistake. + * + * WHY A "IS IT ALREADY PUBLISHED" CHECK WORKS HERE, given design.md D5 says it + * cannot: D5 is right about the STAMPED version, which is novel by construction + * and so always answers "not published". It is the BASE version that is + * checkable. "Has anything been published for Vale 3.17.1?" is answered by + * looking for a published version equal to 3.17.1 or beginning with `3.17.1-`, + * which is exactly the set of stamps this workflow can mint for it. + * + * WHAT IT DELIBERATELY DOES NOT DO. It never suppresses an explicit dispatch: + * a human asking for a publish gets one, stamp collision or not. And it skips + * only when ALL six packages already carry the pinned base version. Checking a + * single package would silently skip a half-published set, so requiring all six + * makes the gate double as partial-release repair — it re-runs precisely the + * case the publish loop's failure aggregation is there to report. + * + * Usage: + * node .github/scripts/vale-gate.cjs [--force] + * + * --force publish regardless of what is already on the registry (dispatch). + * + * Outputs (appended to $GITHUB_OUTPUT when set): + * should_publish "true" | "false" + */ +const { appendFileSync, readFileSync } = require("node:fs"); +const { join } = require("node:path"); + +const { assertManifest } = require("./vale-release.cjs"); + +const MANIFEST_PATH = join(__dirname, "vale-manifest.json"); +const REGISTRY = "https://registry.npmjs.org"; + +function setOutput(key, value) { + const file = process.env.GITHUB_OUTPUT; + if (file) { + appendFileSync(file, `${key}=${value}\n`); + } +} + +/** + * A published version counts as covering `pinned` when it is the bare version + * or one of this workflow's stamps for it. The `-` is required: without it + * `3.1.1` would be judged as covering pinned `3.1`, and a real upstream bump + * would be skipped. + */ +function coversVersion(versions, pinned) { + return versions.some( + (version) => version === pinned || version.startsWith(`${pinned}-`) + ); +} + +/** + * A package that does not exist yet reads as "nothing published", not as an + * error — that is the ordinary state before the one-time bootstrap publish, and + * treating a 404 as a failure would wedge the gate closed exactly when the + * packages most need publishing. + */ +async function fetchPublishedVersions(packageName) { + const url = `${REGISTRY}/${packageName.replace("/", "%2F")}`; + const response = await fetch(url, { + headers: { accept: "application/json" }, + }); + if (response.status === 404) { + return []; + } + if (!response.ok) { + throw new Error(`GET ${url} responded ${response.status}`); + } + const document = await response.json(); + return Object.keys(document.versions ?? {}); +} + +/** + * Pure decision, separated from the network so the table of cases is testable: + * forced, nothing published, everything published, and a partial set. + */ +function planPublish({ manifest, publishedByPackage, forced }) { + const missing = manifest.platforms + .filter( + (platform) => + !coversVersion( + publishedByPackage[platform.package] ?? [], + manifest.valeVersion + ) + ) + .map((platform) => platform.package); + + if (forced) { + return { shouldPublish: true, missing, reason: "forced" }; + } + return { + shouldPublish: missing.length > 0, + missing, + reason: missing.length > 0 ? "missing" : "already-published", + }; +} + +async function main({ + argv = process.argv.slice(2), + published = fetchPublishedVersions, +} = {}) { + const forced = argv.includes("--force"); + const manifest = assertManifest( + JSON.parse(readFileSync(MANIFEST_PATH, "utf8")) + ); + + const publishedByPackage = {}; + for (const platform of manifest.platforms) { + publishedByPackage[platform.package] = await published(platform.package); + } + + const plan = planPublish({ manifest, publishedByPackage, forced }); + + console.log(`Vale ${manifest.valeVersion}`); + for (const platform of manifest.platforms) { + const covered = !plan.missing.includes(platform.package); + console.log( + ` ${covered ? "published" : "MISSING "} ${platform.package}` + ); + } + + if (plan.reason === "forced") { + console.log( + `\nExplicitly dispatched — publishing regardless (${plan.missing.length} of ${manifest.platforms.length} not yet on the registry).` + ); + } else if (plan.shouldPublish) { + console.log( + `\n${plan.missing.length} of ${manifest.platforms.length} package(s) lack Vale ${manifest.valeVersion}. Publishing.` + ); + } else { + console.log( + `\nEvery package already carries Vale ${manifest.valeVersion}. Nothing to publish; dispatch with phase=publish to force.` + ); + } + + setOutput("should_publish", String(plan.shouldPublish)); + return plan; +} + +// Exported so vale-gate.test.cjs can drive main() with the registry stubbed, +// and can exercise planPublish()'s cases without any network at all. +module.exports = { coversVersion, planPublish, main }; + +if (require.main === module) { + main().catch((error) => { + console.error(`\nvale-gate failed: ${error.message}`); + process.exitCode = 1; + }); +} diff --git a/.github/scripts/vale-gate.test.cjs b/.github/scripts/vale-gate.test.cjs new file mode 100644 index 0000000..7a77b68 --- /dev/null +++ b/.github/scripts/vale-gate.test.cjs @@ -0,0 +1,133 @@ +// SPDX-License-Identifier: MIT +"use strict"; + +/** + * Tests for vale-gate.cjs — the decision that keeps an unrelated edit to + * vale-manifest.json from publishing six packages. + * + * The interesting cases are all "what does the registry already have", so the + * registry is stubbed throughout and nothing here touches the network. The + * committed manifest is only read. + */ + +const test = require("node:test"); +const assert = require("node:assert/strict"); +const { mkdtempSync, readFileSync, rmSync } = require("node:fs"); +const { tmpdir } = require("node:os"); +const { join } = require("node:path"); + +const { coversVersion, planPublish, main } = require("./vale-gate.cjs"); + +const MANIFEST = JSON.parse( + readFileSync(join(__dirname, "vale-manifest.json"), "utf8") +); +const PINNED = MANIFEST.valeVersion; +const PACKAGES = MANIFEST.platforms.map((platform) => platform.package); + +/** A registry stub answering from a {package: [versions]} table. */ +const registry = (table) => async (packageName) => table[packageName] ?? []; + +const allPublished = () => + Object.fromEntries( + PACKAGES.map((name) => [name, [`${PINNED}-20260101000000`]]) + ); + +test("coversVersion matches the bare version and this workflow's stamps", () => { + assert.equal(coversVersion(["3.17.1"], "3.17.1"), true); + assert.equal(coversVersion(["3.17.1-20260810000724"], "3.17.1"), true); + assert.equal(coversVersion([], "3.17.1"), false); + assert.equal(coversVersion(["3.17.0", "3.16.9"], "3.17.1"), false); +}); + +test("coversVersion does not treat a longer version as covering a shorter one", () => { + // Without the `-` separator, pinned "3.1" would read 3.1.1 as covered and a + // real upstream bump would be silently skipped. + assert.equal(coversVersion(["3.1.1"], "3.1"), false); + assert.equal(coversVersion(["3.17.10"], "3.17.1"), false); +}); + +test("publishes when nothing is on the registry yet (pre-bootstrap)", () => { + const plan = planPublish({ + manifest: MANIFEST, + publishedByPackage: {}, + forced: false, + }); + assert.equal(plan.shouldPublish, true); + assert.equal(plan.reason, "missing"); + assert.deepEqual(plan.missing, PACKAGES); +}); + +test("skips when every package already carries the pinned version", () => { + const plan = planPublish({ + manifest: MANIFEST, + publishedByPackage: allPublished(), + forced: false, + }); + assert.equal(plan.shouldPublish, false); + assert.equal(plan.reason, "already-published"); + assert.deepEqual(plan.missing, []); +}); + +test("publishes when the set is only partially published", () => { + const published = allPublished(); + delete published[PACKAGES[3]]; + const plan = planPublish({ + manifest: MANIFEST, + publishedByPackage: published, + forced: false, + }); + assert.equal(plan.shouldPublish, true); + assert.deepEqual(plan.missing, [PACKAGES[3]]); +}); + +test("an explicit dispatch publishes even when everything is already out", () => { + const plan = planPublish({ + manifest: MANIFEST, + publishedByPackage: allPublished(), + forced: true, + }); + assert.equal(plan.shouldPublish, true); + assert.equal(plan.reason, "forced"); +}); + +test("main writes should_publish=false when the version is already out", async () => { + const directory = mkdtempSync(join(tmpdir(), "vale-gate-")); + const outputFile = join(directory, "output"); + const previous = process.env.GITHUB_OUTPUT; + process.env.GITHUB_OUTPUT = outputFile; + try { + const plan = await main({ argv: [], published: registry(allPublished()) }); + assert.equal(plan.shouldPublish, false); + assert.match(readFileSync(outputFile, "utf8"), /should_publish=false/); + } finally { + if (previous === undefined) delete process.env.GITHUB_OUTPUT; + else process.env.GITHUB_OUTPUT = previous; + rmSync(directory, { recursive: true, force: true }); + } +}); + +test("main writes should_publish=true when --force is passed", async () => { + const directory = mkdtempSync(join(tmpdir(), "vale-gate-")); + const outputFile = join(directory, "output"); + const previous = process.env.GITHUB_OUTPUT; + process.env.GITHUB_OUTPUT = outputFile; + try { + const plan = await main({ + argv: ["--force"], + published: registry(allPublished()), + }); + assert.equal(plan.shouldPublish, true); + assert.match(readFileSync(outputFile, "utf8"), /should_publish=true/); + } finally { + if (previous === undefined) delete process.env.GITHUB_OUTPUT; + else process.env.GITHUB_OUTPUT = previous; + rmSync(directory, { recursive: true, force: true }); + } +}); + +test("a 404 package reads as unpublished rather than failing the gate", async () => { + // fetchPublishedVersions maps 404 -> []; the stub models that contract. + const plan = await main({ argv: [], published: registry({}) }); + assert.equal(plan.shouldPublish, true); + assert.deepEqual(plan.missing, PACKAGES); +}); diff --git a/.github/workflows/vale-binaries.yml b/.github/workflows/vale-binaries.yml index f72b024..b071a6d 100644 --- a/.github/workflows/vale-binaries.yml +++ b/.github/workflows/vale-binaries.yml @@ -180,13 +180,44 @@ jobs: )" \ --label skip-changeset + # Is there anything to publish? The push trigger fires on ANY edit to + # vale-manifest.json — a reworded comment, a reformat, a digest correction — + # and it cannot tell those from a version bump. Without this gate each of them + # publishes six packages at a fresh - stamp, which + # nothing downstream can absorb because every stamp is novel by construction. + # + # Cheap on purpose: it reads the registry and decides BEFORE `prepare` + # downloads ~60 MB of third-party archives. Credential-free, like prepare. + gate: + name: "publish gate" + if: >- + github.event_name == 'push' || + (github.event_name == 'workflow_dispatch' && inputs.phase == 'publish') + runs-on: ubuntu-latest + permissions: + contents: read # checkout only + outputs: + should_publish: ${{ steps.gate.outputs.should_publish }} + steps: + - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 + with: + persist-credentials: false + - uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4 + with: + node-version: 24 + # --force on dispatch: an explicit human request publishes even when the + # pinned version is already out. Only the automatic push path is gated. + - id: gate + run: | + node .github/scripts/vale-gate.cjs \ + ${{ github.event_name == 'workflow_dispatch' && '--force' || '' }} + # Credential-free. Downloads third-party bytes, verifies them against the # reviewed digests, and produces tarballs. Cannot publish anything. prepare: name: Fetch, verify, stamp, pack - if: >- - github.event_name == 'push' || - (github.event_name == 'workflow_dispatch' && inputs.phase == 'publish') + needs: gate + if: needs.gate.outputs.should_publish == 'true' runs-on: ubuntu-latest permissions: contents: read # checkout only diff --git a/openspec/changes/add-vale-binary-packages/design.md b/openspec/changes/add-vale-binary-packages/design.md index e84a003..457f1b1 100644 --- a/openspec/changes/add-vale-binary-packages/design.md +++ b/openspec/changes/add-vale-binary-packages/design.md @@ -71,7 +71,9 @@ Nobody has to notice a Vale release, and nothing is published on bytes a human h Safe to automate because **publishing a platform package changes nothing on its own** — the CLI pins an exact version (D8), so a newly published package is inert until someone bumps that pin. Two independent gates, then: review to publish the package, and a separate deliberate bump to adopt it. -This also avoids a trap: a freshly stamped timestamp is never already on npm, so any "is this version published?" check would fire on every run. The upstream-version comparison, not a published-version check, is what bounds releases. +This also avoids a trap, but only a specific one, and the distinction is load-bearing. A freshly stamped timestamp is never already on npm, so a check against the **stamped** version would answer "not published" every time and could never suppress anything. A check against the **base** version is a different question and is answerable: "has anything been published for Vale 3.17.1?" is satisfied by a published `3.17.1` or any `3.17.1-…` stamp, which is exactly the set this workflow can mint for it. + +That check is required, not optional. The publish phase fires on a push to `main` touching `vale-manifest.json`, and a `paths:` filter cannot see _why_ the file changed — a reworded comment, a reformat, or a digest correction is indistinguishable from a version bump, and each would publish six packages nobody asked for. So the publish path is gated on the base-version comparison (`.github/scripts/vale-gate.cjs`), which skips only when **all** platform packages already carry the pinned version; a partial set still publishes, so the gate doubles as partial-release repair. An explicit `workflow_dispatch` passes `--force` and is never suppressed — a human asking for a publish gets one. - **Alternative — route these through `release.yml`:** rejected; it is built around changesets and a published-version check, neither of which applies here, and coupling them would mean a Vale release could not ship without a CLI release. From cd1da85ed8a7be743935262a66d40203d8af4eae Mon Sep 17 00:00:00 2001 From: Jakob Heuser Date: Sun, 9 Aug 2026 21:30:57 -0700 Subject: [PATCH 2/2] docs(vale): correct the workflow header that still contradicted the gate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The PR that added the gate amended design.md D5 to distinguish a stamped-version check (impossible) from a base-version check (the gate), specifically so nobody would later delete the gate for disagreeing with the design — and left the near-identical paragraph in the workflow's own header, a few dozen lines above the job it describes, still asserting that such a check "could never suppress anything". Fix the header to draw the same distinction, and list `gate` in the TWO PHASES summary alongside prepare and publish. Also run the six registry lookups concurrently. They are independent, and the job's whole justification is deciding cheaply before prepare downloads ~60 MB; sequential awaits made the gate six round trips deep for no reason. Order is unaffected — `missing` is built by filtering manifest.platforms, not by completion order. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Jwc9FFroR3mTZ4hLiSkkX3 --- .github/scripts/vale-gate.cjs | 15 +++++++++++---- .github/workflows/vale-binaries.yml | 24 ++++++++++++++++++------ 2 files changed, 29 insertions(+), 10 deletions(-) diff --git a/.github/scripts/vale-gate.cjs b/.github/scripts/vale-gate.cjs index 8ce8d76..116ca1e 100644 --- a/.github/scripts/vale-gate.cjs +++ b/.github/scripts/vale-gate.cjs @@ -114,10 +114,17 @@ async function main({ JSON.parse(readFileSync(MANIFEST_PATH, "utf8")) ); - const publishedByPackage = {}; - for (const platform of manifest.platforms) { - publishedByPackage[platform.package] = await published(platform.package); - } + // Concurrent, not sequential: the six lookups are independent, and this job + // exists to decide cheaply BEFORE prepare downloads ~60 MB. Sequential awaits + // would make the gate six round trips deep for no reason. + const publishedByPackage = Object.fromEntries( + await Promise.all( + manifest.platforms.map(async (platform) => [ + platform.package, + await published(platform.package), + ]) + ) + ); const plan = planPublish({ manifest, publishedByPackage, forced }); diff --git a/.github/workflows/vale-binaries.yml b/.github/workflows/vale-binaries.yml index b071a6d..6a825ab 100644 --- a/.github/workflows/vale-binaries.yml +++ b/.github/workflows/vale-binaries.yml @@ -16,19 +16,31 @@ # It publishes nothing. # # publish Runs on the push to main that merges that pull request — i.e. once -# a human has reviewed the digests. Split further into `prepare` and -# `publish` below. +# a human has reviewed the digests. Split further into `gate`, +# `prepare`, and `publish` below, where `gate` decides whether the +# pinned version still needs publishing at all. # # A single job that discovered a digest and then verified downloads against the # digest it had just discovered would verify nothing. Splitting the phases is # what makes the automation trustworthy: nothing is published on bytes nobody # signed off on, and nobody has to notice a Vale release for the process to run. # -# WHAT BOUNDS A RUN is the upstream-version comparison, and only that. A "is -# this version already on npm?" check — the thing release.yml uses — cannot work +# WHAT BOUNDS A RUN is the upstream-version comparison plus the `gate` job, and +# the distinction between them is worth stating precisely because half of it is +# a trap (design D5). +# +# A check against the STAMPED version — the thing release.yml uses — cannot work # here: every publish stamps -, a version npm has -# never seen, so such a check would answer "not published" every time and could -# never suppress anything (design D5). +# never seen, so it would answer "not published" every time and could never +# suppress anything. +# +# A check against the BASE version is a different question and does work. "Has +# anything been published for Vale 3.17.1?" is satisfied by a published 3.17.1 +# or any 3.17.1-* stamp, which is exactly the set this workflow can mint for it. +# `gate` runs that check, because the push trigger below fires on ANY edit to +# vale-manifest.json and a `paths:` filter cannot see why the file changed — a +# reworded comment would otherwise publish six packages. An explicit dispatch +# passes --force and is never suppressed. # # WHY prepare AND publish ARE SEPARATE JOBS: `prepare` downloads third-party # bytes off the internet. It holds `contents: read`, no environment, and no