fix(release): bump package-lock.json alongside package.json - #290
Merged
Conversation
|
Warning Review limit reached
Next review available in: 45 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe change adds a shared release-version utility, tests manifest updates and validation, and uses it in prerelease and promotion workflows to update both package manifests. ChangesRelease version synchronization
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant ReleaseWorkflow
participant setReleaseVersion
participant PackageManifests
ReleaseWorkflow->>setReleaseVersion: invoke with release version and directory
setReleaseVersion->>PackageManifests: update package.json and package-lock.json
PackageManifests-->>setReleaseVersion: persist synchronized versions
setReleaseVersion-->>ReleaseWorkflow: report completion
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/scripts/set-release-version.mjs:
- Around line 39-53: Update the release-version flow around the manifest edits
to parse and validate both package.json and package-lock.json, including
lock.packages[""], before writing either file. Preserve the existing error for
an invalid lockfile, then apply the version updates only after validation
succeeds. Extend the invalid-lockfile test to verify package.json remains at
1.8.0.
- Line 58: Normalize the direct-invocation comparison around
import.meta.filename and argv[1] in the script’s entry-point guard so equivalent
absolute and repository-relative paths match and the version-update flow runs
when invoked from the repository root. Add a CLI test that executes the script
with version 1.9.0 from the repository root and verifies both package.json and
package-lock.json are updated.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: facd82fd-3454-49a2-ab4e-116a2b31f23a
📒 Files selected for processing (4)
.github/scripts/set-release-version.mjs.github/scripts/set-release-version.test.mjs.github/workflows/prerelease.yml.github/workflows/promote.yml
prerelease.yml and promote.yml rewrote the version with a sed over package.json alone, so every release shipped a lockfile whose root version disagreed with the package it locks: v1.7.0 package.json=1.7.0 lock=1.6.0 v1.8.0 package.json=1.8.0 lock=1.8.0-rc.4 v1.9.0 package.json=1.9.0 lock=1.8.0 It went unnoticed for three releases because npm ci only fails on dependency drift, never on this field. The mismatch is inert until someone reads the diff, which is how it finally surfaced. Both workflows now call one script that writes package.json and both root version fields of the lockfile (lockfileVersion 3 repeats it under packages[""]). A plain sed cannot do this: the lockfile has a "version" key per dependency, so a naive substitution would rewrite the whole tree. The files are tab-indented JSON that JSON.stringify round-trips byte for byte, so rewriting them whole still yields a three-line diff. That is load-bearing rather than incidental, and the test pins it: if npm ever changes its lockfile formatting, the test fails instead of a release commit silently becoming a 40k-line reformat. The script refuses to write when packages[""] is absent rather than skipping it through optional chaining, since a silent half-bump is the exact failure being fixed.
EtienneLescot
force-pushed
the
fix/release-bump-lockfile
branch
from
August 8, 2026 21:17
afcd182 to
542573a
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/scripts/set-release-version.mjs:
- Around line 32-36: Update the manifest-writing flow in the release-version
script to make the paired package.json and package-lock.json update atomic:
stage both outputs, then replace them with a rollback or recovery path if either
replacement fails, preserving both original files. Add a test that forces the
second write to fail and verifies both manifests remain byte-for-byte unchanged.
- Around line 29-30: Strengthen validation in setReleaseVersion before either
manifest is updated: reject non-string values, blank or whitespace-only strings,
and malformed release versions using the project’s expected version format.
Preserve valid version handling, and add tests covering numeric, blank,
whitespace-only, and malformed inputs.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 95fae4a8-f55c-4d67-93ed-48b2112da13a
📒 Files selected for processing (3)
.github/scripts/set-release-version.mjs.github/scripts/set-release-version.test.mjs.github/workflows/promote.yml
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/scripts/set-release-version.test.mjs
The script wrote package.json, then validated the lockfile. A lockfile without
packages[""] therefore left package.json bumped and the lockfile untouched —
the exact half-bump this script exists to end, reproduced by its own error path.
Both files are now read and validated up front and written only once every check
has passed.
The existing test claimed to cover this ("throws rather than half-bumping") but
only asserted the throw; it passes unchanged against the buggy script. It now
asserts package.json is still at 1.8.0 and the lockfile never saw 1.9.0, and
fails against the previous implementation with expected '1.9.0' to be '1.8.0'.
Also pins the direct-invocation guard with a CLI test. CodeRabbit read
`import.meta.filename === argv[1]` as always false because the workflows pass a
relative path; Node resolves argv[1] before exposing it, so it holds. Nothing
covered that, and the whole script is dead code if it ever stops being true.
EtienneLescot
force-pushed
the
fix/release-bump-lockfile
branch
from
August 8, 2026 21:23
542573a to
1b4f35b
Compare
Truthiness alone let 123, " " and "not-a-version" reach both manifests; a number writes `"version": 123`, which is not a legal package.json. Narrower than semver on purpose — this gates what may be written into a published manifest, so build metadata or a leading v is a caller bug rather than a version to honour. Defence in depth rather than a live bug: both callers compute the version from an already-validated RC tag. Taken because a script whose purpose is to stop bad version metadata should not be the thing that writes it.
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.
The bug
prerelease.ymlandpromote.ymlrewrote the version with asedoverpackage.jsonalone. Every release therefore shipped a lockfile whose root version disagreed with the package it locks:package.jsonpackage-lock.jsonThree releases, three mismatches. Nothing caught it because
npm cionly fails on dependency drift, never on this field — the mismatch is inert until someone reads the diff, which is exactly how it finally surfaced (CodeRabbit, on the 1.9.0 release-sync PR).The fix
Both workflows now call
.github/scripts/set-release-version.mjs, which writespackage.jsonand both root version fields of the lockfile —lockfileVersion3 repeats the version underpackages[""].A plain
sedcannot do this, which is presumably why it was never extended: the lockfile carries a"version"key for every dependency, so a naive substitution would rewrite the entire tree.Why rewriting whole JSON files is safe here
Both files are tab-indented JSON that
JSON.stringify(…, null, "\t")round-trips byte for byte, so rewriting them whole still produces a three-line diff:That property is load-bearing, not incidental, so the test pins it rather than trusting it. If npm ever changes its lockfile formatting, the test fails — instead of a release commit silently becoming a 40 000-line reformat that nobody reviews.
The script also refuses to write when
packages[""]is missing, rather than skipping it via optional chaining. A silent half-bump is the precise failure being fixed here; turning a format change into a loud error is the point.Verification
Six tests, in
.github/scripts/where the other CI scripts are already covered by vitest (scripts/is outside the vitestinclude, which is why the script lives here):Also run end-to-end against the repo's real
package.jsonandpackage-lock.json, then reverted — exactly three changed lines, nothing else touched. Full.github/scriptssuite: 31 tests passing. Both workflows re-parsed to confirmSetup Node.jsstill precedes the step that now invokes node.One transition note
promote.ymlchecks out the frozen release branch before running the script, so the script must exist on that branch. Branches cut after this merges inherit it frommainviaprerelease.yml. A release branch cut before this merges —release/v1.9.0— does not have it, so promoting such a branch would fail on a missing file. 1.9.0 is already promoted, so nothing is currently affected; if an old branch ever needs promoting, cherry-pick the script onto it first.Summary by CodeRabbit
Bug Fixes
Tests
Chores