Skip to content

install: gate a 2-day default minimumReleaseAge for new projects behind configVersion 2#31608

Closed
robobun wants to merge 1 commit into
farm/9555d6db/minimum-release-age-default-and-msfrom
farm/9555d6db/minimum-release-age-2d-default
Closed

install: gate a 2-day default minimumReleaseAge for new projects behind configVersion 2#31608
robobun wants to merge 1 commit into
farm/9555d6db/minimum-release-age-default-and-msfrom
farm/9555d6db/minimum-release-age-2d-default

Conversation

@robobun

@robobun robobun commented May 30, 2026

Copy link
Copy Markdown
Collaborator

Stacked on #30529 (ms-style duration strings). Review/merge that one first — this PR's diff is shown against its branch.

What

Adds a gated 2-day default minimumReleaseAge for new projects, rolled out via a new lockfile configVersion:

  • ConfigVersion::V2 carries a 2-day default minimumReleaseAge for projects whose lockfile is at configVersion >= 2 and that haven't set the option explicitly.
  • bun pm migrate and fresh installs write ConfigVersion::CURRENT; existing lockfiles keep their saved version; 0 always disables.

Gating — does not ship until 1.4

  • BREAKING_CHANGES_1_4 (compile-time, currently false) flips ConfigVersion::CURRENT from V1 to V2.
  • Until then, the BUN_FEATURE_FLAG_INSTALL_CONFIG_V2 env var opts fresh projects into V2 at runtime (for testing).
  • ConfigVersion::MAX_KNOWN (= V2) lets this build read V2 lockfiles even while CURRENT is still V1, so a lockfile written under the flag is honored without it.

So the code ships now and is testable, but no user sees a behavior change until 1.4 flips the flag.

How

PackageManager::apply_config_version_defaults() centralizes deriving config_version from the lockfile load result and applying the gated default. It's idempotent and is called from install, bun outdated, bun update --interactive, and bun pm trust so none of them clobber a saved configVersion.

Verification

test/cli/install/minimum-release-age.test.ts — 59 pass / 0 fail on debug+ASAN (54 base/ms + 5 configVersion-2 gated tests). Covers: new projects default to 2 days (flag on), existing configVersion 1 unaffected, minimumReleaseAge = 0 disables it, existing configVersion 2 lockfile applies the default without the flag, and flag-off preserves legacy behavior.

@robobun

robobun commented May 30, 2026

Copy link
Copy Markdown
Collaborator Author
Updated 11:21 AM PT - May 30th, 2026

@robobun, your commit e78489e has 1 failures in Build #59315 (All Failures):


🧪   To try this PR locally:

bunx bun-pr 31608

That installs a local version of the PR into your bun-31608 executable, so you can run:

bun-31608 --bun

@github-actions

Copy link
Copy Markdown
Contributor

This PR may be a duplicate of:

  1. feat: set default minimumReleaseAge to 3 days (259200s) #28729 - Also sets a default minimumReleaseAge for new bun install projects (3 days vs 2 days in this PR), but without configVersion gating

🤖 Generated with Claude Code

@robobun

robobun commented May 30, 2026

Copy link
Copy Markdown
Collaborator Author

Related to #28729, but intentionally a different design rather than a duplicate:

  • feat: set default minimumReleaseAge to 3 days (259200s) #28729 sets a 3-day default unconditionally (ships to everyone immediately).
  • This PR makes it a gated breaking change: the 2-day default only applies to lockfiles at configVersion >= 2, and ConfigVersion::CURRENT stays at V1 behind the BREAKING_CHANGES_1_4 compile-time flag — so existing projects are untouched and nothing activates until Bun 1.4 (with BUN_FEATURE_FLAG_INSTALL_CONFIG_V2 available to opt in for testing meanwhile).

The configVersion gating + feature flag is the point of this approach. Also stacked on #30529 (the ms-style duration-string parsing), which is independent of the default-age question.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't find any bugs, but this introduces a new product-level default (2-day minimumReleaseAge gated behind configVersion 2 / BREAKING_CHANGES_1_4) and threads new unsafe reborrows through several install entry points, so it's worth a human sign-off on both the rollout design and the aliasing invariants.

Extended reasoning...

Overview

This PR adds infrastructure to roll out a 2-day default minimumReleaseAge for new projects via a new lockfile configVersion 2. It touches:

  • ConfigVersion.rs: new V2 variant, CURRENT gated on a new compile-time BREAKING_CHANGES_1_4 const, new MAX_KNOWN for forward-compat clamping.
  • PackageManager.rs: new apply_config_version_defaults() that derives config_version from the load result, optionally bumps fresh projects to V2 under BUN_FEATURE_FLAG_INSTALL_CONFIG_V2, applies the 2-day default, and normalizes Some(0.0)None.
  • install_with_manager.rs, outdated_command.rs, pm_trusted_command.rs, update_interactive_command.rs: each now calls the new helper, with new unsafe { &mut *pm_ptr } reborrows justified by disjoint-field comments.
  • feature_flags.rs / env_var.rs: new BREAKING_CHANGES_1_4 const and BUN_FEATURE_FLAG_INSTALL_CONFIG_V2 env flag.
  • Docs and ~160 lines of new tests; test/harness.ts strips the new env var from bunEnv.

Security risks

The change is security-adjacent (supply-chain hardening) but doesn't itself touch auth, crypto, or permissions. The main risk surface is the new unsafe aliasing in the CLI commands — each is commented as touching only manager.options while load_result borrows manager.lockfile, which matches what apply_config_version_defaults does today, but it's another invariant that future edits to that method must preserve.

Level of scrutiny

