Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 12 additions & 11 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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:
Expand Down
5 changes: 3 additions & 2 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
9 changes: 8 additions & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Comment thread
coderabbitai[bot] marked this conversation as resolved.

---

Expand Down
19 changes: 14 additions & 5 deletions SECURITY.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
33 changes: 29 additions & 4 deletions VERSIONING.md
Original file line number Diff line number Diff line change
Expand Up @@ -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**.
Expand Down
30 changes: 30 additions & 0 deletions bestax-mcp/CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
9 changes: 5 additions & 4 deletions bestax-mcp/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@
},
"files": [
"dist",
"data"
"data",
"!data/.sync-skills"
],
"scripts": {
"build": "node scripts/sync-skills.mjs && tsc",
Expand All @@ -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",
Expand Down Expand Up @@ -64,7 +66,6 @@
"typescript": "^6.0.3"
},
"publishConfig": {
"access": "public",
"provenance": true
"access": "public"
}
}
12 changes: 6 additions & 6 deletions bestax-mcp/release.config.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { pnpmPublishPlugins } from '../scripts/lib/pnpm-publish.mjs';

export default {
branches: ['main'],
tagFormat: 'bestax-mcp@${version}',
Expand Down Expand Up @@ -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',
{
Expand Down
94 changes: 21 additions & 73 deletions bestax-migrate/CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 <tarball>` 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
Expand Down
Loading
Loading