Skip to content

fix(npm): honor install_before without day drift (#9156)#9157

Merged
jdx merged 5 commits intojdx:mainfrom
risu729:fix/install-before-drift-9156
Apr 17, 2026
Merged

fix(npm): honor install_before without day drift (#9156)#9157
jdx merged 5 commits intojdx:mainfrom
risu729:fix/install-before-drift-9156

Conversation

@risu729
Copy link
Copy Markdown
Contributor

@risu729 risu729 commented Apr 17, 2026

Summary

Fixes the off-by-one-day issue described in discussion #9156, where install_before = "3d" (or an equivalent CLI flag / per-tool option) caused npm to reject versions mise had just resolved.

Root cause

install_before was resolved to an absolute cutoff once during version resolution, but the npm backend then called Timestamp::now() a second time when constructing --min-release-age. The small drift between the two now()s meant 3d became 3d + 30s, which ceil-divided to --min-release-age=4, so npm refused the version mise chose with 3d.

Fix

  • Stable per-process "now". Introduce duration::process_now() so relative durations like "3d" resolve to the same Timestamp for the entire lifetime of a mise invocation. parse_into_timestamp and all npm backend call sites now use it instead of Timestamp::now().
  • Single precedence path. effective_before_date now takes Option<&ToolRequest> so cli::install_into and the toolset installer share one code path (CLI flag > per-tool option > global install_before).
  • Drift tolerance in the --min-release-age branch only. Subtract 60s from the elapsed seconds before ceil-dividing to days, clamped at a minimum of 1 day. Bun and pnpm already emit the cutoff in finer units (seconds / minutes), so they keep using the unadjusted elapsed seconds. Sub-day windows (e.g. install_before = "1h") still fall back to --before using the exact timestamp.

Behavior examples (npm, --min-release-age supported)

install_before elapsed at install time before fix after fix
3d 3d + 30s (drift) --min-release-age=4 (bug) --min-release-age=3
3d 3d + 2min (beyond tolerance) --min-release-age=4 --min-release-age=4
1d 1d + 5s --min-release-age=1 (falls back to --before in old code near the boundary) --min-release-age=1
1h anything < 24h --before <ts> --before <ts> (unchanged)

Test plan

  • cargo test --bin mise build_transitive_release_age (new regression tests for drift, past-tolerance rounding, 1d boundary, plus existing npm/bun/pnpm cases)
  • cargo test --bin mise duration (new tests for process_now() stability and parse_into_timestamp relative/absolute parsing)
  • cargo test --bin mise effective_before_date (covers CLI > per-tool > global precedence and the new Option<&ToolRequest> signature)
  • cargo clippy --bin mise --tests -- -D warnings
  • cargo fmt --check

Closes #9156.

Made with Cursor

@risu729 risu729 marked this pull request as draft April 17, 2026 05:15
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a stable 'now' timestamp via process_now() to ensure that relative durations, such as '3d', resolve to the same absolute time throughout a process's lifetime. It also implements a 60-second tolerance in the npm backend when calculating release age arguments to prevent minor execution delays from causing durations to round up unexpectedly. Additionally, the logic for determining the effective 'before' date has been centralized and refined. I have no feedback to provide as there were no review comments to evaluate.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Apr 17, 2026

Greptile Summary

This PR fixes the off-by-one-day bug in npm's --min-release-age handling (#9156) via two complementary mechanisms: a process_now() singleton that pins "now" for the entire mise invocation so relative durations like \"3d\" resolve identically at every call site, and a 60-second tolerance subtraction in the npm day-rounding path that absorbs any residual drift when an absolute before_date sits just past a day boundary. The effective_before_date function is consolidated to accept Option<&ToolRequest>, unifying the CLI and toolset installer code paths.

Confidence Score: 5/5

This PR is safe to merge — the fix is logically sound, well-tested, and confined to the npm install path.

No P0/P1 issues found. The two-pronged approach (process_now() eliminates drift for relative durations; 60s tolerance absorbs residual rounding for absolute timestamps near a day boundary) is correct, and the regression tests directly verify the before/after behavior described in the PR table. Refactoring of effective_before_date is a clean mechanical change with full test coverage.

No files require special attention.

Important Files Changed

Filename Overview
src/duration.rs Adds process_now() (OnceLock-based stable timestamp) and elapsed_seconds_ceil() (extracted from npm.rs); both are well-tested and correctly scoped.
src/backend/npm.rs Refactors install_version_ to compute install_before_args once before the match, splits into per-PM helpers, adds 60s tolerance for npm day-rounding, and adds targeted regression tests — logic is correct across all tested boundaries.
src/toolset/tool_request.rs Changes effective_before_date to accept Option<&ToolRequest>, unifying the precedence chain; all callers updated and new tests cover the None request and stability invariants.
src/cli/install_into.rs Simplified: old manual None-vs-Some dispatch replaced by the single effective_before_date(self.tool.tvr.as_ref(), ...) call; semantically equivalent.
src/toolset/toolset_install.rs One-line mechanical update to pass Some(tr) to effective_before_date; no logic change.

Sequence Diagram

sequenceDiagram
    participant User as mise invocation
    participant Duration as duration::process_now()
    participant EBD as effective_before_date()
    participant NpmBackend as NPMBackend
    participant Npm as npm CLI

    User->>Duration: first call initialises OnceLock<Timestamp>
    Note over Duration: T₀ = Timestamp::now() (frozen for process lifetime)

    User->>EBD: effective_before_date(request, opts)
    EBD->>Duration: parse_into_timestamp("3d") → T₀ - 3d
    EBD-->>User: before_date = T₀ - 3d

    User->>NpmBackend: install_version_(ctx { before_date })
    NpmBackend->>Duration: elapsed_seconds_ceil(before_date, process_now())
    Note over Duration: process_now() returns same T₀ → elapsed = exactly 3d
    Duration-->>NpmBackend: seconds = 259200

    NpmBackend->>NpmBackend: build_npm_release_age_args(before_date, 259200, true)
    Note over NpmBackend: saturating_sub(60) → 259140<br/>div_ceil(86400) → 3<br/>max(1) → 3
    NpmBackend->>Npm: npm install --min-release-age=3
Loading

Reviews (5): Last reviewed commit: "chore(npm,duration): trim redundant comm..." | Re-trigger Greptile

Comment thread src/backend/npm.rs
@risu729 risu729 marked this pull request as ready for review April 17, 2026 08:32
risu729 added 5 commits April 17, 2026 19:45
Resolve `install_before` once per process and tolerate small amounts of
elapsed time when converting the absolute cutoff back into npm's
day-granular `--min-release-age` flag.

Previously the cutoff was resolved when picking a version, then re-read
via `Timestamp::now()` a few seconds/minutes later when invoking npm.
The extra drift pushed e.g. `install_before = "3d"` to `3d + 30s`, which
ceil-divided to `--min-release-age=4` and caused npm to reject the
version mise had just resolved.

- Introduce `duration::process_now()` so relative durations like "3d"
  resolve to a stable timestamp for the lifetime of a mise invocation.
- Centralize precedence (CLI > per-tool > global) in
  `effective_before_date`, accepting an optional `ToolRequest` so
  `install_into` and the toolset installer share one code path.
- Apply a 60s tolerance in the npm `--min-release-age` branch only, so
  brief drift doesn't round `3d` up to `4d`. Bun/pnpm already emit the
  cutoff in finer units and keep using the unadjusted elapsed seconds.

Made-with: Cursor
Move `npm_supports_min_release_age_flag` into
`build_transitive_release_age_args` and drop the now-redundant bool arg
from its signature. Merge the `--before` branches that fire for older
npm (no `--min-release-age`) and for sub-day windows (the flag is
day-granular), since the CLI output is identical. As a bonus, the
version probe is skipped entirely when the cutoff is <24h because the
result is always `--before` regardless of npm version.

Pure per-package-manager helpers (`build_npm_release_age_args`,
`build_bun_release_age_args`, `build_pnpm_release_age_args`) keep the
branching logic easy to unit-test without needing a Config/ToolSet.

Made-with: Cursor
`process_now()` is the only value ever passed in, so just call it
directly inside `build_transitive_release_age_args`. The inner pure
helpers still take `seconds` so unit tests can exercise precise drift
boundaries without touching the process-wide clock.

Made-with: Cursor
The helper isn't npm-specific — it's a generic Timestamp delta
computation — so move it next to the other `Timestamp` helpers in
`duration`. Add unit tests covering the sub-second rounding, exact
boundary, and not-elapsed clamping semantics.

Made-with: Cursor
Drop comments that just restated test names or assertions, and shorten
the tolerance / process_now blurbs so they don't duplicate the doc
comments on their respective symbols.

Made-with: Cursor
@risu729 risu729 force-pushed the fix/install-before-drift-9156 branch from 4e7a81a to dccc0e9 Compare April 17, 2026 09:58
@jdx jdx merged commit 10a1254 into jdx:main Apr 17, 2026
34 checks passed
@risu729 risu729 deleted the fix/install-before-drift-9156 branch April 17, 2026 18:48
jdx pushed a commit that referenced this pull request Apr 18, 2026
### 🐛 Bug Fixes

- **(backend)** respect install_before in latest lookup by @risu729 in
[#9193](#9193)
- **(backend)** route explicit latest through stable lookup by @risu729
in [#9228](#9228)
- **(backends)** deprecate b shorthand by @risu729 in
[#9234](#9234)
- **(config)** warn for deprecated env keys by @risu729 in
[#9205](#9205)
- **(config)** treat enable_tools empty as disable-all by @risu729 in
[#9108](#9108)
- **(github)** avoid auth on release asset downloads by @risu729 in
[#9060](#9060)
- **(gitlab)** warn when glab OAuth2 token is expired by @stanhu in
[#9195](#9195)
- **(npm)** honor install_before without day drift by @risu729 in
[#9157](#9157)
- **(npm)** warn on old bun and pnpm for install_before by @risu729 in
[#9232](#9232)
- **(pipx)** honor install_before for uv and pipx installs by @risu729
in [#9190](#9190)
- **(registry)** allow shfmt on Windows by @zeitlinger in
[#9191](#9191)

### 🚜 Refactor

- **(backend)** remove unused rolling release helper by @risu729 in
[#9175](#9175)
- **(backend)** use file util for removals by @risu729 in
[#9206](#9206)

### 📚 Documentation

- **(config)** clarify always_keep_download behavior by @risu729 in
[#9235](#9235)
- **(configuration)** add rust to idiomatic version files by @jjt in
[#9233](#9233)
- **(contributing)** expand contribution guide introduction by
@marianwolf in [#9208](#9208)
- **(github)** document multiple release assets workaround by @risu729
in [#9236](#9236)

### 📦️ Dependency Updates

- update actions/setup-node action to v6 by @renovate[bot] in
[#9183](#9183)
- update dependency @types/node to v25 by @renovate[bot] in
[#9187](#9187)
- update crazy-max/ghaction-import-gpg action to v7 by @renovate[bot] in
[#9186](#9186)
- update actions/cache action to v5 by @renovate[bot] in
[#9181](#9181)
- update amannn/action-semantic-pull-request action to v6 by
@renovate[bot] in [#9184](#9184)
- update apple-actions/import-codesign-certs action to v6 by
@renovate[bot] in [#9185](#9185)
- update dependency eslint to v10 by @renovate[bot] in
[#9200](#9200)
- update dependency toml to v4 by @renovate[bot] in
[#9201](#9201)
- update rust crate reqwest to 0.13 by @renovate[bot] in
[#9171](#9171)
- update ghcr.io/jdx/mise:deb docker digest to 523d826 by @renovate[bot]
in [#9198](#9198)
- update ghcr.io/jdx/mise:alpine docker digest to 05617e0 by
@renovate[bot] in [#9196](#9196)
- update ghcr.io/jdx/mise:rpm docker digest to c1992f9 by @renovate[bot]
in [#9199](#9199)
- update ghcr.io/jdx/mise:copr docker digest to 90db6cd by
@renovate[bot] in [#9197](#9197)
- update taiki-e/install-action digest to 58e8625 by @renovate[bot] in
[#9209](#9209)
- update fedora docker tag to v45 by @renovate[bot] in
[#9213](#9213)
- update docker/setup-buildx-action action to v4 by @renovate[bot] in
[#9212](#9212)
- update docker/metadata-action action to v6 by @renovate[bot] in
[#9211](#9211)
- update docker/login-action action to v4 by @renovate[bot] in
[#9210](#9210)
- update dependency typescript to v6 by @renovate[bot] in
[#9202](#9202)
- update docker/build-push-action action to v7 by @renovate[bot] in
[#9203](#9203)
- update github artifact actions (major) by @renovate[bot] in
[#9215](#9215)
- update rust crate duct to v1 by @renovate[bot] in
[#9220](#9220)
- update rust crate demand to v2 by @renovate[bot] in
[#9219](#9219)
- update rust crate clx to v2 by @renovate[bot] in
[#9218](#9218)
- update nick-fields/retry action to v4 by @renovate[bot] in
[#9217](#9217)
- update jdx/mise-action action to v4 by @renovate[bot] in
[#9216](#9216)
- update rust crate self_update to 0.44 by @renovate[bot] in
[#9174](#9174)
- migrate eslint config to flat format for v10 compat by @jdx in
[#9222](#9222)
- update actions/checkout action to v6 by @renovate[bot] in
[#9182](#9182)
- update rust crate toml to v1 by @renovate[bot] in
[#9225](#9225)
- update rust crate versions to v7 by @renovate[bot] in
[#9226](#9226)
- update rust crate which to v8 by @renovate[bot] in
[#9227](#9227)
- update rust crate rmcp to v1 by @renovate[bot] in
[#9221](#9221)

### 📦 Registry

- add sheldon by @3w36zj6 in
[#9104](#9104)
- add pocketbase by @ranfdev in
[#9123](#9123)
- add worktrunk ([aqua:max-sixty/worktrunk,
cargo:worktrunk](https://github.com/max-sixty/worktrunk,
cargo:worktrunk))#1 by @edouardr in
[#8796](#8796)
- add dependency-check
([aqua:dependency-check/DependencyCheck](https://github.com/dependency-check/DependencyCheck))
by @kapitoshka438 in [#9204](#9204)
- add janet by @ranfdev in
[#9241](#9241)

### New Contributors

- @ranfdev made their first contribution in
[#9241](#9241)
- @jjt made their first contribution in
[#9233](#9233)
- @marianwolf made their first contribution in
[#9208](#9208)
- @edouardr made their first contribution in
[#8796](#8796)

## 📦 Aqua Registry Updates

#### New Packages (3)

- [`LargeModGames/spotatui`](https://github.com/LargeModGames/spotatui)
-
[`android-sms-gateway/cli`](https://github.com/android-sms-gateway/cli)
- [`velero-io/velero`](https://github.com/velero-io/velero)

#### Updated Packages (1)

- [`skim-rs/skim`](https://github.com/skim-rs/skim)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants