Add changelog and release workflow for npm publishing#6
Add changelog and release workflow for npm publishing#6
Conversation
WalkthroughAdds release infrastructure: comprehensive CHANGELOG with v1.0.0, README release links, a maintainer releasing guide, release scripts and config, a new Bash release orchestration script, and a GitHub Actions workflow to publish releases on pushes to main. Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant Script as ./scripts/release.sh
participant Git as Git (local & remote)
participant CI as CI / Tests & Build
participant NPM as npm Registry
participant GH as GitHub
Dev->>Script: run (--dry-run / --npm-tag / --skip-tests)
Script->>Git: verify branch == main & working tree clean
Script->>Script: extract target version from CHANGELOG & package.json
Script->>Git: ensure no existing tag v{target} (local & remote)
alt tests/build enabled
Script->>CI: run tests & build
CI-->>Script: result
end
Script->>NPM: npm publish (--tag X) [--dry-run]
alt not dry-run
Script->>Git: git tag v{target} & push
Script->>GH: create GitHub release with changelog notes
end
Script-->>Dev: print summary & exit
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| esac | ||
| done | ||
|
|
||
| if ! git diff --quiet || ! git diff --cached --quiet; then |
There was a problem hiding this comment.
The working-tree check misses untracked files. git diff only reports changes to tracked files, so a dirty tree with new, uncommitted files will pass this guard silently.
Use git status --porcelain instead:
| if ! git diff --quiet || ! git diff --cached --quiet; then | |
| if [[ -n "$(git status --porcelain)" ]]; then |
| CURRENT_BRANCH="$(git rev-parse --abbrev-ref HEAD)" | ||
| if [[ "$CURRENT_BRANCH" != "main" ]]; then | ||
| echo "Error: release must be run from main (current: $CURRENT_BRANCH)." >&2 | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
The branch check confirms you're on main, but it doesn't verify that local main is up-to-date with origin/main. A maintainer could inadvertently release from a stale local branch that is missing recent commits.
Consider adding a sync check after the branch check:
| CURRENT_BRANCH="$(git rev-parse --abbrev-ref HEAD)" | |
| if [[ "$CURRENT_BRANCH" != "main" ]]; then | |
| echo "Error: release must be run from main (current: $CURRENT_BRANCH)." >&2 | |
| exit 1 | |
| fi | |
| CURRENT_BRANCH="$(git rev-parse --abbrev-ref HEAD)" | |
| if [[ "$CURRENT_BRANCH" != "main" ]]; then | |
| echo "Error: release must be run from main (current: $CURRENT_BRANCH)." >&2 | |
| exit 1 | |
| fi | |
| git fetch origin main --quiet | |
| LOCAL=$(git rev-parse HEAD) | |
| REMOTE=$(git rev-parse origin/main) | |
| if [[ "$LOCAL" != "$REMOTE" ]]; then | |
| echo "Error: local main is not in sync with origin/main. Run 'git pull' first." >&2 | |
| exit 1 | |
| fi |
scripts/release.sh
Outdated
| if [[ "$SKIP_TESTS" == false ]]; then | ||
| echo "Running tests..." | ||
| npm test | ||
| echo "Building dist..." | ||
| npm run build | ||
| else | ||
| echo "Skipping tests/build checks." | ||
| fi |
There was a problem hiding this comment.
package.json already has "prepublishOnly": "npm run build", so npm publish will trigger a second build automatically. The explicit npm run build here means the build runs twice on every release.
You can drop the redundant explicit build call and rely on prepublishOnly:
| if [[ "$SKIP_TESTS" == false ]]; then | |
| echo "Running tests..." | |
| npm test | |
| echo "Building dist..." | |
| npm run build | |
| else | |
| echo "Skipping tests/build checks." | |
| fi | |
| if [[ "$SKIP_TESTS" == false ]]; then | |
| echo "Running tests..." | |
| npm test | |
| else | |
| echo "Skipping tests." | |
| fi |
|
|
||
| ### Build | ||
|
|
||
| - Added `prepare` script so installs from GitHub build `dist/` automatically. |
There was a problem hiding this comment.
The [Unreleased] and [v1.0.0] headings use Keep a Changelog's link-reference style but there are no link definitions at the bottom of the file. Most renderers and changelog tooling (e.g. git-cliff, standard-version) expect comparison links here.
Add these at the end of the file:
[Unreleased]: https://github.com/shakacode/pack-config-diff/compare/v1.0.0...HEAD
[v1.0.0]: https://github.com/shakacode/pack-config-diff/releases/tag/v1.0.0
Review: Add changelog and release workflow for npm publishingOverall this is a solid, well-structured release workflow. The script covers the important safety rails (clean tree, branch check, tag de-dup, changelog validation) and the docs are clear. A few things worth fixing before merging: Issues
Minor / Nits
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5860cc875e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| esac | ||
| done | ||
|
|
||
| if ! git diff --quiet || ! git diff --cached --quiet; then |
There was a problem hiding this comment.
The clean-tree guard only checks tracked/staged changes via git diff --quiet and git diff --cached --quiet, so untracked files are treated as clean and the release continues. That can leak stale artifacts into a publish (for example, extra files left in dist/) even though the script reports the working tree is clean. This is introduced by the new check and should use a status command that also covers untracked files.
Useful? React with 👍 / 👎.
scripts/release.sh
Outdated
| exit 1 | ||
| fi | ||
|
|
||
| if git ls-remote --exit-code --tags origin "refs/tags/v$VERSION" >/dev/null 2>&1; then |
There was a problem hiding this comment.
The remote-tag check treats every non-zero git ls-remote --exit-code status as “tag not found” because it is used directly as an if condition. Per git ls-remote -h, --exit-code returns 2 when no matching refs are found, but auth/network/remote errors return different non-zero codes; those should abort the release. As written, an unreachable or unauthorized origin can still proceed to npm publish, then fail at final tag push, leaving npm published without the expected git tag.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
CHANGELOG.md (1)
5-5: Consider linking to Keep a Changelog and Semantic Versioning.Adding links helps contributors understand the conventions being followed.
📝 Optional: Add reference links
-The format is based on Keep a Changelog and this project follows Semantic Versioning. +The format is based on [Keep a Changelog](https://keepachangelog.com/) and this project follows [Semantic Versioning](https://semver.org/).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CHANGELOG.md` at line 5, Update the sentence "The format is based on Keep a Changelog and this project follows Semantic Versioning." to include hyperlinks to the referenced standards (Keep a Changelog and Semantic Versioning) so contributors can quickly access them; specifically, modify the line in CHANGELOG.md that contains that sentence to embed or append the URLs for Keep a Changelog and SemVer (e.g., link text "Keep a Changelog" -> https://keepachangelog.com and "Semantic Versioning" -> https://semver.org).scripts/release.sh (2)
73-81: Consider fetching before checking remote tags.The remote tag check queries origin directly via
git ls-remote, which should be current. However, in fast-moving environments, agit fetch --tagsbefore the check could prevent race conditions where another maintainer just pushed a tag.This is a minor edge case and the current implementation is functional.
🔧 Optional: Add git fetch before tag checks
+echo "Fetching latest tags from origin..." +git fetch --tags --quiet + if git rev-parse -q --verify "refs/tags/v$VERSION" >/dev/null; then echo "Error: git tag v$VERSION already exists locally." >&2 exit 1 fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/release.sh` around lines 73 - 81, Add a tags fetch before the remote tag existence check to avoid races: run a `git fetch --tags` (or `git fetch --tags origin`) prior to calling `git ls-remote --exit-code --tags origin "refs/tags/v$VERSION"` so the script uses up-to-date remote tags; keep the local check with `git rev-parse -q --verify "refs/tags/v$VERSION"` and the variable `VERSION` unchanged.
106-114: Minor: Tag push failure leaves local tag.If
git push origin "v$VERSION"fails aftergit tagsucceeds, the local tag remains. On retry, the script will error with "tag already exists locally." The user would need to manually delete the local tag withgit tag -d "v$VERSION"before retrying.This is acceptable behavior given
set -eensures failures are loud, and documenting this edge case in the releasing guide could help.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/release.sh` around lines 106 - 114, Wrap the git push step so that if git push origin "v$VERSION" fails you delete the just-created local tag to avoid left-over local tags; specifically after git tag -a "v$VERSION" -m "v$VERSION" run git push origin "v$VERSION" and on its non-zero exit path run git tag -d "v$VERSION" and then exit with the push failure code (or propagate the error). Use the existing VERSION variable and the exact tag commands shown (git tag -a "v$VERSION" and git push origin "v$VERSION") so the script cleans up the local tag when push fails.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@CHANGELOG.md`:
- Line 5: Update the sentence "The format is based on Keep a Changelog and this
project follows Semantic Versioning." to include hyperlinks to the referenced
standards (Keep a Changelog and Semantic Versioning) so contributors can quickly
access them; specifically, modify the line in CHANGELOG.md that contains that
sentence to embed or append the URLs for Keep a Changelog and SemVer (e.g., link
text "Keep a Changelog" -> https://keepachangelog.com and "Semantic Versioning"
-> https://semver.org).
In `@scripts/release.sh`:
- Around line 73-81: Add a tags fetch before the remote tag existence check to
avoid races: run a `git fetch --tags` (or `git fetch --tags origin`) prior to
calling `git ls-remote --exit-code --tags origin "refs/tags/v$VERSION"` so the
script uses up-to-date remote tags; keep the local check with `git rev-parse -q
--verify "refs/tags/v$VERSION"` and the variable `VERSION` unchanged.
- Around line 106-114: Wrap the git push step so that if git push origin
"v$VERSION" fails you delete the just-created local tag to avoid left-over local
tags; specifically after git tag -a "v$VERSION" -m "v$VERSION" run git push
origin "v$VERSION" and on its non-zero exit path run git tag -d "v$VERSION" and
then exit with the push failure code (or propagate the error). Use the existing
VERSION variable and the exact tag commands shown (git tag -a "v$VERSION" and
git push origin "v$VERSION") so the script cleans up the local tag when push
fails.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8653c2ac-9137-4f96-b603-231f9dd9b770
📒 Files selected for processing (5)
CHANGELOG.mdREADME.mddocs/releasing.mdpackage.jsonscripts/release.sh
| fi | ||
|
|
||
| PUBLISH_ARGS=(publish --access public) | ||
| if [[ -n "$NPM_TAG" ]]; then | ||
| PUBLISH_ARGS+=(--tag "$NPM_TAG") | ||
| fi |
There was a problem hiding this comment.
Bug: Prerelease versions can silently publish to latest
If VERSION contains a prerelease identifier (e.g. 1.0.0-rc.0), npm publish without --tag defaults to the latest dist-tag — exactly what the docs warn against.
The script should auto-detect prerelease semver and either enforce --npm-tag or default to next:
| fi | |
| PUBLISH_ARGS=(publish --access public) | |
| if [[ -n "$NPM_TAG" ]]; then | |
| PUBLISH_ARGS+=(--tag "$NPM_TAG") | |
| fi | |
| PUBLISH_ARGS=(publish --access public) | |
| # Safety: detect prerelease versions (contain "-") and require an explicit npm tag. | |
| if [[ "$VERSION" == *-* ]] && [[ -z "$NPM_TAG" ]]; then | |
| echo "Error: prerelease version '$VERSION' detected. Pass --npm-tag (e.g. --npm-tag next) to avoid publishing to 'latest'." >&2 | |
| exit 1 | |
| fi | |
| if [[ -n "$NPM_TAG" ]]; then | |
| PUBLISH_ARGS+=(--tag "$NPM_TAG") | |
| fi | |
| if [[ "$DRY_RUN" == true ]]; then | |
| PUBLISH_ARGS+=(--dry-run) | |
| fi |
scripts/release.sh
Outdated
|
|
||
| if [[ "$SKIP_TESTS" == false ]]; then | ||
| echo "Running tests..." | ||
| npm test | ||
| echo "Building dist..." | ||
| npm run build | ||
| else |
There was a problem hiding this comment.
Redundant double build
npm run build is called explicitly here, but package.json has "prepublishOnly": "npm run build", which means npm publish (line 103) will trigger a second build automatically.
Either:
- Remove the explicit
npm run buildhere and rely onprepublishOnly(preferred — single source of truth), or - Remove
prepublishOnlyfrompackage.jsonand keep only the explicit call here.
Also note: --skip-tests currently advertises skipping both tests and build, but the build will still run via prepublishOnly regardless.
|
|
||
| if ! grep -Fq "$CHANGELOG_HEADING" CHANGELOG.md; then | ||
| echo "Error: CHANGELOG.md is missing heading: $CHANGELOG_HEADING" >&2 | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
Missing check: local main may be out of sync with origin/main
This confirms the local branch name is main but doesn't verify it's up-to-date with the remote. A release could be cut from a main that has unpushed commits, or that is behind origin/main.
Consider adding:
| if ! grep -Fq "$CHANGELOG_HEADING" CHANGELOG.md; then | |
| echo "Error: CHANGELOG.md is missing heading: $CHANGELOG_HEADING" >&2 | |
| exit 1 | |
| fi | |
| CURRENT_BRANCH="$(git rev-parse --abbrev-ref HEAD)" | |
| if [[ "$CURRENT_BRANCH" != "main" ]]; then | |
| echo "Error: release must be run from main (current: $CURRENT_BRANCH)." >&2 | |
| exit 1 | |
| fi | |
| # Ensure local main is in sync with origin/main. | |
| git fetch origin main --quiet | |
| LOCAL_SHA="$(git rev-parse HEAD)" | |
| REMOTE_SHA="$(git rev-parse origin/main)" | |
| if [[ "$LOCAL_SHA" != "$REMOTE_SHA" ]]; then | |
| echo "Error: local main is not in sync with origin/main. Pull or push first." >&2 | |
| exit 1 | |
| fi |
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 `@docs/releasing.md`:
- Around line 147-148: The doc uses two spellings for the same term; replace the
instance of "pre-release" in the sentence that mentions `--npm-tag next` with
the consistent form "prerelease" (so the line reads "Use `--npm-tag next` for
prerelease channels."), and scan the rest of docs to ensure only "prerelease" is
used instead of "pre-release".
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9da60ad9-7de9-423f-b218-dfc05c5e530d
📒 Files selected for processing (2)
README.mddocs/releasing.md
docs/releasing.md
Outdated
| - Use `--npm-tag next` for pre-release channels. | ||
| - `--skip-tests` should be used only in exceptional cases. |
There was a problem hiding this comment.
Use one term consistently: “prerelease” vs “pre-release”.
Line 147 uses “pre-release” while the rest of the doc predominantly uses “prerelease”. Standardize to one form for consistency.
Suggested doc tweak
-- Use `--npm-tag next` for pre-release channels.
+- Use `--npm-tag next` for prerelease channels.📝 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.
| - Use `--npm-tag next` for pre-release channels. | |
| - `--skip-tests` should be used only in exceptional cases. | |
| - Use `--npm-tag next` for prerelease channels. | |
| - `--skip-tests` should be used only in exceptional cases. |
🧰 Tools
🪛 LanguageTool
[uncategorized] ~147-~147: Do not mix variants of the same word (‘pre-release’ and ‘prerelease’) within a single text.
Context: ...``` Notes: - Use --npm-tag next for pre-release channels. - `--skip-tests` should be us...
(EN_WORD_COHERENCY)
[style] ~148-~148: To form a complete sentence, be sure to include a subject.
Context: ... pre-release channels. - --skip-tests should be used only in exceptional cases. ## ...
(MISSING_IT_THERE)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/releasing.md` around lines 147 - 148, The doc uses two spellings for the
same term; replace the instance of "pre-release" in the sentence that mentions
`--npm-tag next` with the consistent form "prerelease" (so the line reads "Use
`--npm-tag next` for prerelease channels."), and scan the rest of docs to ensure
only "prerelease" is used instead of "pre-release".
| exit 0 | ||
| fi | ||
|
|
||
| if ! grep -Fq "## [v$TARGET_VERSION]" CHANGELOG.md; then |
There was a problem hiding this comment.
This check is redundant. TARGET_VERSION was already extracted from the exact same pattern on line 65 — if that grep succeeded, this one will always succeed too. The only way TARGET_VERSION could be set without this heading existing is if the sed replacement somehow produced output from a different line, which can't happen given the regex. Safe to remove lines 78–81.
scripts/release.sh
Outdated
| exit 0 | ||
| fi | ||
|
|
||
| if [[ -z "${NPM_TOKEN:-}" ]]; then |
There was a problem hiding this comment.
The NPM_TOKEN check is too late — tests and the full build have already run before reaching this point. If NPM_TOKEN isn't configured, you've wasted CI minutes on a release that was never going to publish. Move this check (and any other preconditions) to the top of the script, before SKIP_TESTS and before running tests.
scripts/release.sh
Outdated
| git tag -a "v$TARGET_VERSION" -m "v$TARGET_VERSION" | ||
| git push origin "v$TARGET_VERSION" |
There was a problem hiding this comment.
Inconsistent failure state: if npm publish (line 149) succeeds but git push origin "v$TARGET_VERSION" fails (network blip, permission error, etc.), the package is live on npm with no corresponding git tag. A subsequent re-run will then fail at the tag existence check (lines 83–86), leaving you stuck.
Consider pushing the tag before publishing so a failed push can be safely retried. Alternatively, add a trap ERR that prints remediation instructions when the tag push fails post-publish.
scripts/release.sh
Outdated
| RELEASE_NOTES_FILE="$(mktemp)" | ||
| awk -v version="$TARGET_VERSION" ' | ||
| $0 ~ "^## \\[v"version"\\]" { in_section=1; next } | ||
| in_section && $0 ~ "^## \\[v" { in_section=0 } | ||
| in_section { print } | ||
| ' CHANGELOG.md > "$RELEASE_NOTES_FILE" |
There was a problem hiding this comment.
Two issues here:
-
Temp file leak:
mktempoutput is never cleaned up. Add a trap at the top of this block:Suggested changeRELEASE_NOTES_FILE="$(mktemp)" awk -v version="$TARGET_VERSION" ' $0 ~ "^## \\[v"version"\\]" { in_section=1; next } in_section && $0 ~ "^## \\[v" { in_section=0 } in_section { print } ' CHANGELOG.md > "$RELEASE_NOTES_FILE" RELEASE_NOTES_FILE="$(mktemp)" trap 'rm -f "$RELEASE_NOTES_FILE"' EXIT awk -v version="$TARGET_VERSION" ' $0 ~ "^## \\[v"version"\\]" { in_section=1; next } in_section && $0 ~ "^## \\[v" { in_section=0 } in_section { print } ' CHANGELOG.md > "$RELEASE_NOTES_FILE" -
Regex vs literal in
awk:versionis used in a regex context ($0 ~ "^## \\[v"version"\\]"), so dots in1.0.0are treated as "any character". Useindex()or escape the dots:$0 ~ "^## \\[v"version"\\]" { ... }
Replace dots with
\\.in the version string before passing it to awk, or useindex($0, "## [v"version"]") == 1for a literal match.
scripts/release.sh
Outdated
| gh release create "v$TARGET_VERSION" \ | ||
| --title "v$TARGET_VERSION" \ | ||
| --notes-file "$RELEASE_NOTES_FILE" |
There was a problem hiding this comment.
Prerelease versions (1.2.3-rc.0, 1.2.3-beta.0) should be created with the --prerelease flag so GitHub marks them correctly and they don't appear as the "latest" release:
| gh release create "v$TARGET_VERSION" \ | |
| --title "v$TARGET_VERSION" \ | |
| --notes-file "$RELEASE_NOTES_FILE" | |
| gh release create "v$TARGET_VERSION" \ | |
| --title "v$TARGET_VERSION" \ | |
| $([[ "$TARGET_VERSION" == *-* ]] && echo "--prerelease") \ | |
| --notes-file "$RELEASE_NOTES_FILE" |
| fetch-depth: 0 | ||
|
|
||
| - name: Setup Node | ||
| uses: actions/setup-node@v4 | ||
| with: | ||
| node-version: 20 |
There was a problem hiding this comment.
Actions are pinned to mutable major-version tags (@v4), not immutable commit SHAs. A compromised or accidentally broken actions/checkout@v4 or actions/setup-node@v4 release would affect this workflow with no notice. Since this workflow has contents: write and publishes to npm, the blast radius of a supply-chain issue here is significant.
Pin to commit SHAs, e.g.:
uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2uses: actions/setup-node@39370e3970a6d050c480ffad4ff0ed4d3fdee5af # v4.1.0
Review: Release Workflow & ScriptsThe overall approach is solid — changelog-driven releases with idempotency and dry-run support are the right patterns. A few issues worth addressing before merging: BlockingInconsistent post-publish state ( Branch protection may block CI push to Non-blocking but worth fixing
|
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/release.sh`:
- Around line 73-76: The release flow currently commits/pushes the package.json
version bump before running npm publish (see checks using TARGET_VERSION and
CURRENT_VERSION), which can leave main bumped if publish fails; change the
sequence so npm publish is executed first (using the current working tree or a
temporary pack), and only after a successful publish perform the commit/push and
git tag steps for the version bump (i.e., move the publish step to run before
the code that creates the bump commit and calls git push/tag), and ensure any
temporary changes are cleaned up if publish fails so subsequent runs can retry.
- Around line 54-57: The release pre-check IF block only tests tracked/staged
changes using "git diff --quiet" and "git diff --cached --quiet" and therefore
misses untracked files; update the check to also detect untracked files (e.g.,
run "git status --porcelain" or "git ls-files --others --exclude-standard") and
fail if any untracked entries are present so the script's clean-working-tree
condition is strict; modify the conditional that contains the git diff checks to
include this untracked-files test and ensure the same error message/exit path is
used when untracked files are found.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2aa448e2-2a10-48a8-84fb-0ec5d004f346
📒 Files selected for processing (4)
.github/workflows/release-on-main.ymlREADME.mddocs/releasing.mdscripts/release.sh
| if ! git diff --quiet || ! git diff --cached --quiet; then | ||
| echo "Error: git working tree must be clean before release." >&2 | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
“Clean working tree” check misses untracked files.
Current checks only cover tracked/staged diffs. Untracked files still pass, which weakens the release precondition.
Suggested fix
-if ! git diff --quiet || ! git diff --cached --quiet; then
+if [[ -n "$(git status --porcelain)" ]]; then
echo "Error: git working tree must be clean before release." >&2
exit 1
fi📝 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 ! git diff --quiet || ! git diff --cached --quiet; then | |
| echo "Error: git working tree must be clean before release." >&2 | |
| exit 1 | |
| fi | |
| if [[ -n "$(git status --porcelain)" ]]; then | |
| echo "Error: git working tree must be clean before release." >&2 | |
| exit 1 | |
| fi |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/release.sh` around lines 54 - 57, The release pre-check IF block only
tests tracked/staged changes using "git diff --quiet" and "git diff --cached
--quiet" and therefore misses untracked files; update the check to also detect
untracked files (e.g., run "git status --porcelain" or "git ls-files --others
--exclude-standard") and fail if any untracked entries are present so the
script's clean-working-tree condition is strict; modify the conditional that
contains the git diff checks to include this untracked-files test and ensure the
same error message/exit path is used when untracked files are found.
| runs-on: ubuntu-latest | ||
| steps: | ||
| - name: Checkout | ||
| uses: actions/checkout@v4 |
There was a problem hiding this comment.
Security: action tags (@v4) are mutable — a tag can be moved to a different commit without notice. Pin to a full commit SHA to prevent supply-chain attacks:
| uses: actions/checkout@v4 | |
| uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 |
Same for actions/setup-node@v4 below.
|
|
||
| CURRENT_VERSION="$(node -p "require('./package.json').version")" | ||
|
|
||
| if [[ "$TARGET_VERSION" == "$CURRENT_VERSION" ]]; then |
There was a problem hiding this comment.
This guard only prevents a no-op re-release, but it won't catch a downgrade. If a maintainer accidentally puts v0.9.0 at the top of CHANGELOG while package.json is already at 1.0.0, the script would happily proceed to publish v0.9.0 to npm.
Consider adding a semver ordering check before this early-exit, e.g. using npx semver or a simple string comparison guard:
# After extracting TARGET_VERSION and CURRENT_VERSION:
if [[ "$TARGET_VERSION" == "$CURRENT_VERSION" ]]; then
echo "No release needed: package.json already at v$CURRENT_VERSION."
exit 0
fi
# Guard against accidental downgrades
if npx --yes semver --range ">$CURRENT_VERSION" "$TARGET_VERSION" >/dev/null 2>&1; then
: # target is newer, proceed
else
echo "Error: changelog version v$TARGET_VERSION is not newer than current v$CURRENT_VERSION." >&2
exit 1
fi
| exit 1 | ||
| fi | ||
|
|
||
| CURRENT_BRANCH="$(git rev-parse --abbrev-ref HEAD)" |
There was a problem hiding this comment.
The script verifies the branch name but doesn't check whether local main is up-to-date with origin/main. If CI checks out the commit, runs tests, then a second push lands — the release commit could be pushed onto a stale base and conflict mid-flight.
Add a remote sync guard:
git fetch origin main --quiet
if ! git merge-base --is-ancestor origin/main HEAD; then
echo "Error: local main is behind origin/main. Pull first." >&2
exit 1
fi
| echo "Skipping tests/build checks (SKIP_RELEASE_TESTS=true)." | ||
| fi | ||
|
|
||
| echo "Running: npx release-it ${RELEASE_IT_ARGS[*]}" |
There was a problem hiding this comment.
Prefer the locally installed binary over npx to ensure the exact version from package-lock.json is used and avoid any npx resolution surprises:
| echo "Running: npx release-it ${RELEASE_IT_ARGS[*]}" | |
| ./node_modules/.bin/release-it "${RELEASE_IT_ARGS[@]}" |
| @@ -0,0 +1,20 @@ | |||
| { | |||
There was a problem hiding this comment.
Consider adding "requireUpstream": true to the git section. Without it, release-it won't verify that the local branch tracks a remote and is up-to-date, which complements the clean-tree check but doesn't replace it.
| { | |
| { | |
| "git": { | |
| "requireCleanWorkingDir": true, | |
| "requireUpstream": true, | |
| "requireBranch": "main", |
| "@types/jest": "^29.0.0", | ||
| "@types/js-yaml": "^4.0.0", | ||
| "jest": "^29.0.0", | ||
| "release-it": "^17.11.0", |
There was a problem hiding this comment.
Adding release-it as a dev dependency pulls in ~250 transitive packages (octokit, puppeteer-adjacent browser-compat libs, etc.) as visible in the lockfile diff (+3,795 lines). This meaningfully inflates npm install times for contributors.
A lighter alternative is to keep release-it out of package.json entirely and rely on npx release-it@17 directly in scripts/release.sh for the CI/release path only, or install it globally in the release workflow step. That way regular contributor installs stay lean.
| branches: | ||
| - main | ||
|
|
||
| permissions: |
There was a problem hiding this comment.
The contents: write permission is correct for tagging and creating GitHub releases, but it's set at the workflow level, granting it to all jobs (currently just one). Consider using job-level permissions to keep the scope minimal and make the intent explicit:
| permissions: | |
| permissions: | |
| contents: read | |
| jobs: | |
| release: | |
| runs-on: ubuntu-latest | |
| permissions: | |
| contents: write |
Review: Release InfrastructureThe overall approach is solid — changelog-driven releases via Issues to addressSecurity
Correctness / reliability
Dependency footprint
What looks good
|
There was a problem hiding this comment.
🧹 Nitpick comments (2)
.github/workflows/release-on-main.yml (1)
8-9: Consider addingid-token: writefor npm provenance.If you plan to enable npm provenance (recommended for supply chain security), you'll need to add
id-token: writepermission and configureprovenance: truein your npm publish settings. This is optional but increasingly a best practice.Optional enhancement for npm provenance
permissions: contents: write + id-token: writeThen in
.release-it.json:"npm": { "publish": true, "publishArgs": ["--provenance"] }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/release-on-main.yml around lines 8 - 9, The workflow currently only grants contents: write; to enable npm provenance add the id-token: write permission in the permissions block (i.e., include id-token: write alongside contents: write) and, if enabling provenance, update your release tooling configuration (.release-it.json) to set npm.publish to true and include the publishArgs ["--provenance"] (or set provenance: true where applicable) so the GitHub Actions id token can be used for provenance-enabled npm publishing.scripts/release.sh (1)
78-81: Redundant changelog validation.This check is redundant since
TARGET_VERSIONon line 65 was extracted from the same pattern. If the grep on line 65 succeeded, this check will always pass. However, it's harmless defensive coding.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/release.sh` around lines 78 - 81, The redundant changelog validation duplicates the earlier grep that extracted TARGET_VERSION; remove the repeated block that checks for "## [v$TARGET_VERSION]" (the if ! grep -Fq ... CHANGELOG.md; then ... fi) from scripts/release.sh to avoid unnecessary duplication, or if you want to keep it as defensive coding replace it with a short comment explaining why it's retained; locate the check referencing TARGET_VERSION and grep -Fq and delete or comment it accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.github/workflows/release-on-main.yml:
- Around line 8-9: The workflow currently only grants contents: write; to enable
npm provenance add the id-token: write permission in the permissions block
(i.e., include id-token: write alongside contents: write) and, if enabling
provenance, update your release tooling configuration (.release-it.json) to set
npm.publish to true and include the publishArgs ["--provenance"] (or set
provenance: true where applicable) so the GitHub Actions id token can be used
for provenance-enabled npm publishing.
In `@scripts/release.sh`:
- Around line 78-81: The redundant changelog validation duplicates the earlier
grep that extracted TARGET_VERSION; remove the repeated block that checks for
"## [v$TARGET_VERSION]" (the if ! grep -Fq ... CHANGELOG.md; then ... fi) from
scripts/release.sh to avoid unnecessary duplication, or if you want to keep it
as defensive coding replace it with a short comment explaining why it's
retained; locate the check referencing TARGET_VERSION and grep -Fq and delete or
comment it accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: cf4dca74-675f-40fa-9aee-bffe79574abd
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (6)
.github/workflows/release-on-main.yml.release-it.jsonREADME.mddocs/releasing.mdpackage.jsonscripts/release.sh
🚧 Files skipped from review as they are similar to previous changes (1)
- package.json
1 participant
Summary
CHANGELOG.mdwith initialv1.0.0release notesdocs/releasing.mdscripts/release.shpublish script with dry-run supportreleaseandrelease:dry-runValidation
npm testnpm run build./scripts/release.sh --helpNote
Low Risk
Low risk: adds release documentation and a maintainer-facing publish script; no runtime/library logic changes aside from new npm scripts.
Overview
Adds a top-level
CHANGELOG.md(with initialv1.0.0notes) and a maintainer release guide atdocs/releasing.md.Introduces
scripts/release.shand wires it intopackage.jsonasnpm run release/release:dry-run, enforcing cleanmain, matching changelog/version, running tests/build (unless skipped), publishing to npm (optional dist-tag / dry-run), and tagging/pushingv<version>; README now links to the new release docs.Written by Cursor Bugbot for commit 5860cc8. Configure here.
Summary by CodeRabbit