fix(npm): honor install_before without day drift (#9156)#9157
Conversation
There was a problem hiding this comment.
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 SummaryThis PR fixes the off-by-one-day bug in npm's Confidence Score: 5/5This 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
Sequence DiagramsequenceDiagram
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
Reviews (5): Last reviewed commit: "chore(npm,duration): trim redundant comm..." | Re-trigger Greptile |
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
4e7a81a to
dccc0e9
Compare
### 🐛 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)
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_beforewas resolved to an absolute cutoff once during version resolution, but the npm backend then calledTimestamp::now()a second time when constructing--min-release-age. The small drift between the twonow()s meant3dbecame3d + 30s, which ceil-divided to--min-release-age=4, so npm refused the version mise chose with3d.Fix
duration::process_now()so relative durations like"3d"resolve to the sameTimestampfor the entire lifetime of a mise invocation.parse_into_timestampand all npm backend call sites now use it instead ofTimestamp::now().effective_before_datenow takesOption<&ToolRequest>socli::install_intoand the toolset installer share one code path (CLI flag > per-tool option > globalinstall_before).--min-release-agebranch 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--beforeusing the exact timestamp.Behavior examples (npm,
--min-release-agesupported)install_before3d3d + 30s(drift)--min-release-age=4(bug)--min-release-age=33d3d + 2min(beyond tolerance)--min-release-age=4--min-release-age=41d1d + 5s--min-release-age=1(falls back to--beforein old code near the boundary)--min-release-age=11h< 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 forprocess_now()stability andparse_into_timestamprelative/absolute parsing)cargo test --bin mise effective_before_date(covers CLI > per-tool > global precedence and the newOption<&ToolRequest>signature)cargo clippy --bin mise --tests -- -D warningscargo fmt --checkCloses #9156.
Made with Cursor