Conversation
…to latest `bun info pkg@<spec>` and `bun pm diff pkg@<spec>` looked the spec up as a dist-tag and then, when that missed, as a semver range. The range parser skips words it cannot read, so an unknown dist-tag parsed to an empty group, and an empty group satisfies every version. Both commands printed the data for `latest` and exited 0. npm exits 1 with E404. Add PackageManifest::find_by_spec, which does the dist-tag lookup and only treats the spec as a range when the parser found a comparator in it, and use it in both commands so they take their existing not-found path. The "Recent versions" hint on that path listed the tail of the manifest's raw versions buffer, which ends with the dist-tag targets, so it repeated versions. List the newest release versions instead (prereleases when the package has no release).
|
Warning Review limit reached
On-demand reviews are free for the next 12 days. After that, they cost $0.25 per reviewed file. Or wait 19 seconds for your next included review. View limit detailsLimit details: You’ve used all 10 included reviews currently available. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (5)
Comment |
|
Status: ready for review. Reproduced on bun 1.4.3 against the public registry, in a directory with a package.json: npm exits 1 with The new tests use a local CI (build 112941, finished): 180 of 181 jobs passed. The one red job, |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — focused bugfix with the shared logic extracted and both call sites covered.
What was reviewed:
find_by_specinnpm.rs: dist-tag → range fallback with theis_empty()guard; error type isAllocError(OOM only), propagated via?at both callers as before inpm view.pm_diff_command.rspreviously swallowed the parse error withif let Ok(...); now?-propagates it — checked thatSemver::query::parseonly fails on allocation, so no user-facing behavior change there.- "Recent versions" hint now slices
release_versions()(fallbackprerelease_versions()) and the "... and N more" count usesstart_indexfrom the same slice — no duplicate/off-by-one. - Tests: local
Bun.serve({ port: 0 })registry,describe.concurrent, pipes drained viaPromise.all, stdout/stderr asserted before exit code; the updatedis-number@ 999.0.0snapshot lives in the file's pre-existing real-registry block (noted in the PR).
Extended reasoning...
Overview
This PR fixes bun info / bun pm view and bun pm diff so that a name@ spec where spec is neither a known dist-tag nor a parseable semver range is reported as an error rather than silently resolving to latest. The root cause was that Semver::query::parse on an arbitrary word returns an empty Group, which find_best_version treats as satisfied by every version. The fix adds PackageManifest::find_by_spec in src/install/npm.rs that centralizes the dist-tag → range fallback and returns None when the parsed range is empty. Both pm_view_command.rs and pm_diff_command.rs now call it, replacing ~10 lines of duplicated labeled-block logic each. A secondary fix corrects the "Recent versions:" hint to read from release_versions() (falling back to a new prerelease_versions() accessor) instead of the raw versions slice that included dist-tag-target duplicates.
Security risks
None. This is CLI presentation and version-resolution logic for pm view/pm diff; no auth, crypto, filesystem writes, or network trust decisions are touched. The only input is a user-typed spec string, and the change makes handling stricter (rejecting more inputs), not looser.
Level of scrutiny
Low-to-moderate. The Rust change is small (~30 net lines), extracts a helper following the exact pattern already present at both call sites plus one is_empty() guard that the PR notes is the same check add_catalog.rs and audit_fix.rs already use for the same purpose. The ?-propagated error from Semver::query::parse is AllocError — the pm view path already propagated it, and pm diff previously swallowed it with if let Ok, so the only behavior change there is on OOM. The "Recent versions" rewrite is a straightforward slice of a sorted list with saturating_sub; the ... and N more count now correctly uses start_index from the same slice. No CODEOWNERS entries cover the changed paths.
Other factors
Test coverage is thorough and follows the repo's test conventions closely: a describe.concurrent block backed by a local Bun.serve({ port: 0 }) packument registry, bunExe()/bunEnv, await using proc, Promise.all to drain stdout/stderr/exited, content asserted before exit code, test.each for the info/pm view alias matrix, and inline snapshots for error output. The positive-path test ("known dist-tags and ranges still resolve") guards against over-rejection. The bun-pm-diff.test.ts addition slots into an existing error-case block. The one pre-existing real-registry snapshot update (is-number@ 999.0.0) is acknowledged in the PR notes and sits alongside neighboring tests that already depend on the same registry data. The bug hunt exited on dry_streak with no findings and no ruled-out candidates, and there are no prior reviews or open threads on the PR.
|
Updated 8:50 AM PT - Sep 8th, 2026
❌ @robobun, your commit 21a08b8 has 1 failures in 🧪 To try this PR locally: bunx bun-pr 41992That installs a local version of the PR into your bun-41992 --bun |
|
Closing in favor of #42063. It fixes the same dist-tag fallback in |
1 participant
Problem
bun info pkg@<tag>andbun pm diff pkg@<tag> <v>print the data forlatestand exit 0:bun info is-number@nonexistenttag versionprints7.0.0. npm exits 1 withE404 No match found for version.pm_view_command.rs:200,pm_diff_command.rs:765).Semver::query::parseskips words it cannot read, so the tag parses to an emptyGroup. An emptyGroupsatisfies every version, andfind_best_versionreturnslatest.Fix
PackageManifest::find_by_spec: the dist-tag if there is one, else the range, but only when the parser found a comparator. Both commands call it and take their existing not-found exit, plain and--json.PackageManifest::versions, which ends with the dist-tag targets, so it repeated versions. It now lists the newest five releases (prereleases if there is no release).test/cli/install/bun-info.test.ts(new block, 5 of 6 fail on 1.4.3) andbun-pm-diff.test.ts(new case, fails on 1.4.3). Both files pass.github:/file:test no longer pins the error wording), 2 answered under Notes.Background
dist-tagsmaps names likelatestornextto versions. npm'sviewswaps a known tag for its version, then filters withsemver.satisfies, false for an invalid range.1.0.0 || boopignoresboop.Group::is_empty()reports a parse that found no comparator.PackageManifest::versionsholds releases, then prereleases, then the dist-tag targets.Notes
bun info react@nxt) was the motivating case: it showedlatestwith no hint that the tag does not exist.Group::is_empty()is the same checkadd_catalog.rsandaudit_fix.rsuse to reject an unparseable range.bun installis not affected: it classifies a spec withTag::inferbefore it parses a range, andfind_best_versionitself is unchanged.find_best_versionhad two callers outsidenpm.rs, the two fixed here.bun info pkgagainst a packument whoselatesttag is missing, or points at a version that is not inversions, now reportsNo version of "pkg" satisfying "latest" foundwith the recent versions. Before, it printed the highest release through the same accidental path.bun install pkg@latestalready fails in that case (DistTagNotFound). install: fall back to highest version when dist-tags.latest does not resolve #36671 proposes a fallback for install and leavespm viewout on purpose. If that fallback lands and a maintainer wantspm viewto follow,find_by_specis the one place to add it.github:user/repo,file:./xandnpm:other@1also went down thelatestpath. They now exit 1 through the same not-found message.pm viewnever supported them, and the test only asserts the exit code and anerror:line, so a later dedicated "unsupported spec" message does not break it.is-number@999.0.0snapshot in the pre-existing real-registry block now reads3.0.0 .. 7.0.0, ... and 10 moreinstead of7.0.0twice and11 more. That test already depended on registry.npmjs.org data (as do its neighbours, e.g.versions: 15). is-number last published in 2018.pm_view_command.rsfor a different bug (range buffer). Whichever lands second needs a small rebase. With this PR the range is parsed and matched insidefind_by_specagainstspec, which is also what pm view: match the requested range against the buffer it was parsed from #38671 wants.pm view(several property arguments, array-pluck paths, the two--jsonerror shapes, the package.json requirement inbun info / pm viewrequires a package.json #20673 / install: allow bun info and pm view without a package.json #38151) are separate and not touched here.redacted-config-logs.test.ts(which runspm view) also passes.no test proof · iteration 0 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/cli/install/bun-pm-diff.test.ts