diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ec0226cd6..1bf15fdbe 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -217,18 +217,16 @@ jobs: cache: 'pnpm' registry-url: 'https://registry.npmjs.org' - # bulma-ui, create-bestax and bestax-mcp publish via - # @semantic-release/npm, which shells out to `npm publish`. OIDC trusted - # publishing requires npm >= 11.5.1, so pin a known-good npm even though - # dependencies are managed by pnpm. + # The npm pin is gone: it existed because OIDC trusted publishing needs + # npm >= 11.5.1, and nothing here runs `npm publish` any more. Every + # package publishes with `pnpm publish` (#436 for bestax-migrate, #532 + # for the other three), which carries its own OIDC exchange. # - # bestax-migrate is the exception and does not need this: it publishes - # with `pnpm publish` (#436), which carries its own OIDC exchange, because - # `npm publish` does not resolve the `workspace:` protocol it keeps in - # devDependencies (#412). Its release step below is otherwise identical. - - name: Update npm for OIDC trusted publishing - run: npm install -g npm@latest - + # This does NOT mean the job has no use for npm. @semantic-release/npm + # keeps its prepare step, which shells out to `npm version` to write the + # release version into package.json. So the runner still needs a working + # npm and setup-node still needs its registry-url; what no longer + # applies is the version floor, because that came from publishing. - name: Install dependencies run: pnpm install --frozen-lockfile @@ -281,6 +279,9 @@ jobs: # No NPM_TOKEN: publishing authenticates via OIDC trusted publishing. # A present NPM_TOKEN takes precedence over OIDC and reintroduces the # EOTP (one-time password) failure on 2FA-protected accounts. + # All four release steps are identical on purpose: each package's + # release.config.js decides how it publishes, and since #532 all four + # answer `pnpm publish` through the same shared helper. - name: Semantic Release (bulma-ui) working-directory: bulma-ui env: diff --git a/CLAUDE.md b/CLAUDE.md index 5775a08f5..57816d53a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -91,8 +91,9 @@ Full versioning details (breaking-change footers, tag formats): `VERSIONING.md`. - Isolated node linker: undeclared (phantom) dependencies fail — declare everything you import. - **How a package publishes decides what its manifest may contain.** `npm publish` resolves no pack-time protocol at all, so a package published that way must not ship one — the - tarball becomes uninstallable (#412). bestax-migrate hands its publish step to - `pnpm publish` instead (#436), which buys it a **narrow** exemption: + tarball becomes uninstallable (#412). Every package here hands its publish step to + `pnpm publish` instead (#436 for bestax-migrate, #532 for the rest), through the shared + `scripts/lib/pnpm-publish.mjs`, which buys each a **narrow** exemption: `workspace:`/`catalog:` in **devDependencies** only. `jsr:` becomes an aliased `npm:@jsr/…` specifier and `link:`/`portal:`/`file:` are not rewritten at all, so those four are a violation in **any** section, exemption or not. `workspace:`/`catalog:` are diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index ac52d14ce..e85becc91 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -255,7 +255,14 @@ no `npm publish`, no tag, no GitHub release. > **Safe to run; never publishes:** everything above. The only things that actually publish are > `pnpm exec semantic-release` **without** `--dry-run` (CI-only, on merge to `main`) and a manual -> `npm publish` / `pnpm publish` — none of which is in this list. +> `pnpm publish --provenance --embed-readme --access public` — neither of which is in this +> list. Those flags are not optional: pnpm defaults `embed-readme` to false and ignores +> `publishConfig.provenance`, which no package carries, so a bare `pnpm publish` ships +> unattested and loses the npm page's README. Every package publishes with `pnpm publish`, +> and each one's `prepack` and `prepublishOnly` hooks refuse the publishers they recognise as not +> being pnpm, so a stray `npm publish` or `npm pack` exits with an explanation rather than +> shipping an unresolved specifier (#412). Both hooks are skipped by `--ignore-scripts`, and +> neither travels with a tarball that was packed elsewhere. --- diff --git a/SECURITY.md b/SECURITY.md index 0a3c56569..49b941cfe 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -44,11 +44,20 @@ Measures active in this repository and its release pipeline: one deliberate exception: it re-resolves to pin the requested React major for testing, and never publishes.) - **npm provenance** — every release carries a signed attestation linking the - tarball to the exact commit and CI run that built it. Three packages request - it with `publishConfig.provenance`; bestax-migrate publishes with - `pnpm publish`, which does not read that field, so it passes `--provenance` - on the command instead and carries no `publishConfig.provenance` at all - (having one there would imply the flag was redundant). + tarball to the exact commit and CI run that built it. Releases come from CI + and nowhere else, which is what backs that claim: every package publishes + with `pnpm publish`, which does not read `publishConfig.provenance`, so the + `--provenance` flag CI passes is the only thing turning it on. A hand publish + that omitted it would ship unattested, which is why the `prepublishOnly` + guard prints the flags before letting one through. + No package carries a `publishConfig.provenance` at all — having one there + would imply the flag was redundant, and dropping the flag is the quiet way to + lose attestations entirely (#436, #532). +- **Licence text comes from the workspace root** — no package carries its own + `LICENSE` file, and `pnpm publish` copies the root one into every tarball. + npm did not, so releases before #532 shipped none. It is the right file + today because every package is MIT, but a package published under different + terms would need its own `LICENSE` rather than inheriting this one. - **OIDC trusted publishing** — releases authenticate to npm with short-lived OIDC tokens minted per run; there is no long-lived `NPM_TOKEN` to steal. diff --git a/VERSIONING.md b/VERSIONING.md index 833f80097..1bee0fa94 100644 --- a/VERSIONING.md +++ b/VERSIONING.md @@ -60,15 +60,40 @@ On merge to `main`, CI (`.github/workflows/ci.yml`) runs semantic-release in eac 2. If a release is due: version bump, `CHANGELOG.md` update, publish to npm (OIDC trusted publishing — no `NPM_TOKEN`), a signed `chore(release): X.Y.Z [skip ci]` commit, git tag, and GitHub release. - - Three packages publish via `@semantic-release/npm`, which shells out to `npm publish`. - **bestax-migrate publishes with `pnpm publish`** (`@semantic-release/exec`), because it - keeps a `workspace:` devDependency and `npm publish` ships that protocol verbatim — - which is how 1.0.0 went out uninstallable (#412, #436). + - **Every package publishes with `pnpm publish`** (`@semantic-release/exec`), not + `npm publish`. `@semantic-release/npm` stays in each chain with `npmPublish: false` + purely for its `prepare` step, which writes the version the release commit carries. + bestax-migrate moved first, because it keeps a `workspace:` devDependency and + `npm publish` ships that protocol verbatim — which is how its 1.0.0 went out + uninstallable (#412, #436); the other three followed once one real release had proved + the OIDC handshake (#532). The publish command and the reasons behind each of its flags + live in `scripts/lib/pnpm-publish.mjs`. - Note the ordering, because it decides what a failed publish costs: semantic-release runs **every** `prepare` step — including the release commit and tag — before **any** `publish` step. A publish that fails leaves the commit and tag behind, and that version is spent. 3. A push may release any subset of the packages — they never bump each other. +Three things about that publish step are load-bearing, and none of them fails loudly: + +- **`--provenance` is required.** pnpm reads `publishConfig.registry` and `.access` but takes + `provenance` from options only. `publishConfig.provenance` is deliberately absent from every + manifest rather than left in place: it does nothing under pnpm, and the most likely reason + anyone would delete the flag is reading `"provenance": true` in a package.json and concluding + it is redundant. Drop the flag and #411's provenance quietly stops being produced. +- **`--embed-readme` is required.** pnpm defaults it to false where npm defaults it to true; + without it the npmjs.com page loses its README. +- **The auth pre-flight is weaker than it was.** `@semantic-release/npm` exchanged a real OIDC + token during `verifyConditions`. With `npmPublish: false` that is off, so + `scripts/verify-oidc-context.mjs` runs as the exec plugin's `verifyConditionsCmd` and checks + only that an OIDC context exists; it does not prove npm will accept the token. Combined with + the ordering above, a failed publish spends the version. + +Outside CI, each package's `prepack` and `prepublishOnly` hooks run +`scripts/require-pnpm-publish.mjs`, which refuses packers it recognises as not being pnpm — so +a stray `npm publish` or `npm pack` exits with an explanation instead of shipping a manifest +nobody can install. It is a guard against the likely mistake, not a proof: `--ignore-scripts` +skips it, and a tarball packed elsewhere carries no guard with it. + `main` is ruleset-protected, so the release commit and tag are pushed by a dedicated GitHub App that is the ruleset's only automation bypass — not by `github-actions[bot]`. The commit is still GPG-signed with the maintainer's key, so it shows as **Verified**. diff --git a/bestax-mcp/CLAUDE.md b/bestax-mcp/CLAUDE.md index 43ecfa33a..550d72a20 100644 --- a/bestax-mcp/CLAUDE.md +++ b/bestax-mcp/CLAUDE.md @@ -78,3 +78,33 @@ test would pass while the server advertised nothing. They run against the committed index, not fixtures. The index is the product; a test that stubs it proves nothing about what ships. + +## Releases + +Independent semantic-release keyed off the `bestax-mcp` commit scope +(`release.config.js`, tag `bestax-mcp@x.y.z`). It publishes with +`pnpm publish`, like every package here (#532) — the command, its flags, and the +three ways that publish fails quietly are documented in `VERSIONING.md` and +`scripts/lib/pnpm-publish.mjs`. + +Its `prepack` runs the guard and then `scripts/sync-skills.mjs`, which fills +`data/skills/` at pack time. That directory is gitignored while the manifest +`data/skills.json` is committed, so packing locally does not dirty the tree — +which also keeps it clear of the post-release restamp step in `ci.yml`, whose +diff check is scoped to `bestax-mcp/data`. + +`files` carries a `"!data/.sync-skills"` negation, and it is not cosmetic. +`files` ships all of `data/`, and `sync-skills.mjs` keeps its fingerprint and +lock state in `data/.sync-skills/`, so without the negation that build state +ships as product. + +**This is not a `pnpm publish` behaviour, and an earlier version of this note +said it was.** npm and pnpm both include dot-directories under a `files` +entry; a synthetic package with `files: ["data"]` and `data/.state/x` packs +identically under either. The published 1.0.0 tarball has no `.sync-skills` +only because the state directory did not exist yet — it arrived with #520 on +2026-08-14, two days after 1.0.0 shipped. So this was a latent bug that the +next release would have hit whoever packed it, surfaced by diffing a +`pnpm -C bestax-mcp pack` against the published tarball while moving +publishers (#532). That diff is the check worth repeating whenever `files` or +the sync script changes, because nothing asserts tarball contents. diff --git a/bestax-mcp/package.json b/bestax-mcp/package.json index dde3bf02b..a96e7d185 100644 --- a/bestax-mcp/package.json +++ b/bestax-mcp/package.json @@ -8,7 +8,8 @@ }, "files": [ "dist", - "data" + "data", + "!data/.sync-skills" ], "scripts": { "build": "node scripts/sync-skills.mjs && tsc", @@ -22,7 +23,8 @@ "format:check": "prettier --check \"src/**/*.ts\"", "clean": "rimraf dist", "release": "npx semantic-release", - "prepack": "node scripts/sync-skills.mjs" + "prepack": "node ../scripts/require-pnpm-publish.mjs && node scripts/sync-skills.mjs", + "prepublishOnly": "node ../scripts/require-pnpm-publish.mjs" }, "keywords": [ "bestax", @@ -64,7 +66,6 @@ "typescript": "^6.0.3" }, "publishConfig": { - "access": "public", - "provenance": true + "access": "public" } } diff --git a/bestax-mcp/release.config.js b/bestax-mcp/release.config.js index 3a9a253a3..8ca9dbe16 100644 --- a/bestax-mcp/release.config.js +++ b/bestax-mcp/release.config.js @@ -1,3 +1,5 @@ +import { pnpmPublishPlugins } from '../scripts/lib/pnpm-publish.mjs'; + export default { branches: ['main'], tagFormat: 'bestax-mcp@${version}', @@ -44,12 +46,10 @@ export default { changelogFile: 'CHANGELOG.md', }, ], - [ - '@semantic-release/npm', - { - pkgRoot: '.', - }, - ], + // Publishing goes to `pnpm publish` rather than `npm publish` (#436, #532). + // The two plugins are one decision and every flag they pass is load-bearing + // in a way that fails quietly, so both live in the helper with the reasons. + ...pnpmPublishPlugins(import.meta.dirname), [ '@semantic-release/git', { diff --git a/bestax-migrate/CLAUDE.md b/bestax-migrate/CLAUDE.md index 5138b4b43..85fe1342c 100644 --- a/bestax-migrate/CLAUDE.md +++ b/bestax-migrate/CLAUDE.md @@ -64,79 +64,27 @@ each source library registers in `src/sources/registry.ts`; the first is Independent semantic-release, keyed off the `bestax-migrate` commit scope (`release.config.js`, tag `bestax-migrate@x.y.z`). -**This is the one package that publishes with `pnpm publish`, not `npm publish` -(#436).** `npm publish` does not resolve pnpm's `workspace:` protocol, so -`workspace:^` shipped verbatim in 1.0.0 and made the package uninstallable -(#412). That was patched by a `prepack` hook reimplementing pnpm's rewrite, and -the reimplementation was wrong twice in one review — so the publish step now -goes to `@semantic-release/exec` running `pnpm publish`, which resolves every -pnpm specifier shape by construction. `@semantic-release/npm` stays in the chain -with `npmPublish: false` purely for its `prepare` step, which writes the version -`@semantic-release/git` then commits. - -Three things about that split are load-bearing, and none of them fails loudly: - -- **`--provenance` is required.** pnpm reads `publishConfig.registry` and - `.access` but takes `provenance` from options only. `publishConfig.provenance` - was deliberately REMOVED from this package's manifest rather than left in - place: it does nothing under pnpm, and the most likely reason anyone would - delete the flag is reading `"provenance": true` in package.json and concluding - it is redundant. Drop the flag and #411's provenance quietly stops being - produced. -- **`--embed-readme` is required.** pnpm defaults it to false where npm defaults - it to true; without it the npmjs.com page loses its README. -- **The auth pre-flight is weaker than it was.** `@semantic-release/npm` - exchanged a real OIDC token during `verifyConditions`. With `npmPublish: false` - that is off, and semantic-release finishes every `prepare` step — the release - commit and the tag — before the first `publish` step. So a failed publish - leaves both behind and spends the version. - `scripts/verify-oidc-context.mjs` runs as the exec plugin's - `verifyConditionsCmd` and checks only that an OIDC context exists; it does not - prove npm will accept the token. - -**This package must be published with `pnpm publish`, and the likely mistakes -are refused.** The -old `prepack` hook rewrote `workspace:^` for whatever was packing, so it covered a -manual publish as well as the release pipeline. Deleting it left the guarantee -living only in `release.config.js`, which the conformance rule then exempts -precisely because pnpm handles it, so the specifier had no mechanical guard at all -outside CI. The hooks now run repo-root `scripts/require-pnpm-publish.mjs` (not -`bestax-migrate/scripts/`, which still exists for `validate-corpus.mjs`), which -refuses packers it recognises as not being pnpm. Both `npm publish` and `pnpm -publish` run those hooks. - -**It keys on `npm_execpath`, not `npm_config_user_agent`, and that is not a -detail to tidy up.** The user agent is inherited: npm relays whatever it finds, -so `pnpm exec npm publish` runs the hook reporting `pnpm/…` while npm assembles -the tarball, and an agent check waves it through. `npm_execpath` is rewritten by -whichever process actually runs the script, so it names the real packer. - -**It refuses named packers, and deliberately allows unrecognised ones.** npm, -yarn, bun and friends are refused by name; anything the guard cannot place is -let through. That asymmetry is not laziness, and reversing it would be worse -than the hole it closes: pnpm's own lifecycle runner sets -`npm_execpath = process.argv[1] || process.cwd()`, so a pnpm build where -`argv[1]` is falsy reports the package **directory**. Refusing what we cannot -recognise would kill a genuine release from inside a pack hook, after -semantic-release has pushed the commit and the tag, which is the one direction -this guard must never fail in. So it is a guard against the publisher someone -actually reaches for, not a proof that only pnpm can ever pack this package. - -`pnpm check:conformance` reports a violation if either hook is missing, so the -exemption and its compensating guard cannot drift apart. (It reports the missing -hook; it does not retract the exemption, so a manifest with both problems shows -one violation for each.) - -The hook is wired to **both `prepack` and `prepublishOnly`**, and both are -required by `check:conformance`. `prepack` is the load-bearing one for `npm -pack`: `npm publish ` runs no scripts at all, so a tarball packed by -npm would otherwise be publishable with nothing left to refuse it. - -It is still a guard against the likely mistake rather than a proof. -`--ignore-scripts` skips both hooks outright (npm and pnpm each gate lifecycle -scripts on it), and a tarball packed before this existed, or packed elsewhere, -carries no guard with it. Check what a manifest will actually ship with -`pnpm -C bestax-migrate pack`. +**Publishing is shared, and this package is why it looks the way it does.** +Every package here publishes with `pnpm publish` rather than `npm publish` +(#436, then #532), through the plugin pair in `scripts/lib/pnpm-publish.mjs`. +The mechanism and its three quiet failure modes are documented once in +`VERSIONING.md` and in that helper. The `prepack` / `prepublishOnly` guard is +documented in `scripts/require-pnpm-publish.mjs`, and that header is worth +reading before touching it: why it keys on `npm_execpath` rather than the +inherited `npm_config_user_agent`, and why it refuses only packers it can +name instead of allow-listing pnpm. `pnpm check:conformance` reports a +violation if either hook is missing, so the exemption and the guard that +compensates for it cannot drift apart. + +What is specific to bestax-migrate is the specifier that forced it. +`npm publish` does not resolve pnpm's `workspace:` protocol, so the +`workspace:^` below shipped verbatim in 1.0.0 and made the package +uninstallable (#412). That was first patched by a `prepack` hook +reimplementing pnpm's rewrite, and the reimplementation was wrong twice in one +review — which is the whole argument for handing the job to pnpm instead of +owning a subset of its logic. This is still the only package carrying such a +specifier, so it is the one whose tarball is worth inspecting after any change +to how packing works: `pnpm -C bestax-migrate pack`. `@allxsmith/bestax-bulma` still stays a **devDependency** — it is only the typecheck target for the e2e, never imported at runtime, and consumers of a diff --git a/bestax-migrate/release.config.js b/bestax-migrate/release.config.js index 92b399b77..d3f2f9567 100644 --- a/bestax-migrate/release.config.js +++ b/bestax-migrate/release.config.js @@ -1,16 +1,4 @@ -import path from 'node:path'; -// `sh` quotes the paths below for /bin/sh: a checkout under a directory with a -// space would otherwise split into two arguments and fail with a confusing -// "Cannot find module". It is colocated with its inverse, which the conformance -// check uses to read these commands back. -import { quote as sh } from '../scripts/lib/shell-words.mjs'; - -// Absolute, so neither exec command depends on the cwd semantic-release was -// invoked from. It works today only because ci.yml sets `working-directory`, -// and a relative `../scripts/...` would break the release the moment anything -// ran it from the repo root. -const PKG_DIR = import.meta.dirname; -const SCRIPTS = path.join(PKG_DIR, '..', 'scripts'); +import { pnpmPublishPlugins } from '../scripts/lib/pnpm-publish.mjs'; export default { branches: ['main'], @@ -58,113 +46,10 @@ export default { changelogFile: 'CHANGELOG.md', }, ], - // Publishing is split deliberately (#436). This plugin keeps its `prepare` - // step — that is what writes nextRelease.version into package.json for - // @semantic-release/git to commit — but its `publish` step shells out to - // `npm publish`, which does not resolve pnpm's `workspace:` protocol and - // shipped an uninstallable 1.0.0 because of it (#412). So the publish half - // goes to `pnpm publish` below, which resolves every pnpm specifier shape - // by construction rather than by a script of ours reimplementing a subset. - // - // `npmPublish: false` also switches off this plugin's npm auth check in - // verifyConditions — see verifyConditionsCmd on the exec plugin for what - // partially replaces it, and why only partially. - [ - '@semantic-release/npm', - { - pkgRoot: '.', - npmPublish: false, - }, - ], - [ - '@semantic-release/exec', - { - // Every command below runs here, not in whatever directory - // semantic-release was started from. `pnpm publish` resolves its target - // package from the cwd, so without this a run from the repo root would - // reach the publish step (after the release commit and tag are already - // pushed) and fail on the private root package. - execCwd: PKG_DIR, - - // Guards the one failure this swap newly introduces rather than - // inherits: with npmPublish false, nothing exchanges an OIDC token - // during verifyConditions any more, and semantic-release runs every - // `prepare` step (including the release commit and tag) before any - // `publish` step. The script says what it does and does not prove. - // `${options.dryRun}` is available because exec renders its commands - // as lodash templates over the semantic-release context. Needed - // because verifyConditions is marked `dryRun: true` upstream, so this - // runs during `semantic-release --dry-run` as well, and a dry run - // publishes nothing and needs no token. - verifyConditionsCmd: - `node ${sh(path.join(SCRIPTS, 'verify-oidc-context.mjs'))}` + - ' ${options.dryRun ? "--dry-run" : ""}', - - // Every flag here is load-bearing; none is decoration. - // - // --provenance The ONLY thing turning provenance on. pnpm reads - // publishConfig.registry and .access but takes - // `provenance` from options, and it is absent from - // the whitelist that hoists publishConfig keys. The - // package.json no longer carries a - // `publishConfig.provenance` at all, precisely so - // nobody reads one and concludes this flag is - // redundant. Passing it explicitly also survives an - // OIDC response that omits provenance, since pnpm - // assigns that with `??=`. - // --embed-readme pnpm defaults this to false, npm defaults it to - // true. Without it the npmjs.com page for this - // package loses its README on the next release. - // --no-git-checks pnpm otherwise refuses to publish from a branch it - // does not recognise as the publish branch; - // semantic-release is mid-release when this runs. - // --access belt and braces, not load-bearing: pnpm falls back - // to publishConfig.access, which this package sets. - // Stated explicitly because it is the value pnpm - // errors on when generating provenance for a package - // it believes is private, so it should be visible - // next to --provenance rather than a file away. - // - // No --tag on purpose. The dist-tag would have to be derived from - // nextRelease.channel, and @semantic-release/npm does not derive it - // naively: get-channel.js maps a channel that is a valid semver RANGE - // to `release-`, because the registry rejects a dist-tag that - // parses as a range. Reimplementing that in a lodash template needs - // semver and would be a copy of upstream logic drifting out of sight, - // which is the bug class #436 exists to stop repeating. `branches` is - // ['main'], so the channel is always null and pnpm's default of - // `latest` is already right. The test asserts that `branches` has not - // changed, so adding a maintenance or prerelease branch fails CI here - // rather than silently publishing it to the stable tag. - // pnpm's own output goes to stderr so stdout carries only the JSON - // release object exec parses. Nothing is hidden by that: exec pipes - // stdout and stderr separately to the job log, so the publish output - // still appears exactly where it does today, and a failed publish - // still throws. Without it, exec's parse fails, it returns undefined, - // and the "release is available on" comment on every linked issue and - // PR shows a bare `bestax-migrate@x.y.z` instead of the npm link the - // other three packages get. - // - // The `&&` tail cannot fail the release: npm-release-info.mjs always - // exits 0, degrading to `{}` if anything goes wrong. A non-zero exit - // there would throw out of the publish step with the tarball already - // on the registry, skipping @semantic-release/github and spending the - // version, for the sake of a link in a comment. - // `|| true` and not just the script's own error handling: if node - // cannot LOAD the script (moved, renamed, a syntax error), it exits - // non-zero before that handling is ever reached, and the `&&` chain - // would then fail the publish step with the tarball already on the - // registry. The guarantee has to live in the shell, where it holds - // whatever happens to the script. - publishCmd: - 'pnpm publish --no-git-checks --provenance --embed-readme ' + - '--access public 1>&2 && { node ' + - sh(path.join(SCRIPTS, 'npm-release-info.mjs')) + - ' --dir=' + - sh(PKG_DIR) + - ' ${nextRelease.version} || true; }', - }, - ], + // Publishing goes to `pnpm publish` rather than `npm publish` (#436, #532). + // The two plugins are one decision and every flag they pass is load-bearing + // in a way that fails quietly, so both live in the helper with the reasons. + ...pnpmPublishPlugins(import.meta.dirname), [ '@semantic-release/git', { diff --git a/bulma-ui/CLAUDE.md b/bulma-ui/CLAUDE.md index a9ec07a09..01fbb5cb8 100644 --- a/bulma-ui/CLAUDE.md +++ b/bulma-ui/CLAUDE.md @@ -73,3 +73,23 @@ improvising. - Must build and pass tests on **React 18 and 19** (CI matrix) — avoid single-major APIs. - Bundle size is marketing-visible (the READMEs link the live bundlephobia badge) — check `pnpm bundle:stats` (writes `dist/stats.html`) when adding anything with real runtime weight. + +## Releases + +Independent semantic-release keyed off the `bulma-ui` commit scope +(`release.config.js`, tag `@allxsmith/bestax-bulma@x.y.z`). It publishes with +`pnpm publish`, like every package here (#532) — the command, its flags, and the +three ways that publish fails quietly are documented in `VERSIONING.md` and +`scripts/lib/pnpm-publish.mjs`. + +Two things specific to this package. It is the only **scoped** one, and scoped +packages default to `restricted`, so `access` has to be set somewhere or the +release is private. It is set twice over — `publishConfig.access` here and +`--access public` on the shared publish command — and either alone is enough, +so neither is load-bearing on its own. Removing both is the mistake, and it +costs more here than anywhere else. And its `prepack`/`postpack` +pair (`scripts/pack-pointer-files.mjs`, #344) swaps the contributor +`CLAUDE.md` for the consumer copy of `AGENTS.md` inside the tarball and puts it +back — pnpm runs both hooks, so the round trip holds, but if a pack is +interrupted a `CLAUDE.md.bak` is left behind and the next `prepack` refuses +until you restore it. diff --git a/bulma-ui/package.json b/bulma-ui/package.json index 4de14c865..2dbe847ed 100644 --- a/bulma-ui/package.json +++ b/bulma-ui/package.json @@ -38,7 +38,8 @@ "test-storybook": "test-storybook", "test-storybook:dark": "STORYBOOK_THEME=dark test-storybook", "test-storybook:ci": "node scripts/test-storybook-ci.mjs", - "prepack": "node scripts/pack-pointer-files.mjs prepack", + "prepack": "node ../scripts/require-pnpm-publish.mjs && node scripts/pack-pointer-files.mjs prepack", + "prepublishOnly": "node ../scripts/require-pnpm-publish.mjs", "postpack": "node scripts/pack-pointer-files.mjs postpack" }, "devDependencies": { @@ -201,7 +202,6 @@ "url": "https://github.com/allxsmith/bestax/issues" }, "publishConfig": { - "access": "public", - "provenance": true + "access": "public" } } diff --git a/bulma-ui/release.config.js b/bulma-ui/release.config.js index 6d9f19c43..4fbe9babf 100644 --- a/bulma-ui/release.config.js +++ b/bulma-ui/release.config.js @@ -1,3 +1,5 @@ +import { pnpmPublishPlugins } from '../scripts/lib/pnpm-publish.mjs'; + export default { branches: ['main'], tagFormat: '@allxsmith/bestax-bulma@${version}', // Ensures correct tag format for this package @@ -42,12 +44,10 @@ export default { changelogFile: 'CHANGELOG.md', }, ], - [ - '@semantic-release/npm', - { - pkgRoot: '.', - }, - ], + // Publishing goes to `pnpm publish` rather than `npm publish` (#436, #532). + // The two plugins are one decision and every flag they pass is load-bearing + // in a way that fails quietly, so both live in the helper with the reasons. + ...pnpmPublishPlugins(import.meta.dirname), [ '@semantic-release/git', { diff --git a/create-bestax/CLAUDE.md b/create-bestax/CLAUDE.md index 4b5a5fcfb..4bcd9fabe 100644 --- a/create-bestax/CLAUDE.md +++ b/create-bestax/CLAUDE.md @@ -41,3 +41,17 @@ path (`-y` + flags, no TTY) must never hang or regress (#192). - Manual smoke: build, then scaffold **outside the repo** (`node create-bestax/dist/index.js /tmp/app -t vite-ts -y`) — inside the workspace you'd need `--ignore-workspace`. + +## Releases + +Independent semantic-release keyed off the `create-bestax` commit scope +(`release.config.js`, tag `create-bestax@x.y.z`). It publishes with +`pnpm publish`, like every package here (#532) — the command, its flags, and the +three ways that publish fails quietly are documented in `VERSIONING.md` and +`scripts/lib/pnpm-publish.mjs`. + +Its `prepack` runs the guard and then `scripts/sync-skills.mjs`, so the skills +bundled into `templates/` are refreshed as part of packing rather than +committed. There is no `postpack` because nothing is swapped out, only copied +in — a local `pnpm pack` therefore leaves `templates/skills/` populated, which +is gitignored and expected. diff --git a/create-bestax/package.json b/create-bestax/package.json index a870d43e1..e77496ff4 100644 --- a/create-bestax/package.json +++ b/create-bestax/package.json @@ -12,7 +12,8 @@ ], "scripts": { "build": "node scripts/sync-skills.mjs && tsc", - "prepack": "node scripts/sync-skills.mjs", + "prepack": "node ../scripts/require-pnpm-publish.mjs && node scripts/sync-skills.mjs", + "prepublishOnly": "node ../scripts/require-pnpm-publish.mjs", "dev": "tsc --watch", "test": "NODE_OPTIONS=\"--experimental-vm-modules\" jest", "test:watch": "NODE_OPTIONS=\"--experimental-vm-modules\" jest --watch", @@ -73,7 +74,6 @@ "wait-on": "^9.1.0" }, "publishConfig": { - "access": "public", - "provenance": true + "access": "public" } } diff --git a/create-bestax/release.config.js b/create-bestax/release.config.js index 1e216bf96..2d076146b 100644 --- a/create-bestax/release.config.js +++ b/create-bestax/release.config.js @@ -1,3 +1,5 @@ +import { pnpmPublishPlugins } from '../scripts/lib/pnpm-publish.mjs'; + export default { branches: ['main'], tagFormat: 'create-bestax@${version}', @@ -44,12 +46,10 @@ export default { changelogFile: 'CHANGELOG.md', }, ], - [ - '@semantic-release/npm', - { - pkgRoot: '.', - }, - ], + // Publishing goes to `pnpm publish` rather than `npm publish` (#436, #532). + // The two plugins are one decision and every flag they pass is load-bearing + // in a way that fails quietly, so both live in the helper with the reasons. + ...pnpmPublishPlugins(import.meta.dirname), [ '@semantic-release/git', { diff --git a/docs/docs/guides/getting-started/contributing.md b/docs/docs/guides/getting-started/contributing.md index 7db22d250..ab9a13c98 100644 --- a/docs/docs/guides/getting-started/contributing.md +++ b/docs/docs/guides/getting-started/contributing.md @@ -142,10 +142,13 @@ no `npm publish`, no tag, no GitHub release. :::tip Safe to run; never publishes Everything above is safe. The only things that actually publish are `pnpm exec semantic-release` -**without** `--dry-run` (CI-only, on merge to `main`) and a manual `npm publish` / `pnpm publish` — -none of which is in this list. `bestax-migrate` publishes with `pnpm publish`, and its +**without** `--dry-run` (CI-only, on merge to `main`) and a manual +`pnpm publish --provenance --embed-readme --access public` — neither of which is in this list. +Those flags are not optional: pnpm defaults `embed-readme` to false and ignores +`publishConfig.provenance`, which no package carries, so a bare `pnpm publish` ships unattested +and loses the npm page's README. Every package publishes with `pnpm publish`, and each one's `prepack` and `prepublishOnly` hooks refuse the publishers they recognise as not being pnpm, so a -stray `npm publish` or `npm pack` there exits with an explanation rather than producing an +stray `npm publish` or `npm pack` exits with an explanation rather than producing an unresolved `workspace:` specifier (#412). Both hooks are skipped by `--ignore-scripts`, and neither travels with a tarball that was packed elsewhere. ::: diff --git a/scripts/check-conformance.mjs b/scripts/check-conformance.mjs index d9e3a9620..0a52c9b5d 100644 --- a/scripts/check-conformance.mjs +++ b/scripts/check-conformance.mjs @@ -35,7 +35,8 @@ * and names only props that really exist * publishable-manifests no published package ships a specifier consumers * cannot resolve (#412). Which packages publish with - * `pnpm publish` is declared, not inferred (#436) + * `pnpm publish` is declared, not inferred (#436, + * #532) * bypass-expiry every supply-chain bypass in pnpm-workspace.yaml * carries a `# bestax:review ` or * `# bestax:permanent` marker, and no review date has @@ -53,6 +54,12 @@ import { registerVarsKeys } from './lib/scss-vars.mjs'; // The inverse of the quoting bestax-migrate/release.config.js uses to build its // exec commands. Shared so the two halves cannot drift (#436). import { tokenize } from './lib/shell-words.mjs'; +import { + PNPM_RESOLVES_TO_PLAIN_RANGE, + DEP_SECTIONS, + CONSUMER_SECTIONS, + packTimeProtocol, +} from './lib/pack-time-protocols.mjs'; import { readRegions, sectionSpans } from './lib/api-page.mjs'; import { renderPage } from './gen-api-docs.mjs'; import { @@ -1010,9 +1017,16 @@ export function parseWorkspacePackages(yaml) { * that shipped as bestax-migrate@1.0.0 (#412), invisibly, because nothing in * CI installs the published artifact. * - * bestax-migrate publishes with `pnpm publish` instead (#436), which resolves - * those protocols at pack time, so it is exempt from part of this rule. Which - * packages those are is DECLARED below, not inferred from their release config. + * Every package here publishes with `pnpm publish` instead — bestax-migrate + * first (#436), the other three once one real release had proved the OIDC + * handshake (#532) — which resolves those protocols at pack time, so each is + * exempt from part of this rule. Which packages those are is DECLARED below, + * not inferred from their release config. + * + * The npm branch below therefore has no package left to fire on today, and it + * stays anyway: it is what holds a NEW package to the strict rule until it is + * deliberately moved and declared. Deleting it would make "not yet declared" + * mean "exempt", which is the failure mode the rest of this comment is about. * * That is the whole design, and it is worth saying why, because the obvious * alternative was tried and failed four times. Reading `release.config.js` and @@ -1031,49 +1045,12 @@ export function parseWorkspacePackages(yaml) { * the release configs actually do. Getting THAT wrong fails a test, loudly, * instead of silently exempting a package. */ -const PNPM_PUBLISHED = new Set(['bestax-migrate']); - -/** - * Protocols that mean something inside this workspace and are not a plain - * installable specifier once published. - * - * `link:`, `portal:` and `file:` are here because NEITHER publisher rewrites - * them: pnpm's export converter chain is workspace/catalog/jsr (verified in the - * 11.9.0 bundle), and npm has no notion of the first two at all. A published - * manifest carrying one points at a path that does not exist on any consumer's - * machine. - */ -const PACK_TIME_PROTOCOLS = [ - 'workspace:', - 'catalog:', - 'jsr:', - 'link:', - 'portal:', - 'file:', -]; - -/** - * The subset `pnpm publish` turns into a plain, installable semver range. That - * is what makes a pnpm publisher safe to exempt, and it is not all of them: - * pnpm rewrites `jsr:@scope/pkg@^1` to `npm:@jsr/scope__pkg@^1`, which resolves - * only for a consumer who has configured the @jsr registry. - */ -const PNPM_RESOLVES_TO_PLAIN_RANGE = ['workspace:', 'catalog:']; - -const DEP_SECTIONS = [ - 'dependencies', - 'devDependencies', - 'peerDependencies', - 'optionalDependencies', -]; - -/** Sections a consumer of the published package resolves. */ -const CONSUMER_SECTIONS = DEP_SECTIONS.filter(s => s !== 'devDependencies'); - -const packTimeProtocol = spec => - typeof spec === 'string' - ? PACK_TIME_PROTOCOLS.find(p => spec.startsWith(p)) - : undefined; +const PNPM_PUBLISHED = new Set([ + 'bulma-ui', + 'create-bestax', + 'bestax-migrate', + 'bestax-mcp', +]); /** * The per-package rule, split out from the filesystem walk so it can be driven diff --git a/scripts/lib/pack-time-protocols.mjs b/scripts/lib/pack-time-protocols.mjs new file mode 100644 index 000000000..cd3c76f15 --- /dev/null +++ b/scripts/lib/pack-time-protocols.mjs @@ -0,0 +1,53 @@ +/** + * Specifier protocols that mean something inside this workspace and are not a + * plain installable specifier once published (#412, #436, #532). + * + * One copy, because there were two: check-conformance.mjs uses this to decide + * whether a manifest is a violation, and scripts/require-pnpm-publish.mjs uses + * it to name the offending specifier when it refuses a packer. The second copy + * carried a comment conceding that the two could drift and arguing the drift + * was cheap. It was, but only because the list happened to be right — add a + * seventh protocol to one and the other goes quiet about it. + */ + +/** + * `link:`, `portal:` and `file:` are here because NEITHER publisher rewrites + * them: pnpm's export converter chain is workspace/catalog/jsr (verified in the + * 11.9.0 bundle), and npm has no notion of the first two at all. A published + * manifest carrying one points at a path that does not exist on any consumer's + * machine. + */ +export const PACK_TIME_PROTOCOLS = [ + 'workspace:', + 'catalog:', + 'jsr:', + 'link:', + 'portal:', + 'file:', +]; + +/** + * The subset `pnpm publish` turns into a plain, installable semver range. That + * is what makes a pnpm publisher safe to exempt, and it is not all of them: + * pnpm rewrites `jsr:@scope/pkg@^1` to `npm:@jsr/scope__pkg@^1`, which resolves + * only for a consumer who has configured the @jsr registry. + */ +export const PNPM_RESOLVES_TO_PLAIN_RANGE = ['workspace:', 'catalog:']; + +export const DEP_SECTIONS = [ + 'dependencies', + 'devDependencies', + 'peerDependencies', + 'optionalDependencies', +]; + +/** Sections a consumer of the published package resolves. */ +export const CONSUMER_SECTIONS = DEP_SECTIONS.filter( + s => s !== 'devDependencies' +); + +/** The pack-time protocol `spec` uses, or undefined if it is a plain range. */ +export const packTimeProtocol = spec => + typeof spec === 'string' + ? PACK_TIME_PROTOCOLS.find(p => spec.startsWith(p)) + : undefined; diff --git a/scripts/lib/pnpm-publish.mjs b/scripts/lib/pnpm-publish.mjs new file mode 100644 index 000000000..49ba8d2d3 --- /dev/null +++ b/scripts/lib/pnpm-publish.mjs @@ -0,0 +1,153 @@ +/** + * The publish half of every package's semantic-release config (#436, #532). + * + * `@semantic-release/npm` shells out to `npm publish`, which resolves none of + * pnpm's pack-time protocols. A `workspace:^` left in a published manifest is + * uninstallable by every package manager, and that shipped as + * bestax-migrate@1.0.0 (#412), invisibly, because nothing in CI installs the + * published artifact. #436 moved that one package to `pnpm publish`, which + * resolves every pnpm specifier shape by construction rather than by a script + * of ours reimplementing a subset. #532 moved the other three, so this is now + * how the whole workspace publishes. + * + * It lives here rather than in each release.config.js because the four configs + * were identical and the reasons below are not obvious from the code. Four + * copies of an explanation is four chances for three of them to go stale, and + * every flag here is load-bearing in a way that fails quietly. + */ +import path from 'node:path'; +// `sh` quotes the paths below for /bin/sh: a checkout under a directory with a +// space would otherwise split into two arguments and fail with a confusing +// "Cannot find module". It is colocated with its inverse, which the conformance +// check uses to read these commands back. +import { quote as sh } from './shell-words.mjs'; + +const SCRIPTS = path.join(import.meta.dirname, '..'); + +/** + * The two plugins that publish a package, as a pair. + * + * They are returned together because they are one decision. `npmPublish: false` + * is what stops @semantic-release/npm publishing, and the exec plugin is what + * publishes instead; wiring one without the other either publishes twice or not + * at all. Keeping them separate in each config made that a thing a reviewer had + * to notice. + * + * @param pkgDir absolute path to the package, i.e. `import.meta.dirname` from + * the release config. Absolute, so neither exec command depends on the cwd + * semantic-release was invoked from. It works today only because ci.yml sets + * `working-directory`, and a relative `../scripts/...` would break the release + * the moment anything ran it from the repo root. + */ +export function pnpmPublishPlugins(pkgDir) { + return [ + // Publishing is split deliberately. This plugin keeps its `prepare` step — + // that is what writes nextRelease.version into package.json for + // @semantic-release/git to commit — but its `publish` step shells out to + // `npm publish`, which is the thing being replaced. + // + // `npmPublish: false` also switches off this plugin's npm auth check in + // verifyConditions — see verifyConditionsCmd below for what partially + // replaces it, and why only partially. + [ + '@semantic-release/npm', + { + pkgRoot: '.', + npmPublish: false, + }, + ], + [ + '@semantic-release/exec', + { + // Every command below runs in the package, not in whatever directory + // semantic-release was started from. `pnpm publish` resolves its target + // package from the cwd, so without this a run from the repo root would + // reach the publish step (after the release commit and tag are already + // pushed) and fail on the private root package. + execCwd: pkgDir, + + // Guards the one failure this swap newly introduces rather than + // inherits: with npmPublish false, nothing exchanges an OIDC token + // during verifyConditions any more, and semantic-release runs every + // `prepare` step (including the release commit and tag) before any + // `publish` step. The script says what it does and does not prove. + // `${options.dryRun}` is available because exec renders its commands + // as lodash templates over the semantic-release context. Needed + // because verifyConditions is marked `dryRun: true` upstream, so this + // runs during `semantic-release --dry-run` as well, and a dry run + // publishes nothing and needs no token. + verifyConditionsCmd: + `node ${sh(path.join(SCRIPTS, 'verify-oidc-context.mjs'))}` + + ' ${options.dryRun ? "--dry-run" : ""}', + + // Every flag here is load-bearing; none is decoration. + // + // --provenance The ONLY thing turning provenance on. pnpm reads + // publishConfig.registry and .access but takes + // `provenance` from options, and it is absent from + // the whitelist that hoists publishConfig keys. No + // package here carries a `publishConfig.provenance` + // any more, precisely so nobody reads one and + // concludes this flag is redundant. Passing it + // explicitly also survives an OIDC response that + // omits provenance, since pnpm assigns that with + // `??=`. + // --embed-readme pnpm defaults this to false, npm defaults it to + // true. Without it the npmjs.com page for the + // package loses its README on the next release. + // --no-git-checks pnpm otherwise refuses to publish from a branch it + // does not recognise as the publish branch; + // semantic-release is mid-release when this runs. + // --access load-bearing for the scoped package and belt and + // braces for the rest: @allxsmith/bestax-bulma is + // scoped, and scoped packages default to restricted. + // pnpm would otherwise fall back to + // publishConfig.access, which every package sets. It + // is also the value pnpm errors on when generating + // provenance for a package it believes is private, + // so it belongs next to --provenance rather than a + // file away. + // + // No --tag on purpose. The dist-tag would have to be derived from + // nextRelease.channel, and @semantic-release/npm does not derive it + // naively: get-channel.js maps a channel that is a valid semver RANGE + // to `release-`, because the registry rejects a dist-tag that + // parses as a range. Reimplementing that in a lodash template needs + // semver and would be a copy of upstream logic drifting out of sight, + // which is the bug class #436 exists to stop repeating. Every package's + // `branches` is ['main'], so the channel is always null and pnpm's + // default of `latest` is already right. The tests assert that + // `branches` has not changed, so adding a maintenance or prerelease + // branch fails CI here rather than silently publishing it to the + // stable tag. + // + // pnpm's own output goes to stderr so stdout carries only the JSON + // release object exec parses. Nothing is hidden by that: exec pipes + // stdout and stderr separately to the job log, so the publish output + // still appears exactly where it does today, and a failed publish + // still throws. Without it, exec's parse fails, it returns undefined, + // and the "release is available on" comment on every linked issue and + // PR shows a bare `@x.y.z` instead of an npm link. + // + // The `&&` tail cannot fail the release: npm-release-info.mjs always + // exits 0, degrading to `{}` if anything goes wrong. A non-zero exit + // there would throw out of the publish step with the tarball already + // on the registry, skipping @semantic-release/github and spending the + // version, for the sake of a link in a comment. + // `|| true` and not just the script's own error handling: if node + // cannot LOAD the script (moved, renamed, a syntax error), it exits + // non-zero before that handling is ever reached, and the `&&` chain + // would then fail the publish step with the tarball already on the + // registry. The guarantee has to live in the shell, where it holds + // whatever happens to the script. + publishCmd: + 'pnpm publish --no-git-checks --provenance --embed-readme ' + + '--access public 1>&2 && { node ' + + sh(path.join(SCRIPTS, 'npm-release-info.mjs')) + + ' --dir=' + + sh(pkgDir) + + ' ${nextRelease.version} || true; }', + }, + ], + ]; +} diff --git a/scripts/lib/release-config.mjs b/scripts/lib/release-config.mjs index f9c677b3c..eea0ecb10 100644 --- a/scripts/lib/release-config.mjs +++ b/scripts/lib/release-config.mjs @@ -12,6 +12,7 @@ * each dereferencing `[1]` without checking it found anything. */ const EXEC = '@semantic-release/exec'; +const NPM = '@semantic-release/npm'; /** * Understands the three plugin shapes semantic-release accepts, because two of @@ -20,22 +21,22 @@ const EXEC = '@semantic-release/exec'; * a form this cannot read — the caller is asserting things about that config, * so degrading to `{}` would turn a real assertion into a vacuous pass. */ -export async function execOptions(dir) { +export async function pluginOptions(dir, plugin) { const { default: config } = await import(`../../${dir}/release.config.js`); const plugins = config.plugins ?? []; for (const p of plugins) { if (typeof p === 'string') { - if (p === EXEC) { + if (p === plugin) { throw new Error( - `${dir}/release.config.js declares ${EXEC} with no options; there is ` + + `${dir}/release.config.js declares ${plugin} with no options; there is ` + 'nothing to read.' ); } continue; } - if (Array.isArray(p) && p[0] === EXEC) return p[1] ?? {}; - if (p && typeof p === 'object' && p.path === EXEC) { + if (Array.isArray(p) && p[0] === plugin) return p[1] ?? {}; + if (p && typeof p === 'object' && p.path === plugin) { // eslint-disable-next-line no-unused-vars const { path: _path, ...options } = p; return options; @@ -44,6 +45,17 @@ export async function execOptions(dir) { return null; } +/** Options on the exec plugin, which is what actually publishes. */ +export const execOptions = dir => pluginOptions(dir, EXEC); + +/** + * Options on the npm plugin, which is kept only for its `prepare` step. Its + * `npmPublish: false` is half of the publish decision: without it + * semantic-release publishes with npm AND then runs `pnpm publish`, which is a + * duplicate publish rather than a failure. + */ +export const npmOptions = dir => pluginOptions(dir, NPM); + /** The `branches` a package releases from. */ export async function releaseBranches(dir) { const { default: config } = await import(`../../${dir}/release.config.js`); diff --git a/scripts/npm-release-info.mjs b/scripts/npm-release-info.mjs index 84f96ecaa..06d0b6e49 100644 --- a/scripts/npm-release-info.mjs +++ b/scripts/npm-release-info.mjs @@ -24,8 +24,10 @@ * throws. * * Name and URL deliberately match `@semantic-release/npm/lib/get-release-info.js` - * exactly, so bestax-migrate's comments are indistinguishable from the three - * packages still publishing through that plugin. + * exactly, so the comment a pnpm-published release posts is indistinguishable + * from the one that plugin used to post. Every package publishes this way now + * (#532), so there is no longer a package to compare against — which is the + * reason to keep matching the upstream shape rather than drift from it. */ import fs from 'node:fs'; import path from 'node:path'; diff --git a/scripts/npm-release-info.test.mjs b/scripts/npm-release-info.test.mjs index 52f077ff8..824961c1e 100644 --- a/scripts/npm-release-info.test.mjs +++ b/scripts/npm-release-info.test.mjs @@ -1,11 +1,12 @@ /** - * Covers scripts/npm-release-info.mjs (#436). + * Covers scripts/npm-release-info.mjs (#436, #532). * * The output shape is the contract: @semantic-release/github renders it into * the comment posted on every linked issue and PR, and it has to be - * indistinguishable from what @semantic-release/npm produces for the three - * packages still publishing that way. So the field names and the URL format - * are pinned against that plugin's get-release-info.js. + * indistinguishable from what @semantic-release/npm used to produce. Since + * #532 no package publishes through that plugin, so there is nothing left to + * compare a release comment against by eye — which is why the field names and + * the URL format stay pinned against that plugin's get-release-info.js here. */ import { test } from 'node:test'; import assert from 'node:assert/strict'; diff --git a/scripts/publishable-manifests.test.mjs b/scripts/publishable-manifests.test.mjs index 4be03c901..cd3ad11e1 100644 --- a/scripts/publishable-manifests.test.mjs +++ b/scripts/publishable-manifests.test.mjs @@ -24,7 +24,12 @@ import { manifestViolations, parseWorkspacePackages, } from './check-conformance.mjs'; -import { execOptions, releaseBranches } from './lib/release-config.mjs'; +import { + execOptions, + npmOptions, + releaseBranches, +} from './lib/release-config.mjs'; +import { basename } from 'node:path'; import { tokenize } from './lib/shell-words.mjs'; const repoFile = rel => @@ -44,14 +49,26 @@ const DECLARED = new Set( .filter(Boolean) ); +// parseWorkspacePackages returns each `packages:` entry as literal text, so a +// glob like `packages/*` comes back unexpanded. Swallowing that would quietly +// shrink PUBLISHABLE, and PUBLISHABLE is what the declaration is compared +// against — the "a new package must be declared" guarantee would stop holding +// for everything under the glob while this file stayed green. So an entry whose +// manifest cannot be read is a failure, not a filtered-out row. const PUBLISHABLE = parseWorkspacePackages( repoFile('pnpm-workspace.yaml') ).filter(dir => { + let manifest; try { - return !JSON.parse(repoFile(`${dir}/package.json`)).private; + manifest = repoFile(`${dir}/package.json`); } catch { - return false; + throw new Error( + `pnpm-workspace.yaml lists "${dir}", which has no readable package.json. ` + + 'If that is a glob, parseWorkspacePackages does not expand it and every ' + + 'package under it is silently exempt from the checks in this file.' + ); } + return !JSON.parse(manifest).private; }); test('the declaration was actually parsed out of the check', () => { @@ -108,23 +125,44 @@ for (const dir of PUBLISHABLE) { }); } -test('bestax-migrate is the one declared package, and it wires the guard', () => { - // Stated rather than derived, so adding a package to the declaration is a - // deliberate act that fails this test first. - assert.deepEqual([...DECLARED], ['bestax-migrate']); - const pkg = JSON.parse(repoFile('bestax-migrate/package.json')); - assert.match(pkg.scripts.prepublishOnly, /require-pnpm-publish\.mjs/); +test('every publishable package is declared, and each wires the guard', () => { + // Derived from the workspace rather than restated, so a NEW publishable + // package fails here until someone decides how it publishes — which is the + // one moment that decision is cheap. Adding a name to PNPM_PUBLISHED without + // moving its release config fails the per-package test above instead. + assert.deepEqual([...DECLARED].sort(), [...PUBLISHABLE].sort()); + + // The exemption assumes pnpm packs these. In CI that is the release config's + // job; everywhere else it is the hooks'. `npm pack` runs only prepack, and + // `npm publish ` runs neither, so both are required. + for (const dir of DECLARED) { + const pkg = JSON.parse(repoFile(`${dir}/package.json`)); + for (const hook of ['prepack', 'prepublishOnly']) { + assert.match( + pkg.scripts?.[hook] ?? '', + /require-pnpm-publish\.mjs/, + `${dir} must run the guard on ${hook}` + ); + } + } }); -test('the declared package passes --provenance and --embed-readme', async () => { +test('every declared package passes --provenance and --embed-readme', async () => { // Neither is optional and neither fails loudly if dropped: pnpm ignores - // publishConfig.provenance (and this package no longer carries one), and + // publishConfig.provenance (and no package carries one any more), and // defaults embed-readme to false where npm defaults it true. Asserted - // against the command, not the file: both flags appear in the config's - // comments, so a source grep passed with them deleted from publishCmd. - const cmd = await publishCommand('bestax-migrate'); - assert.match(cmd, /(^|\s)--provenance(\s|$)/); - assert.match(cmd, /(^|\s)--embed-readme(\s|$)/); + // against the command, not the file: both flags appear in the shared + // helper's comments, so a source grep passed with them deleted from + // publishCmd. + // + // Looped over every package even though one helper builds all four commands, + // because that helper is an implementation detail. What must hold is that + // each package's config actually produces them. + for (const dir of DECLARED) { + const cmd = await publishCommand(dir); + assert.match(cmd, /(^|\s)--provenance(\s|$)/, `${dir} loses provenance`); + assert.match(cmd, /(^|\s)--embed-readme(\s|$)/, `${dir} loses its README`); + } }); test('the scripts the release config names all exist', async () => { @@ -133,8 +171,12 @@ test('the scripts the release config names all exist', async () => { // than by pattern-matching filenames out of the source — which broke on any // path shape other than `path.join(SCRIPTS, 'x.mjs')` and invented // requirements for filenames mentioned in comments. - const exec = await execOptions('bestax-migrate'); - assert.ok(exec, 'bestax-migrate must publish through @semantic-release/exec'); + for (const dir of DECLARED) await assertNamedScriptsExist(dir); +}); + +async function assertNamedScriptsExist(dir) { + const exec = await execOptions(dir); + assert.ok(exec, `${dir} must publish through @semantic-release/exec`); // Only the *Cmd options are shell commands. execCwd is a raw, unquoted path, // and feeding it to tokenize threw on a checkout containing an apostrophe — @@ -146,15 +188,15 @@ test('the scripts the release config names all exist', async () => { assert.ok( named.length >= 2, - `expected the config to name scripts, got ${named}` + `expected ${dir}'s config to name scripts, got ${named}` ); for (const abs of named) { assert.ok( existsSync(abs), - `release.config.js runs "${abs}", which does not exist` + `${dir}/release.config.js runs "${abs}", which does not exist` ); } -}); +} test('the release stays on a single branch, or the dist-tag needs revisiting', async () => { // publishCmd passes no --tag, which is only correct while every release goes @@ -168,7 +210,9 @@ test('the release stays on a single branch, or the dist-tag needs revisiting', a // `// branches: ['main']` would have satisfied it while the real value was // ['main', 'next'], which is the direction that publishes a prerelease to // the stable dist-tag. - assert.deepEqual(await releaseBranches('bestax-migrate'), ['main']); + for (const dir of DECLARED) { + assert.deepEqual(await releaseBranches(dir), ['main'], dir); + } }); // --- the rule ---------------------------------------------------------------- @@ -190,13 +234,24 @@ const WS = spec => ({ devDependencies: { '@allxsmith/bestax-bulma': spec }, }); -// Real directory names, because manifestViolations consults the declaration -// itself. `bulma-ui` is not declared, so it stands for an npm publisher; -// `bestax-migrate` is, so it stands for a pnpm one. A rule that ignored the -// declaration would pass fixtures that carried the verdict as an argument. -const NPM_PKG = 'bulma-ui'; +// Directory NAMES, because manifestViolations consults the declaration itself +// rather than taking a verdict as an argument — a rule that ignored the +// declaration would pass these fixtures. +// +// NPM_PKG used to be `bulma-ui`, a real undeclared package. #532 declared the +// last of those, so nothing real is left to play the part and the npm branch +// is exercised through a name that is deliberately not a workspace package: +// the case it now guards is a package that does not exist yet. The assertion +// below is what keeps that true, since declaring this name would silently turn +// every npm-branch test into a pnpm-branch one. +const NPM_PKG = 'a-package-that-has-not-moved-yet'; const PNPM_PKG = 'bestax-migrate'; +test('the npm-publisher fixture really is undeclared', () => { + assert.ok(!DECLARED.has(NPM_PKG), `${NPM_PKG} must not be declared`); + assert.ok(DECLARED.has(PNPM_PKG), `${PNPM_PKG} must be declared`); +}); + test('an npm publisher is held to every pack-time protocol', () => { for (const spec of [ 'workspace:^', @@ -351,11 +406,15 @@ test('an undeclared package gets no exemption, whatever the walk does', () => { // The mutation this exists for: a walk that hands every package the pnpm // verdict. Because manifestViolations consults the declaration itself, a // package that is not in it cannot be exempted from anywhere. - const v = manifestViolations('bulma-ui', WS('workspace:^')); - assert.equal(v.length, 1, 'bulma-ui is not declared and must not be exempt'); + const v = manifestViolations(NPM_PKG, WS('workspace:^')); + assert.equal( + v.length, + 1, + `${NPM_PKG} is not declared and must not be exempt` + ); assert.match(v[0], /npm publish/); - // …and the declared one still is. - assert.deepEqual(manifestViolations('bestax-migrate', WS('workspace:^')), []); + // …and a declared one still is. + assert.deepEqual(manifestViolations(PNPM_PKG, WS('workspace:^')), []); }); test('a declared package that drops the guard is flagged for it', () => { @@ -370,21 +429,70 @@ test('a declared package that drops the guard is flagged for it', () => { }); test('an undeclared package is not asked for the guard', () => { - // bulma-ui has no exemption, so it has nothing to compensate for. + // An undeclared package has no exemption, so it has nothing to compensate + // for. This said `bulma-ui` until #532 declared it, which made the comment + // describe the opposite of what the fixture does. const v = manifestViolations(NPM_PKG, { dependencies: { bulma: '^1.0.4' } }); assert.deepEqual(v, []); }); -test('the exec plugin pins its cwd', async () => { +test('every declared package disables the npm plugin publish step', async () => { + // The other half of the publish decision, and the half with no symptom. + // @semantic-release/npm is kept ONLY for its prepare step, which writes the + // version the release commit carries. Delete `npmPublish: false` and it + // publishes with `npm publish` first — shipping whatever pack-time protocol + // the manifest holds, which is #412 — and THEN the exec plugin runs + // `pnpm publish` against a version that is already on the registry. + // + // Every other assertion here pins the exec half. Nothing pinned this one, so + // deleting one line left the whole suite green. + for (const dir of DECLARED) { + const npm = await npmOptions(dir); + assert.ok( + npm, + `${dir} must keep @semantic-release/npm for its prepare step` + ); + assert.equal( + npm.npmPublish, + false, + `${dir} must set npmPublish: false, or it publishes twice` + ); + } +}); + +test('every declared package pins its exec cwd to itself', async () => { // `pnpm publish` resolves its target package from the cwd, which for exec is // wherever semantic-release was started. Without execCwd a run from the repo // root reaches the publish step — after the release commit and tag are - // pushed — and fails on the private root package. Every other exec option is - // pinned by a test; this one was not. - const exec = await execOptions('bestax-migrate'); - assert.ok(exec, 'bestax-migrate must publish through @semantic-release/exec'); - assert.ok(exec.execCwd, 'execCwd must be set'); - assert.match(exec.execCwd, /bestax-migrate$/); + // pushed — and fails on the private root package. + // + // Looped, and matched against the package it belongs to, because + // pnpmPublishPlugins now takes the directory as an ARGUMENT. A config that + // passed a copy-pasted path would publish the wrong package from a release + // whose commit and tag are already on main, and asserting only that execCwd + // is truthy would not notice. + for (const dir of DECLARED) { + const exec = await execOptions(dir); + assert.ok(exec, `${dir} must publish through @semantic-release/exec`); + assert.ok(exec.execCwd, `${dir}: execCwd must be set`); + assert.match(exec.execCwd, new RegExp(`(^|/)${dir}$`), `${dir}: execCwd`); + // The release-info tail is pointed by --dir= and has the same failure + // mode. Read through tokenize rather than a regex over the raw command: + // the path is shell-quoted, so a checkout under "~/My Projects" or a + // directory with an apostrophe puts characters inside the quotes that a + // naive pattern reads as the end of the argument. The first version of + // this assertion did exactly that and would have failed CI for those + // contributors — which is the case shell-words exists to survive. + const dirArg = tokenize(exec.publishCmd) + .filter(w => w.startsWith('--dir=')) + .map(w => w.slice('--dir='.length)); + assert.equal(dirArg.length, 1, `${dir}: expected exactly one --dir`); + assert.equal( + basename(dirArg[0]), + dir, + `${dir}: publishCmd --dir must name the same package` + ); + } }); test('a violation names a fix that fits the protocol', () => { diff --git a/scripts/require-pnpm-publish.mjs b/scripts/require-pnpm-publish.mjs index af9bf4abb..b9241edd2 100644 --- a/scripts/require-pnpm-publish.mjs +++ b/scripts/require-pnpm-publish.mjs @@ -1,30 +1,34 @@ #!/usr/bin/env node /** - * Refuses a publish driven by anything other than pnpm (#436). + * Refuses a publish driven by anything other than pnpm (#436, #532). * - * bestax-migrate keeps `"@allxsmith/bestax-bulma": "workspace:^"` in - * devDependencies. `pnpm publish` rewrites that to a real range at pack time; - * `npm publish` ships the protocol verbatim, which is what made 1.0.0 - * uninstallable (#412). + * Every package in this workspace publishes with `pnpm publish`. `npm publish` + * resolves none of pnpm's pack-time protocols, so a `workspace:^` reaches the + * registry verbatim and the published package is uninstallable for everyone — + * which is exactly what shipped as bestax-migrate@1.0.0 (#412). * - * That used to be covered twice over: a `prepack` hook rewrote the specifier - * for whatever was packing, and `check:conformance` refused to let the - * specifier exist without those hooks wired. #436 removed the hook, because - * hand-rolling pnpm's rewrite is the bug class it exists to stop owning, and - * the conformance rule now EXEMPTS this package precisely because pnpm handles - * it. Both of those are right, and together they left the specifier with no - * mechanical guard at all: correct in the release pipeline, and a prose rule in - * CLAUDE.md everywhere else. + * That used to be covered twice over for the one package that carried such a + * specifier: a `prepack` hook rewrote it for whatever was packing, and + * `check:conformance` refused to let the specifier exist without those hooks + * wired. #436 removed the hook, because hand-rolling pnpm's rewrite is the bug + * class it exists to stop owning, and the conformance rule now EXEMPTS a + * declared pnpm publisher precisely because pnpm handles it. Both of those are + * right, and together they left the exemption resting on an assumption — + * "pnpm packs this" — that nothing outside CI enforced. * * So guard the packer instead of the specifier. `pnpm publish` runs * `prepublishOnly` (verified against pnpm 11.9.0, which invokes it alongside * `prepublish` before packing), and so does `npm publish`, so this hook sees - * both and can tell them apart by the user agent each sets: + * both and can tell them apart. * - * pnpm/11.9.0 npm/? node/v25.2.1 darwin arm64 - * npm/11.6.2 node/v25.2.1 darwin arm64 workspaces/false + * It is wired into every publishable package, not only the one with a + * `workspace:` specifier. Two reasons, and the second is the load-bearing one: + * a package that gains such a dependency should not also have to remember to + * add a guard, and the flags the release passes (`--provenance`, + * `--embed-readme`) are not pnpm's defaults, so a hand `npm publish` degrades + * every package here, not just that one. * - * Scope, stated plainly rather than implied. bestax-migrate wires this to BOTH + * Scope, stated plainly rather than implied. Each package wires this to BOTH * `prepack` and `prepublishOnly`, which between them cover `npm publish` and * `npm pack` — the latter matters because `npm publish ` runs no * scripts at all, so a tarball packed by npm could otherwise be published with @@ -33,19 +37,30 @@ * - `--ignore-scripts`, which skips these hooks entirely. Both npm and pnpm * gate lifecycle scripts on it (pnpm 11.9.0 wraps the `prepublishOnly` / * `prepublish` call in `if (!opts.ignoreScripts)`), so `npm publish - * --ignore-scripts` ships the unresolved specifier with no signal at all. + * --ignore-scripts` ships whatever the manifest says with no signal at all. * Worth naming rather than leaving implied, because a repo whose * supply-chain policy is built on blocking install scripts is exactly the * kind of place that reaches for that flag out of habit. * - a tarball packed before this guard existed, or packed elsewhere. * * A hook cannot cover those, so this is a guard against the likely mistake, not - * a proof. Inspect the tarball with `pnpm -C bestax-migrate pack` if you are - * unsure what a manifest will ship. + * a proof. Inspect the tarball with `pnpm -C pack` if you are unsure + * what a manifest will ship. */ +import fs from 'node:fs'; +import path from 'node:path'; import process from 'node:process'; import { pathToFileURL } from 'node:url'; +// Shared with check-conformance.mjs, which uses the same list to decide +// whether a manifest is a violation. This file only uses it to name the +// offending specifier, but the two must agree or a protocol the check +// refuses is one this refusal cannot describe. +import { + PACK_TIME_PROTOCOLS, + DEP_SECTIONS, +} from './lib/pack-time-protocols.mjs'; + /** * Keyed on `npm_execpath`, NOT on `npm_config_user_agent`. * @@ -91,14 +106,58 @@ export function isPnpmPublish(execPath) { return !OTHER_PACKAGE_MANAGERS.test(name); } -export function main(env = process.env, log = console.error) { +/** Every specifier in `pkg` that only means something inside this workspace. */ +export function packTimeSpecifiers(pkg) { + const found = []; + for (const section of DEP_SECTIONS) { + for (const [name, spec] of Object.entries(pkg?.[section] ?? {})) { + if (typeof spec !== 'string') continue; + if (PACK_TIME_PROTOCOLS.some(p => spec.startsWith(p))) { + found.push({ section, name, spec }); + } + } + } + return found; +} + +/** + * What is being packed, read from the cwd — lifecycle hooks run in the package + * root. Never throws: this is called only to write a better error message, and + * a guard that crashed while explaining itself would report the wrong problem. + */ +function describePackage(cwd) { + const dir = path.basename(cwd); + try { + const pkg = JSON.parse(fs.readFileSync(path.join(cwd, 'package.json'))); + return { dir, name: pkg.name ?? dir, specifiers: packTimeSpecifiers(pkg) }; + } catch { + return { dir, name: dir, specifiers: [] }; + } +} + +export function main( + env = process.env, + log = console.error, + cwd = process.cwd() +) { if (isPnpmPublish(env.npm_execpath)) { // A hand-run `pnpm publish` is allowed, and silently produces neither - // provenance nor an embedded README: those flags live in the release - // config's publishCmd, and this package deliberately carries no - // publishConfig.provenance for pnpm to fall back on. CI passes them, so say - // nothing there; a human gets one line before the tarball goes out. - if (!env.CI && !env.GITHUB_ACTIONS) { + // provenance nor an embedded README: those flags live in the shared + // publishCmd, and no package here carries a publishConfig.provenance for + // pnpm to fall back on. CI passes them, so say nothing there; a human gets + // one line before the tarball goes out. + // + // Only on prepublishOnly, which is the hook that runs when something is + // actually being published. `pnpm pack` runs prepack alone, and warning + // there meant four packages printed "publishing by hand" during a + // read-only inspection — including on `pnpm -C pack`, the command + // this file's own header and three CLAUDE.md files recommend for checking + // what a manifest will ship. That is how a real warning gets tuned out. + if ( + env.npm_lifecycle_event === 'prepublishOnly' && + !env.CI && + !env.GITHUB_ACTIONS + ) { log( 'require-pnpm-publish: publishing by hand. `--provenance ' + '--embed-readme` are not defaults here and CI passes them for you; ' + @@ -108,23 +167,38 @@ export function main(env = process.env, log = console.error) { } return 0; } + + const { dir, name, specifiers } = describePackage(cwd); + // Named only when the manifest actually carries one. Inventing a specifier + // for a package that has none would send the reader looking for something + // that is not there, and the rule holds for that package either way. + const declared = specifiers.length + ? `${name} declares ` + + specifiers + .map(s => `"${s.name}": "${s.spec}" in ${s.section}`) + .join(', ') + + ', which npm would ship verbatim.\n\n' + : ''; + log( - 'This package must be published with `pnpm publish`, not ' + + `This package must be published with \`pnpm publish\`, not ` + `\`npm publish\` (packer: ${env.npm_execpath}).\n` + '\n' + - 'It declares "@allxsmith/bestax-bulma": "workspace:^" in ' + - 'devDependencies. pnpm resolves the workspace: protocol at pack time; ' + - 'npm ships it verbatim, and the published package is then uninstallable ' + - 'for everyone (EUNSUPPORTEDPROTOCOL, #412 shipped exactly this as ' + - '1.0.0).\n' + + declared + + 'Every package in this workspace publishes with pnpm, because ' + + "`npm publish` resolves none of pnpm's pack-time protocols — a " + + '`workspace:` specifier reaches the registry verbatim, and #412 shipped ' + + 'exactly that as bestax-migrate@1.0.0, uninstallable ' + + '(EUNSUPPORTEDPROTOCOL).\n' + '\n' + 'Releases are automated and run from CI. If you really are publishing ' + - 'by hand, the flags are not optional either, because this package no ' + - 'longer carries a publishConfig.provenance for pnpm to read:\n' + + 'by hand, the flags are not optional either, because no package here ' + + 'carries a publishConfig.provenance for pnpm to read:\n' + '\n' + ' pnpm publish --provenance --embed-readme --access public\n' + '\n' + - 'Check what it would ship first with `pnpm -C bestax-migrate pack`.' + 'Check what it would ship first with `pnpm pack`, from this ' + + `directory (${dir}).` ); return 1; } diff --git a/scripts/require-pnpm-publish.test.mjs b/scripts/require-pnpm-publish.test.mjs index ad9657767..c16e59a00 100644 --- a/scripts/require-pnpm-publish.test.mjs +++ b/scripts/require-pnpm-publish.test.mjs @@ -1,5 +1,5 @@ /** - * Covers scripts/require-pnpm-publish.mjs (#436). + * Covers scripts/require-pnpm-publish.mjs (#436, #532). * * Both directions are load-bearing and they fail differently. A guard that * misses `npm publish` lets #412 ship again with no signal. A guard that @@ -8,9 +8,14 @@ * commit and tag. */ import { test } from 'node:test'; +import { fileURLToPath } from 'node:url'; import assert from 'node:assert/strict'; -import { isPnpmPublish, main } from './require-pnpm-publish.mjs'; +import { + isPnpmPublish, + main, + packTimeSpecifiers, +} from './require-pnpm-publish.mjs'; const PNPM = '/Users/x/.cache/node/corepack/v1/pnpm/11.9.0/bin/pnpm.mjs'; const NPM = '/opt/homebrew/lib/node_modules/npm/bin/npm-cli.js'; @@ -100,6 +105,28 @@ test('a windows pnpm is allowed, in every form it ships as', () => { } }); +test('the hand-publish advisory fires on prepublishOnly and nowhere else', () => { + // prepack runs on `pnpm pack` too, which is a read-only inspection and the + // command this repo's docs recommend for checking what a manifest ships. + // Warning there had four packages printing a publish warning at people who + // were not publishing, which is how a real warning gets tuned out. + const hand = { npm_execpath: PNPM }; + let msg = ''; + main({ ...hand, npm_lifecycle_event: 'prepublishOnly' }, m => (msg = m)); + assert.match(msg, /publishing by hand/); + + msg = ''; + main({ ...hand, npm_lifecycle_event: 'prepack' }, m => (msg = m)); + assert.equal(msg, '', 'pnpm pack must not warn about publishing'); + + msg = ''; + main( + { ...hand, npm_lifecycle_event: 'prepublishOnly', CI: 'true' }, + m => (msg = m) + ); + assert.equal(msg, '', 'CI passes the flags, so it needs no advice'); +}); + test('the refusal spells out the flags a hand publish would otherwise lose', () => { // publishConfig.provenance was removed from the manifest, so a hand // `pnpm publish` — the one path this guard permits — produces no provenance @@ -128,14 +155,78 @@ test('the refusal explains the consequence, not just the rule', () => { assert.match(msg, /EUNSUPPORTEDPROTOCOL/); }); -test('bestax-migrate actually wires the hook up', async () => { - // The script existing is not the same as it running. - const pkg = await import('../bestax-migrate/package.json', { - with: { type: 'json' }, - }); - assert.match( - pkg.default.scripts.prepublishOnly, - /require-pnpm-publish\.mjs/, - 'bestax-migrate must run the guard on prepublishOnly' +// Real package directories, because the branch under test READS the manifest. +// The first version of these pointed at /tmp/nowhere/, so describePackage +// took its catch path every time and the "no specifier" assertion passed for +// the wrong reason — leaving the specifier-naming branch, which is the whole +// point of the change, with no coverage at all. +const repoDir = name => fileURLToPath(new URL(`../${name}`, import.meta.url)); + +test('the refusal names the package being packed, not a hardcoded one', () => { + // The message used to describe bestax-migrate's situation to whoever ran it. + // With four callers that was wrong for three of them, so it now reads the + // cwd — which is where a lifecycle hook runs. + let msg = ''; + main({ npm_execpath: NPM }, m => (msg = m), repoDir('bulma-ui')); + assert.match(msg, /from this directory \(bulma-ui\)/); +}); + +test('a readable manifest gets its offending specifier quoted', () => { + // bestax-migrate is the only package carrying one, and naming it is the + // reason this branch exists at all. + let msg = ''; + main({ npm_execpath: NPM }, m => (msg = m), repoDir('bestax-migrate')); + assert.match(msg, /bestax-migrate declares/); + assert.match(msg, /"@allxsmith\/bestax-bulma": "workspace:\^"/); + assert.match(msg, /in devDependencies/); +}); + +test('a package with no pack-time specifier is not told it has one', () => { + // Inventing a specifier sends the reader looking for something that is not + // there. The rule still holds for that package, so the message says why + // without naming a dependency. + let msg = ''; + main({ npm_execpath: NPM }, m => (msg = m), repoDir('bulma-ui')); + assert.doesNotMatch(msg, /bulma-ui declares/); + // …and still explains the rule and its consequence. + assert.match(msg, /#412/); + assert.match(msg, /--provenance/); +}); + +test('an unreadable manifest degrades to the directory name, never throws', () => { + // This runs inside a pack hook. A guard that crashed while explaining itself + // would report the wrong problem, and on the allow path would fail a real + // release after the commit and tag are pushed. + let msg = ''; + const code = main( + { npm_execpath: '/x/npm-cli.js' }, + m => (msg = m), + '/tmp/definitely-not-a-package/some-pkg' + ); + assert.equal(code, 1); + assert.match(msg, /from this directory \(some-pkg\)/); +}); + +test('packTimeSpecifiers finds every protocol, in every section', () => { + assert.deepEqual( + packTimeSpecifiers({ + dependencies: { a: 'workspace:^', ok: '^1.0.0' }, + devDependencies: { b: 'catalog:' }, + peerDependencies: { c: 'link:../y' }, + optionalDependencies: { d: 'file:../z' }, + }), + [ + { section: 'dependencies', name: 'a', spec: 'workspace:^' }, + { section: 'devDependencies', name: 'b', spec: 'catalog:' }, + { section: 'peerDependencies', name: 'c', spec: 'link:../y' }, + { section: 'optionalDependencies', name: 'd', spec: 'file:../z' }, + ] ); }); + +test('packTimeSpecifiers survives a manifest that is missing or odd', () => { + assert.deepEqual(packTimeSpecifiers(undefined), []); + assert.deepEqual(packTimeSpecifiers({}), []); + // A non-string specifier is malformed, not a protocol; it must not crash. + assert.deepEqual(packTimeSpecifiers({ dependencies: { a: { x: 1 } } }), []); +}); diff --git a/scripts/verify-oidc-context.mjs b/scripts/verify-oidc-context.mjs index cda8e4939..89db13b90 100644 --- a/scripts/verify-oidc-context.mjs +++ b/scripts/verify-oidc-context.mjs @@ -1,10 +1,11 @@ #!/usr/bin/env node /** - * Pre-flight for the packages that publish with `pnpm publish` (#436). + * Pre-flight for the packages that publish with `pnpm publish` (#436, #532), + * which is all four of them. * - * bestax-migrate hands its publish step to `@semantic-release/exec` running + * Each hands its publish step to `@semantic-release/exec` running * `pnpm publish`, because `npm publish` does not resolve pnpm's `workspace:` - * protocol and shipped an uninstallable 1.0.0 (#412). Doing that costs one + * protocol and shipped an uninstallable bestax-migrate 1.0.0 (#412). Doing that costs one * thing worth replacing: `@semantic-release/npm` performed a real OIDC token * exchange inside `verifyConditions`, so a job missing `id-token: write` failed * before semantic-release had written anything. With `npmPublish: false` that diff --git a/scripts/verify-oidc-context.test.mjs b/scripts/verify-oidc-context.test.mjs index 5f11c16de..c675a794f 100644 --- a/scripts/verify-oidc-context.test.mjs +++ b/scripts/verify-oidc-context.test.mjs @@ -1,7 +1,7 @@ /** - * Covers scripts/verify-oidc-context.mjs (#436). + * Covers scripts/verify-oidc-context.mjs (#436, #532). * - * This guard exists because moving bestax-migrate to `pnpm publish` switched + * This guard exists because moving to `pnpm publish` switched * off `@semantic-release/npm`'s OIDC check in verifyConditions, and * semantic-release pushes the release commit and tag before any publish step * runs. Both directions are worth pinning: a guard that never fires is