From 2bdd8992dd4eb8f1606cdff80e4897874389f9c7 Mon Sep 17 00:00:00 2001 From: Paul Newling Date: Wed, 26 Aug 2026 16:05:40 +0200 Subject: [PATCH] fix(changeset): reject type-none release-plan entries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `changeset status` lists every dependent of a bumped package in `.releases`, at `"type": "none"` when no version change is due. Both changeset actions read that array by name only, so a `none` entry counted as a bump. In a block repo the private `model`/`ui`/`workflow` siblings are versioned (no repo sets changesets' `privatePackages`), so a sibling-only changeset releases the siblings and lists the block as their dependent at `none`. `require-package-bump` accepted that and let the PR merge without releasing the block — platforma-open/antibody-tcr-lead-selection#181 merged green and shipped nothing, then needed #183 to force the release. 18 block repos opt into the gate today. `check-coverage` had the same defect: a `none` entry satisfied the coverage requirement for a package whose own files the PR edited. Both now filter `.type != "none"`. The existing "fails when only a sibling changeset is present" test passed because the fixture pinned `"privatePackages": false`, which drops private packages from the release plan and suppresses the cascade entirely. Removing it makes the fixture match a real block repo and turns that test red without the fix. `check-coverage`'s fixture gains a publishable `pkg-lib`/`pkg-app` pair on a `workspace:*` devDependency edge for the same reason. Each suite also asserts the `none` entry directly, so a future fixture edit that re-suppresses it fails loudly instead of silently disarming the gate. Nothing ran the `require-package-bump` suite in CI — `0-test-changeset-coverage.yaml` was scoped to `check-coverage/**`. It is now `0-test-changeset-actions.yaml`, a two-leg matrix over `actions/changeset/**`. --- ...age.yaml => 0-test-changeset-actions.yaml} | 19 +++++++++----- .../check-coverage/check-coverage.sh | 10 ++++++- .../changeset/check-coverage/test/README.md | 11 +++++--- .../check-coverage/test/coverage.bats | 25 ++++++++++++++++++ .../packages/pkg-app/index.js | 1 + .../packages/pkg-app/package.json | 8 ++++++ .../packages/pkg-lib/index.js | 1 + .../packages/pkg-lib/package.json | 5 ++++ .../require-package-bump.sh | 15 ++++++++--- .../require-package-bump/test/README.md | 20 +++++++++++--- .../block-workspace/.changeset/config.json | 1 - .../test/require-package-bump.bats | 26 +++++++++++++++++++ 12 files changed, 124 insertions(+), 18 deletions(-) rename .github/workflows/{0-test-changeset-coverage.yaml => 0-test-changeset-actions.yaml} (60%) create mode 100644 actions/changeset/check-coverage/test/fixtures/minimal-workspace/packages/pkg-app/index.js create mode 100644 actions/changeset/check-coverage/test/fixtures/minimal-workspace/packages/pkg-app/package.json create mode 100644 actions/changeset/check-coverage/test/fixtures/minimal-workspace/packages/pkg-lib/index.js create mode 100644 actions/changeset/check-coverage/test/fixtures/minimal-workspace/packages/pkg-lib/package.json diff --git a/.github/workflows/0-test-changeset-coverage.yaml b/.github/workflows/0-test-changeset-actions.yaml similarity index 60% rename from .github/workflows/0-test-changeset-coverage.yaml rename to .github/workflows/0-test-changeset-actions.yaml index 6d8a2d23..e1cb1b80 100644 --- a/.github/workflows/0-test-changeset-coverage.yaml +++ b/.github/workflows/0-test-changeset-actions.yaml @@ -1,19 +1,26 @@ -name: 'Test: changeset/check-coverage' +name: 'Test: changeset actions' on: workflow_call: workflow_dispatch: # manual escape hatch — also lets us re-run after a workflow edit pull_request: paths: - - 'actions/changeset/check-coverage/**' + - 'actions/changeset/**' push: paths: - - 'actions/changeset/check-coverage/**' + - 'actions/changeset/**' branches: ['v4', 'v4-beta'] jobs: bats: + name: bats (${{ matrix.action.dir }}) runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + action: + - { dir: 'check-coverage', suite: 'test/coverage.bats' } + - { dir: 'require-package-bump', suite: 'test/require-package-bump.bats' } steps: - uses: actions/checkout@v4 @@ -36,6 +43,6 @@ jobs: pnpm --version bats --version - - name: Run changeset/check-coverage bats suite - working-directory: actions/changeset/check-coverage - run: bats test/coverage.bats + - name: Run changeset/${{ matrix.action.dir }} bats suite + working-directory: actions/changeset/${{ matrix.action.dir }} + run: bats ${{ matrix.action.suite }} diff --git a/actions/changeset/check-coverage/check-coverage.sh b/actions/changeset/check-coverage/check-coverage.sh index 25c04187..b4592166 100755 --- a/actions/changeset/check-coverage/check-coverage.sh +++ b/actions/changeset/check-coverage/check-coverage.sh @@ -27,6 +27,9 @@ # Skips private (unpublished) workspace packages — they never appear in # the changeset's release set. # +# A package listed in the release plan at `"type": "none"` is a dependent of a +# bumped package, not a release. It does not count as covered. +# # Runs from the repo root after `pnpm install`. set -o nounset @@ -96,10 +99,15 @@ if [ ! -s "${status_json}" ]; then fi fi +# `.releases` also lists every dependent of a bumped package, at +# `"type": "none"` when no version change is due. A `none` entry is not a bump +# and must not cover its package's own edit. declare -A bumped_set=() while IFS= read -r pkg; do [ -n "${pkg}" ] && bumped_set["${pkg}"]=1 -done < <(jq -r '.releases[]?.name // empty' "${status_json}") +done < <( + jq -r '.releases[]? | select(.type != "none") | .name // empty' "${status_json}" +) if [ "${#bumped_set[@]}" -eq 0 ]; then log 'Changeset bumps: ' diff --git a/actions/changeset/check-coverage/test/README.md b/actions/changeset/check-coverage/test/README.md index 39d6445c..f25aa463 100644 --- a/actions/changeset/check-coverage/test/README.md +++ b/actions/changeset/check-coverage/test/README.md @@ -50,13 +50,18 @@ and a couple of git ops. - `packages/pkg-a`, `packages/pkg-b` — both consume `is-number` via `catalog:` - `packages/pkg-c` — consumes `is-string` via `catalog:` +- `packages/pkg-dev` — consumes `is-string` via a `catalog:` devDependency - `packages/pkg-private` — `"private": true`, never requires a bump +- `packages/pkg-lib`, `packages/pkg-app` — both publishable; `pkg-app` + devDepends on `pkg-lib` via `workspace:*`. Bumping `pkg-lib` alone puts + `pkg-app` in the release plan at `"type": "none"`, the entry the script must + not read as a bump. ## CI -`.github/workflows/0-test-changeset-coverage.yaml` runs the suite on -`pull_request` and `push` (`v4`, `v4-beta`) whenever files under -`actions/changeset/check-coverage/**` or the workflow itself change. +`.github/workflows/0-test-changeset-actions.yaml` runs this suite and +`require-package-bump`'s as a two-leg matrix, on `pull_request` and `push` +(`v4`, `v4-beta`) whenever files under `actions/changeset/**` change. ## Adding a test diff --git a/actions/changeset/check-coverage/test/coverage.bats b/actions/changeset/check-coverage/test/coverage.bats index fc42fc11..d4dee632 100644 --- a/actions/changeset/check-coverage/test/coverage.bats +++ b/actions/changeset/check-coverage/test/coverage.bats @@ -46,6 +46,31 @@ setup() { [ "${status}" -eq 0 ] } +# `changeset status` lists a dependent of a bumped package at `"type": "none"` +# — in the plan, but not released. `pkg-app` devDepends on `pkg-lib` via +# `workspace:*`, so bumping `pkg-lib` alone puts `pkg-app` in the plan at +# `none` while leaving its own edit uncovered. +@test "a type-none cascade entry does not cover an edited package" { + touch_file 'packages/pkg-app/index.js' + add_changeset '"@check-coverage-test/pkg-lib": patch' 'bump lib only' + run_check + [ "${status}" -eq 1 ] + [[ "${output}" == *'@check-coverage-test/pkg-app'* ]] +} + +# Anti-drift guard for the fixture, not for the script: the case above is only +# meaningful while the plan really carries `pkg-app` at type `none`. +@test "fixture reproduces the cascade: pkg-lib bump leaves pkg-app at type none" { + add_changeset '"@check-coverage-test/pkg-lib": patch' 'bump lib only' + cd "${WORKSPACE}" + ./node_modules/.bin/changeset status \ + --since=origin/main --output=cascade.json >/dev/null 2>&1 || true + run jq -r '.releases[] | select(.name == "@check-coverage-test/pkg-app") | .type' \ + cascade.json + [ "${status}" -eq 0 ] + [ "${output}" = 'none' ] +} + @test "reports every missing package, not just the first" { touch_file 'packages/pkg-a/index.js' touch_file 'packages/pkg-b/index.js' diff --git a/actions/changeset/check-coverage/test/fixtures/minimal-workspace/packages/pkg-app/index.js b/actions/changeset/check-coverage/test/fixtures/minimal-workspace/packages/pkg-app/index.js new file mode 100644 index 00000000..d5b2e8bb --- /dev/null +++ b/actions/changeset/check-coverage/test/fixtures/minimal-workspace/packages/pkg-app/index.js @@ -0,0 +1 @@ +module.exports = 0; diff --git a/actions/changeset/check-coverage/test/fixtures/minimal-workspace/packages/pkg-app/package.json b/actions/changeset/check-coverage/test/fixtures/minimal-workspace/packages/pkg-app/package.json new file mode 100644 index 00000000..7ec67128 --- /dev/null +++ b/actions/changeset/check-coverage/test/fixtures/minimal-workspace/packages/pkg-app/package.json @@ -0,0 +1,8 @@ +{ + "name": "@check-coverage-test/pkg-app", + "version": "1.0.0", + "main": "index.js", + "devDependencies": { + "@check-coverage-test/pkg-lib": "workspace:*" + } +} diff --git a/actions/changeset/check-coverage/test/fixtures/minimal-workspace/packages/pkg-lib/index.js b/actions/changeset/check-coverage/test/fixtures/minimal-workspace/packages/pkg-lib/index.js new file mode 100644 index 00000000..d5b2e8bb --- /dev/null +++ b/actions/changeset/check-coverage/test/fixtures/minimal-workspace/packages/pkg-lib/index.js @@ -0,0 +1 @@ +module.exports = 0; diff --git a/actions/changeset/check-coverage/test/fixtures/minimal-workspace/packages/pkg-lib/package.json b/actions/changeset/check-coverage/test/fixtures/minimal-workspace/packages/pkg-lib/package.json new file mode 100644 index 00000000..e7a07db2 --- /dev/null +++ b/actions/changeset/check-coverage/test/fixtures/minimal-workspace/packages/pkg-lib/package.json @@ -0,0 +1,5 @@ +{ + "name": "@check-coverage-test/pkg-lib", + "version": "1.0.0", + "main": "index.js" +} diff --git a/actions/changeset/require-package-bump/require-package-bump.sh b/actions/changeset/require-package-bump/require-package-bump.sh index ea5f46d6..9c879f79 100755 --- a/actions/changeset/require-package-bump/require-package-bump.sh +++ b/actions/changeset/require-package-bump/require-package-bump.sh @@ -8,9 +8,10 @@ # for a block, the package has no `workspace:` *dependencies* (its private # siblings live in devDependencies, which `pnpm changeset` never cascades, and # its runtime deps are external `catalog:` pins), so a `changeset status` -# release for the package is equivalent to a changeset that names it directly. -# A sibling-only changeset therefore does NOT satisfy this check — matching the -# "did you bump ./block?" intent. +# release of a non-`none` type is equivalent to a changeset that names the +# package directly. A sibling-only changeset still lists the block, at type +# `none`; that does NOT satisfy this check — matching the "did you bump +# ./block?" intent. # # The empty-changeset opt-out is scoped to changesets ADDED in this branch # (`git diff --diff-filter=A origin/$BASE_BRANCH...HEAD`), so a stale empty @@ -88,7 +89,13 @@ if [ ! -s "${status_json}" ]; then fi fi -if jq -e --arg n "${pkg_name}" '.releases[]? | select(.name == $n)' \ +# `.releases` also lists every dependent of a bumped package, with +# `"type": "none"` when no version change is due. Only a non-`none` type is a +# bump: the private siblings are versioned (block repos leave changesets' +# `privatePackages` at its default), so a sibling-only changeset puts the block +# in the plan as a `none` dependent. +if jq -e --arg n "${pkg_name}" \ + '.releases[]? | select(.name == $n and .type != "none")' \ "${status_json}" >/dev/null; then log "✓ ${pkg_name} is bumped by a changeset." exit 0 diff --git a/actions/changeset/require-package-bump/test/README.md b/actions/changeset/require-package-bump/test/README.md index 9d2a6d9f..e11b08f5 100644 --- a/actions/changeset/require-package-bump/test/README.md +++ b/actions/changeset/require-package-bump/test/README.md @@ -33,9 +33,17 @@ assertions reuse it (each test only tars a copy and runs a couple of git ops). `fixtures/block-workspace` mirrors a block repo: a publishable `block` package (`@block-bump-test/block`) whose private `model`/`ui`/`workflow` siblings are -reached via `workspace:*` **devDependencies** — the topology where stock -`changeset version` never cascades a sibling bump into the block, so the gate -must require the block to be named directly. +reached via `workspace:*` **devDependencies**. `changeset version` never +cascades a version through a devDependency edge, so the gate must require the +block to be named directly. + +`.changeset/config.json` must leave `privatePackages` unset, as every block +repo does. The default versions private packages, so a sibling-only changeset +releases the sibling and lists the block as its dependent at `"type": "none"` — +the shape the gate has to reject. Setting `privatePackages` to `false` drops +the siblings from the release plan, removes that entry, and makes the +sibling-only tests pass without exercising the gate. One test asserts the +`none` entry directly to keep that from recurring. ## How it works @@ -46,3 +54,9 @@ must require the block to be named directly. a `feature` branch; `add_changeset` / `touch_file` / `add_changeset_on_base` stage the scenario; `run_require` runs the script with `PACKAGE_PATH=block` and `BASE_BRANCH=main`. + +## CI + +`.github/workflows/0-test-changeset-actions.yaml` runs this suite and +`check-coverage`'s as a two-leg matrix, on `pull_request` and `push` +(`v4`, `v4-beta`) whenever files under `actions/changeset/**` change. diff --git a/actions/changeset/require-package-bump/test/fixtures/block-workspace/.changeset/config.json b/actions/changeset/require-package-bump/test/fixtures/block-workspace/.changeset/config.json index d71982fe..edef2ebc 100644 --- a/actions/changeset/require-package-bump/test/fixtures/block-workspace/.changeset/config.json +++ b/actions/changeset/require-package-bump/test/fixtures/block-workspace/.changeset/config.json @@ -7,6 +7,5 @@ "access": "restricted", "baseBranch": "main", "updateInternalDependencies": "patch", - "privatePackages": false, "ignore": [] } diff --git a/actions/changeset/require-package-bump/test/require-package-bump.bats b/actions/changeset/require-package-bump/test/require-package-bump.bats index 036324a9..1cd33c0c 100644 --- a/actions/changeset/require-package-bump/test/require-package-bump.bats +++ b/actions/changeset/require-package-bump/test/require-package-bump.bats @@ -40,6 +40,32 @@ setup() { [[ "${output}" == *'@block-bump-test/block'* ]] } +@test "fails when every sibling is bumped but the block is not" { + add_changeset '"@block-bump-test/model": patch' 'bump model' + add_changeset '"@block-bump-test/ui": patch' 'bump ui' + add_changeset '"@block-bump-test/workflow": patch' 'bump workflow' + run_require + [ "${status}" -eq 1 ] + [[ "${output}" == *'@block-bump-test/block'* ]] +} + +# Anti-drift guard for the fixture, not for the script. A sibling-only +# changeset must leave the block in the release plan at type `none` — the +# shape the gate has to reject. `.changeset/config.json` must keep changesets' +# `privatePackages` default (versioning the private siblings), as every block +# repo does; setting it to `false` drops the siblings from the plan, removes +# this entry, and makes the sibling-only cases above pass for the wrong reason. +@test "fixture reproduces the cascade: sibling-only leaves block at type none" { + add_changeset '"@block-bump-test/model": patch' 'bump model only' + cd "${WORKSPACE}" + ./node_modules/.bin/changeset status \ + --since=origin/main --output=cascade.json >/dev/null 2>&1 || true + run jq -r '.releases[] | select(.name == "@block-bump-test/block") | .type' \ + cascade.json + [ "${status}" -eq 0 ] + [ "${output}" = 'none' ] +} + @test "fails when the block is edited without any changeset" { touch_file 'block/src/index.js' run_require