test(scripts): enforce that changeset's ignore covers every private package - #555
Merged
Conversation
…ackage The list fell behind twice in one week. #537 wrote a changeset naming @tapflowio/test-utils, which is private and was absent from ignore -- changeset version would have versioned an unpublished package and written it a CHANGELOG while no published package got a release note, with the CI gate green. The follow-up added test-utils and left @tapflowio/docs in the same state. Both were caught by a human reading the diff. Nothing objects on its own: privatePackages.version defaults to true, so an unignored private package appears in `pnpm changeset` like any other, and config validation only errors when a non-private unskipped package depends on a skipped one. Three assertions, found by inspection rather than from a list -- a list is satisfied by not being edited, which is how both misses survived changesetGateAccuracy.test.mjs, whose IGNORED fixture did not move when the real list did. Two of the four private packages are not under packages/ at all: pnpm-workspace picks up `playground` and `docs` separately. A check that walked packages/* would have declared the list complete while missing half of what it is about, so this uses @manypkg/get-packages -- the enumerator changesets itself uses, now an explicit devDependency. Reimplementing the tool's discovery to guard the tool's behaviour is the shape contributing/test-and-guard-coverage.md calls a floor rather than a fence. The reverse direction is asserted too: an entry naming a package that has been published since, renamed or deleted exempts nothing while reading as deliberate, and what it hides is a package quietly not getting release notes. SHIPS_DESPITE_PRIVATE is read from check-changeset.mjs rather than restated, so the two lists cannot drift. Closes #540. <!-- no-changeset: scripts and a root devDependency only, no published source --> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adversarial review of 04d60a4. Three holes, all in the guard rather than around it. The header said the #537 damage was "no published package got a release note, with the CI changeset job green", and the commit closed neither half of that sentence. `if (added.length > 0)` accepts any changeset; mixedChangesets only catches a file naming both sides. A changeset naming *only* ignored packages sails through -- assemble-release-plan produces no release for it and changeset version deletes the file, so published source ships with nothing written about it. That is the mistake the root AGENTS.md warns about by name: a dashboard change must name @tapflowio/relay. Adding the private packages to ignore made it quieter, not louder. Before, such a changeset at least bumped an unpublished package and wrote it a CHANGELOG, visible in the release PR's diff. Now it emits nothing. ignoredOnlyChangesets closes it, beside the mixed check it is the other half of. ignoredPackages() read a relative path inside a bare catch, so any invocation whose cwd was not the repo root returned "nothing is ignored" -- switching off the mixed check that exists because four mixed changesets stopped the v0.18.0 release. The new test read the config itself, so it was green under exactly the conditions that break the gate. It now goes through that function, and the function fails loudly instead of open. And the anti-vacuity floor was `> 0` against a measured 4. Swapping getPackages for a packages/* walk finds 2 of the 4 and every assertion still passes -- the one enumerator failure the header says getPackages was chosen to prevent. Floor is the measured count now. `private === true` was also stricter than changesets' own shouldSkipPackage, which tests truthiness. Six mutations run, all killed, including one proving the guard now observes the reader it guards. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThe changeset gate now rejects branches whose added changesets affect only ignored packages. Tests validate ignored-only detection and ensure ChangesChangeset validation
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to This localized change adds validation and tests for private-package changeset coverage; no actionable merge-blocking risk remains after normal checks and review. Sequence Diagram(s)sequenceDiagram
participant PRGate
participant ignoredPackages
participant ignoredOnlyChangesets
participant AddedChangesets
PRGate->>ignoredPackages: Read ignored package names
PRGate->>AddedChangesets: Collect added changeset files
PRGate->>ignoredOnlyChangesets: Check package names
ignoredOnlyChangesets-->>PRGate: Return ignored-only changesets
PRGate-->>PRGate: Reject when all added changesets are ignored-only
Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #540.
.changeset/config.json'signorehas to name every private workspace package and nothing checked that it did — the list fell behind twice in one week, both times caught by a human reading the diff.Two of the four private packages are not under
packages/at all, so this uses@manypkg/get-packages, the enumerator changesets itself uses. A check that walkedpackages/*would have declared the list complete while missing half of what it is about.The review then found the guard had the hole it was written to close. Its header said the damage was "no release note, with the CI job green", and neither half was closed: a changeset naming only ignored packages passes the gate, produces no release, and is deleted by
changeset version— the mistake AGENTS.md warns about by name. Adding the private packages toignoremade that quieter rather than louder, since it no longer even bumps an unpublished package. Also fixed:ignoredPackages()read a relative path inside a bare catch, so a non-root cwd silently disabled the mixed-changeset check, and the new test read the config itself rather than through that function — green under exactly the conditions that break the gate.Checklist
anyagent-corefirst — n/aRelated
.work/docs.work/reviews/test__changeset-ignore-covers-private.md— one lens, findings and dispositions, the deliberate non-fix (ignoreentries are micromatch patterns and this compares literals: loud and safe, so stated as a constraint rather than filed), and a rule violation worth recording.🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Tests