Conversation
…eVersion Bun 1.4 writes lockfileVersion 2 for fresh installs and migrations, which older Bun versions cannot read. Existing v1 lockfiles are preserved on re-save, but there was no way to make a fresh install produce v1 for a project shared with older Bun releases. Add `[install.lockfile] formatVersion = N` in bunfig.toml. The written lockfileVersion never exceeds N (floored to 1, since the writer cannot emit v0 content). An existing v2 lockfile is downgraded on the next re-save. Unset or >= current version is a no-op.
WalkthroughAdds ChangesLockfile version capping
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@docs/pm/lockfile.mdx`:
- Around line 55-58: Correct the lockfile format documentation to state that
re-saving a loaded version 0 lockfile upgrades it to version 1 because the
writer cannot emit v0 workspace entries; only version 1 is preserved on re-save,
while fresh installs and migrations still write version 2.
In `@src/bunfig/bunfig.rs`:
- Around line 1405-1409: Update the formatVersion parsing in the lockfile
configuration flow to accept only finite, non-negative integer numbers before
calling num_to_u32; reject fractional, negative, and non-finite values without
setting install.lockfile_format_version, while preserving handling of valid
integers.
In `@test/cli/install/lockfile-version-2.test.ts`:
- Around line 100-122: Extend the formatVersion test matrix to cover 0 and
assert it produces a v1 lockfile, while retaining the existing v2 and
above-current cases. If the parser rejects invalid numeric values, add
representative invalid variants and assert the corresponding validation
behavior.
- Around line 42-55: Update the install test cases around the spawned process
and lockfile reads to assert the drained subprocess result—especially exitCode
and stderr—before calling file(...).text() for bun.lock. Apply this ordering
consistently to the primary case and the additional lockfile-version cases, so
install failures are reported before artifact-read errors.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8d77c70d-2c41-4bd3-b31b-2a779cad9096
📒 Files selected for processing (7)
docs/pm/lockfile.mdxdocs/runtime/bunfig.mdxsrc/bunfig/bunfig.rssrc/install/PackageManager/PackageManagerOptions.rssrc/install/lockfile/bun.lock.rssrc/options_types/schema.rstest/cli/install/lockfile-version-2.test.ts
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
No bugs found, but this adds a new user-facing bunfig key ([install.lockfile] formatVersion), so a maintainer should sign off on the name/semantics. Also note the three comment-cop inline flags are still open — they look like false positives (the flagged lines are ordinary doc comments, not workaround justifications), but worth a quick look.
What was reviewed:
version_to_writecap logic — traced all (loaded, cap) combinations;target = min(loaded, cap)with the v0→v1 floor preserved, andfrom_intreturningNonefor out-of-range values makes cap≥3 a true no-op.- Plumbing through
api::BunInstall→Options::load→Stringifier::savematches the existingsave_text_lockfilepattern. - New tests are hermetic (
file:deps only), drain pipes concurrently, and cover fresh-install cap, v2→v1 downgrade, and cap-at/above-current no-ops.
Extended reasoning...
Overview
Adds [install.lockfile] formatVersion = N to bunfig, plumbed through api::BunInstall.lockfile_format_version: Option<u32> → Options.lockfile_format_version: Option<Version> → a new cap parameter on Stringifier::version_to_write. The cap is applied as min(loaded, cap) before the existing v0→v1 floor and the v2 content-safety checks. Docs added to docs/pm/lockfile.mdx and docs/runtime/bunfig.mdx; four new tests in test/cli/install/lockfile-version-2.test.ts.
Security risks
None identified. This only lowers the version stamp written to bun.lock; the v2 parse-time security checks (off-registry integrity, unsafe git .bun-tag) are gated on the reader's parsed version and are unaffected by what the writer stamps. Capping to v1 does not weaken any check that wasn't already bypassable by hand-editing the lockfile version.
Level of scrutiny
Medium. The Rust changes are small and mechanical, and the version_to_write refactor preserves the cap=None behavior byte-for-byte (verified by tracing every branch). But this is new user-facing API surface — the option name, its location under [install.lockfile], and the choice to silently ignore unknown/out-of-range values (rather than error) are design calls that fall under the repo's "API design" review guidance and should get maintainer sign-off.
Other factors
- Three
comment-copinline comments fromgithub-actions[bot]are outstanding. They flag the new doc comments on the added field/param as "paragraph-long workaround justifications", but on inspection they are ordinary 2–3 line doc strings explaining what the field does — likely bot false positives, but still unaddressed. Version::from_int(99) → Nonemeans the comment "Unknown (future) versions cap above CURRENT" is slightly imprecise (it becomes no-cap, not a cap-above-current), though the observable behavior is identical and the test forformatVersion = 99confirms it.- Test coverage is solid: fresh-install cap, v2→v1 downgrade on re-save, and two no-op controls; all use
file:deps so no registry contact.
Address review: - Rename the bunfig key to `lockfileVersion` to match the bun.lock field and npm's `--lockfile-version`. - Treat cap < loaded version as a FORCE_SAVE_LOCKFILE trigger (same pattern as changed_config_version), so adding the bunfig option alone downgrades an existing v2 lockfile on the next install. - Trim doc comments flagged by the comment linter. - Correct docs/pm/lockfile.mdx: v0 is floored to v1, not preserved. - Add test coverage for cap=0 (floors to v1) and cap=2 on a loaded v1 lockfile (stays v1, cap is a ceiling not a floor).
robobun
changed the title
A V0 cap on a loaded V1 lockfile would otherwise trigger FORCE_SAVE_LOCKFILE on every install while version_to_write floors the output back to V1, so the re-save writes byte-identical content. Flooring the stored cap keeps the force-save trigger and the writer consistent.
There was a problem hiding this comment.
Implementation looks correct after two review passes (both prior findings addressed in b8ba35a and c497917), but this adds a new documented bunfig key — worth a maintainer glance at the API shape (key name, silent no-op on out-of-range values, ceiling-not-floor semantics).
What was reviewed:
- Traced
version_to_writewith cap = None/V1/V2 against loaded = V0/V1/V2/CURRENT — behavior-preserving when unset, floors and ceilings correct. - Confirmed
from_int(n.max(1))floors 0→V1 and returns None for >2, so the force-save trigger and writer agree and out-of-range is a no-op. - Checked the new force-save arm is gated behind
do_.save_lockfile()via the existingshould_save_lockfilechain, so--frozen-lockfileis unaffected.
Extended reasoning...
Overview
Adds [install.lockfile] lockfileVersion to bunfig, threaded through schema.rs → bunfig.rs → PackageManagerOptions → install_with_manager (new FORCE_SAVE_LOCKFILE arm) → bun.lock.rs (version_to_write gains a cap parameter). Docs updated in lockfile.mdx and bunfig.mdx. Six new tests in lockfile-version-2.test.ts.
Security risks
None. The option only influences which integer is written to the lockfileVersion field of bun.lock; it does not affect what is installed, fetched, or trusted. Out-of-range values collapse to no-op via from_int returning None.
Level of scrutiny
Medium-high. The production diff is small (~50 lines) and follows the sibling concurrentScripts / changed_config_version patterns exactly, but it is (a) new documented user-facing API surface, and (b) touches the text-lockfile version-selection path that governs cross-Bun-version compatibility of committed lockfiles. REVIEW.md flags API-surface additions as needing maintainer sign-off on shape.
Other factors
Two earlier passes from me caught the V0-cap spurious re-save and a stdout/stderr assertion mixup; both were fixed and now have regression tests. All CodeRabbit and comment-cop threads are resolved. The bug-hunting system found nothing this run. Tests are hermetic (file: deps, loopback servers), fail-before is demonstrated in the PR body, and the = 0 no-resave test now correctly asserts against stderr. The remaining question is purely whether a maintainer is happy with the key name and the lenient-input semantics — not correctness.
|
CI on c497917 is green for this diff. Ready for review. |
This branch has not been deployed
2 participants
What
Adds a
bunfig.tomloption to cap thelockfileVersionwritten tobun.lock:Why
#31539 bumped the default text lockfile to
lockfileVersion: 2for fresh installs and migrations. Bun releases before 1.4 reject version 2 withUnknown lockfile version. #31602 already ensures an existing v1 lockfile is preserved on re-save, and #32465 makes the error actionable going forward, but there is no way to make a fresh install (or a migration from package-lock/yarn/pnpm) produce a lockfile that a teammate on 1.3.x can read.With
lockfileVersion = 1committed alongside the project:bun installwrites"lockfileVersion": 1FORCE_SAVE_LOCKFILEtrigger, same aschanged_config_version)lockfileVersion = 2set (the cap is a ceiling, not a floor)The v1 format is a strict subset of what the v2 writer emits (v1 to v2 only added parse-time checks on identical content), so capping at v1 never loses information. The existing v0 to v1 floor still applies since the writer cannot emit v0-format workspace entries. Values at or above the current version are a no-op.
--frozen-lockfileis unaffected (force_save_lockfileis gated behinddo_.save_lockfile(), which frozen-lockfile clears).The key name matches the bun.lock field and npm's
--lockfile-versionflag.Tests
New tests in
test/cli/install/lockfile-version-2.test.ts:Fail-before on the first two and the
= 0case; the rest are no-op controls. Full file: 15 pass, 0 fail.Also adds a
Lockfile format versionssection todocs/pm/lockfile.mdxand documents the key indocs/runtime/bunfig.mdx.[review] gate passed · iteration 1 · 8 files touched
fails on main (without fix)
passes on PR (with fix)
diff hotspot
gate history · 3 passed · 0 rejected · iteration 1
evidence per changed file