chore: Fix changeset config + add version cleanup scripts#810
Conversation
|
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughChanges expand Changesets fixed pattern to include plugin packages, add post-version tooling to avoid republishing already-published versions (skip list, collector, unpublisher, and a version-fix script), and update check-changesets to filter by git diff/last tag and broadened scripts matching in knip config. Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant Changeset as Changeset CLI
participant FixVer as fix-versions.js
participant Collector as collect-bad-versions.js
participant Unpub as unpublish.js
participant NPM as npm Registry
participant Git as Git
Dev->>Changeset: pnpm changeset version
Changeset->>Changeset: bump workspace package versions
Changeset->>FixVer: run post-version script
FixVer->>FixVer: read published-version-skip-list.json
FixVer->>FixVer: detect skipped versions and bump patch
FixVer->>FixVer: update package.json and CHANGELOG.md
Dev->>Collector: run collect-bad-versions
Collector->>NPM: fetch published versions & latest tags
Collector->>Collector: compute bad versions
Collector->>Collector: write published-version-skip-list.json
Dev->>Unpub: run unpublish [--dry-run]
Unpub->>Unpub: read published-version-skip-list.json
Unpub->>NPM: check & unpublish listed versions
Unpub->>Git: optionally delete git tags for plugin packages
Unpub->>Unpub: report results
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/version-fix/unpublish.js`:
- Around line 178-197: The early continue prevents cleanup of stale plugin git
tags when the npm version is already gone; adjust the logic in the block that
checks isDryRun and isVersionPublished so that for plugins (use isPlugin and
semverMajor(version)) you still call deleteGitTag(tag) before continuing. Keep
the existing behavior for totalSkipped/totalUnpublished and only call
unpublishVersion(packageName, version) when appropriate (preserve the existing
unpublish path with unpublishVersion and its success handling).
- Around line 97-103: The isVersionPublished(packageName, version) function
currently swallows all errors from the `run('npm view ...')` call and returns
false; change it to detect an actual 404 (package/version not found) and return
false only in that case, while re-throwing any other errors (auth, network,
registry) so they propagate. Concretely, catch the error thrown by run in
isVersionPublished, inspect the error output/code/message for the npm 404
pattern (e.g., "404" or "npm ERR! 404" / ENOTFOUND depending on run's error
shape), return false when it matches the 404 case, and otherwise re-throw the
error; keep the function signature and callers (unpublishVersion, main loop)
unchanged so they will now fail on transient failures instead of treating them
as "already gone."
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9597e0da-a3cd-4dc2-b088-e96a9672aa29
📒 Files selected for processing (7)
.changeset/config.json.github/workflows/release.ymlpackage.jsonscripts/fix-versions.jsscripts/published-version-skip-list.jsonscripts/version-fix/collect-bad-versions.jsscripts/version-fix/unpublish.js
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/check-changesets.ts`:
- Around line 299-317: The prefix-comparison in the affectedPackageNames
calculation can miss matches on Windows because changedFilePaths (from git diff)
uses POSIX-style '/' while packageItem.path may contain platform separators;
normalize both sides before comparing: transform each entry of changedFilePaths
and packageItem.path to a consistent POSIX-style form (e.g., replace backslashes
with '/' and run a normalization) before performing the startsWith check in the
affectedPackageNames mapping over turboData.packages.items so the comparison
correctly detects changed files across platforms.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 863196e9-ee11-4ce0-bc57-0956a429f463
📒 Files selected for processing (2)
knip.config.jsscripts/check-changesets.ts
| if (lastTag) { | ||
| const { stdout: diffOutput } = await execAsync( | ||
| `git diff "${lastTag}"..HEAD --name-only`, | ||
| ); | ||
| changedFilePaths = new Set(diffOutput.trim().split('\n').filter(Boolean)); | ||
| } | ||
|
|
||
| const affectedPackageNames = turboData.packages.items | ||
| .map((item) => item.name) | ||
| .filter((name) => { | ||
| if (!changedFilePaths) return true; | ||
| const packageItem = turboData.packages.items.find( | ||
| (i) => i.name === name, | ||
| ); | ||
| if (!packageItem) return true; | ||
| return [...changedFilePaths].some((f) => | ||
| f.startsWith(`${packageItem.path}/`), | ||
| ); | ||
| }); |
There was a problem hiding this comment.
Normalize both path sources before the prefix check.
git diff --name-only uses /, but packageItem.path can be platform-native. On Windows, the current startsWith(${packageItem.path}/) check can miss every changed package and silently skip validation.
💡 Suggested fix
if (lastTag) {
const { stdout: diffOutput } = await execAsync(
`git diff "${lastTag}"..HEAD --name-only`,
);
changedFilePaths = new Set(diffOutput.trim().split('\n').filter(Boolean));
}
- const affectedPackageNames = turboData.packages.items
- .map((item) => item.name)
- .filter((name) => {
- if (!changedFilePaths) return true;
- const packageItem = turboData.packages.items.find(
- (i) => i.name === name,
- );
- if (!packageItem) return true;
- return [...changedFilePaths].some((f) =>
- f.startsWith(`${packageItem.path}/`),
- );
- });
+ const normalizeRepoPath = (value: string): string =>
+ value.replaceAll('\\', '/').replace(/\/+$/, '');
+
+ const normalizedChangedFilePaths = changedFilePaths
+ ? [...changedFilePaths].map(normalizeRepoPath)
+ : null;
+
+ const affectedPackageNames = turboData.packages.items
+ .filter((item) => {
+ if (!normalizedChangedFilePaths) return true;
+ const packagePathPrefix = `${normalizeRepoPath(item.path)}/`;
+ return normalizedChangedFilePaths.some((file) =>
+ file.startsWith(packagePathPrefix),
+ );
+ })
+ .map((item) => item.name);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (lastTag) { | |
| const { stdout: diffOutput } = await execAsync( | |
| `git diff "${lastTag}"..HEAD --name-only`, | |
| ); | |
| changedFilePaths = new Set(diffOutput.trim().split('\n').filter(Boolean)); | |
| } | |
| const affectedPackageNames = turboData.packages.items | |
| .map((item) => item.name) | |
| .filter((name) => { | |
| if (!changedFilePaths) return true; | |
| const packageItem = turboData.packages.items.find( | |
| (i) => i.name === name, | |
| ); | |
| if (!packageItem) return true; | |
| return [...changedFilePaths].some((f) => | |
| f.startsWith(`${packageItem.path}/`), | |
| ); | |
| }); | |
| if (lastTag) { | |
| const { stdout: diffOutput } = await execAsync( | |
| `git diff "${lastTag}"..HEAD --name-only`, | |
| ); | |
| changedFilePaths = new Set(diffOutput.trim().split('\n').filter(Boolean)); | |
| } | |
| const normalizeRepoPath = (value: string): string => | |
| value.replaceAll('\\', '/').replace(/\/+$/, ''); | |
| const normalizedChangedFilePaths = changedFilePaths | |
| ? [...changedFilePaths].map(normalizeRepoPath) | |
| : null; | |
| const affectedPackageNames = turboData.packages.items | |
| .filter((item) => { | |
| if (!normalizedChangedFilePaths) return true; | |
| const packagePathPrefix = `${normalizeRepoPath(item.path)}/`; | |
| return normalizedChangedFilePaths.some((file) => | |
| file.startsWith(packagePathPrefix), | |
| ); | |
| }) | |
| .map((item) => item.name); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/check-changesets.ts` around lines 299 - 317, The prefix-comparison in
the affectedPackageNames calculation can miss matches on Windows because
changedFilePaths (from git diff) uses POSIX-style '/' while packageItem.path may
contain platform separators; normalize both sides before comparing: transform
each entry of changedFilePaths and packageItem.path to a consistent POSIX-style
form (e.g., replace backslashes with '/' and run a normalization) before
performing the startsWith check in the affectedPackageNames mapping over
turboData.packages.items so the comparison correctly detects changed files
across platforms.
Summary by CodeRabbit