This warrants human review because (a) it encodes a product decision — flipping every new Bun 1.4 project to a 2-day install delay by default — that someone should explicitly sign off on, (b) it introduces the BREAKING_CHANGES_1_4 / MAX_KNOWN rollout pattern that future config bumps will follow, and (c) it adds several raw-pointer reborrows in the install/outdated/trust/update-interactive paths. None of these look wrong, but they're not mechanical.

Other factors

  • Gating is solid: BREAKING_CHANGES_1_4 = false keeps CURRENT = V1, so today's user-visible behavior is unchanged except that a hand-written configVersion: 2 lockfile now activates the default (covered by a test).
  • The Some(0.0)None normalization is a small behavior change for explicit minimumReleaseAge = 0 (now skips extended-manifest fetches), which seems strictly an improvement but is worth a glance.
  • Stacked on #30529; tests reportedly pass on debug+ASAN.

@robobun robobun force-pushed the farm/9555d6db/minimum-release-age-2d-default branch from 124238e to a0b4a89 Compare May 30, 2026 10:43
Comment thread src/install/PackageManager/PackageManagerOptions.rs Outdated
@robobun robobun force-pushed the farm/9555d6db/minimum-release-age-default-and-ms branch from fbd6b70 to 1ebfa5f Compare May 30, 2026 11:30
@robobun robobun force-pushed the farm/9555d6db/minimum-release-age-2d-default branch from a0b4a89 to f491d2a Compare May 30, 2026 11:33
Comment thread src/install/PackageManager.rs
Comment thread src/runtime/cli/outdated_command.rs
@robobun robobun force-pushed the farm/9555d6db/minimum-release-age-2d-default branch from f491d2a to 296abe3 Compare May 30, 2026 12:05
@robobun robobun force-pushed the farm/9555d6db/minimum-release-age-default-and-ms branch from 24f52cb to 29c0f0c Compare May 30, 2026 12:15
@robobun robobun force-pushed the farm/9555d6db/minimum-release-age-2d-default branch from 296abe3 to 46d53b6 Compare May 30, 2026 12:18
Comment thread src/install/PackageManager.rs
@robobun

robobun commented May 30, 2026

Copy link
Copy Markdown
Collaborator Author

@alii opened the Bun.ms PR: #31613 (stacked on this one). It mirrors the npm ms package — Bun.ms("2 days") → 172800000, Bun.ms(60000, { long: true }) → "1 minute" — reusing the parse_ms from #30529 and adding a format_ms. Matches ms exactly (months aren't a unit, format only goes up to days, JS round-half-to-+∞). Supersedes the pre-Rust-rewrite attempt #23162; closes #18597.

…nd configVersion 2

Stacked on the ms-style-duration PR. Introduces ConfigVersion::V2, which
carries a 2-day default minimumReleaseAge for projects whose lockfile is
at configVersion >= 2 and that haven't set the option explicitly. This is
how the breaking change rolls out: fresh projects and `bun pm migrate`
write CURRENT, existing lockfiles keep their saved version, and an explicit
`0` always disables (stored as Some(0.0), which suppresses the default).

The new default is gated so it does not ship until Bun 1.4:
- BREAKING_CHANGES_1_4 (compile-time) flips ConfigVersion::CURRENT to V2.
- Until then, BUN_FEATURE_FLAG_INSTALL_CONFIG_V2 opts fresh projects into
  V2 at runtime for testing.
- MAX_KNOWN (= V2) lets this build read V2 lockfiles even while CURRENT is
  still V1, so a lockfile written under the flag is honored without it.

apply_config_version_defaults() centralizes deriving config_version and
applying the gated default; it is idempotent and called from install,
`bun outdated`, `bun update --interactive`, and `bun pm trust` so none of
them clobber a saved configVersion.
@robobun robobun force-pushed the farm/9555d6db/minimum-release-age-2d-default branch from 46d53b6 to e78489e Compare May 30, 2026 13:47
@robobun

robobun commented May 30, 2026

Copy link
Copy Markdown
Collaborator Author

CI status

The one red lane on build 59315 is flake unrelated to this diff: darwin-14-x64-test-bun exited non-zero because a Bun.spawn PTY/terminal test timed out (90s) alongside repeated Cannot connect to the Docker daemon / node-gyp not in PATH agent-infra errors. 285 of 286 jobs passed, including the minimum-release-age tests; nothing touching bun install / ConfigVersion failed.

This PR (the configVersion-gated 2-day minimumReleaseAge default, behind BREAKING_CHANGES_1_4) passes 60/60 on debug+ASAN and builds clean in debug + release. Diff is green; ready for review/merge. Stacked on #30529 (merge first); Bun.ms follow-up is #31613.

@Jarred-Sumner

Copy link
Copy Markdown
Collaborator

npm registry API doesn't support for this for the compact Content-Type. Until then, this will be a significant slowdown and so we are not doing it.

@alii

alii commented Jun 1, 2026

Copy link
Copy Markdown
Member

Sounds good. This was the reason I stacked this into a separate PR

@robobun

robobun commented Jun 1, 2026

Copy link
Copy Markdown
Collaborator Author

Understood — closing. The age filter needs the time field, which only the full manifest carries, so a default-on minimumReleaseAge would force every package to fetch the full registry manifest instead of the compact application/vnd.npm.install-v1+json one. If the npm registry ever adds publish timestamps to the abbreviated manifest, this can be revisited (the configVersion-gating approach here would still apply).

The rest of the stack is unaffected: #30529 (ms-style duration strings for the existing opt-in option — no default change, no extra manifest fetches for anyone who doesn't opt in) and #31613 (Bun.ms, now rebased directly onto #30529).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants