Skip to content

feat(pacquet): resolver scaffold + npm version picking#11755

Merged
zkochan merged 12 commits into
mainfrom
resolution
May 20, 2026
Merged

feat(pacquet): resolver scaffold + npm version picking#11755
zkochan merged 12 commits into
mainfrom
resolution

Conversation

@zkochan

@zkochan zkochan commented May 19, 2026

Copy link
Copy Markdown
Member

Summary

Two commits that together cover the Stage 2 resolver seam and the npm leg's version-picking surface. First piece of Stage 2 — Tier 1 foundations.


Commit 1 — feat(pacquet): scaffold Stage 2 resolver seam

Lays down the seam future per-protocol resolvers (npm, git, tarball, local, jsr, runtimes, named-registry, workspace) plug into.

New crates:

  • pacquet-resolving-parse-wanted-dependency — ports pnpm's @pnpm/resolving.parse-wanted-dependency with an inline port of validate-npm-package-name@7's validForOldPackages branch. Splits raw manifest specs like foo@1.2.3, @scope/foo@^1, and the npm-alias form foo@npm:lodash@^4 into (alias, bareSpecifier) halves; passes git/tarball/range strings through unchanged. Behavior verified byte-for-byte against the upstream JS function for the test corpus.

  • pacquet-resolving-default-resolver — ports the protocol-routing dispatcher at the top of resolving/default-resolver/src/index.ts. DefaultResolver walks a Vec<Box<dyn Resolver>> chain in declaration order and returns the first resolver's Ok(Some(_)) claim; falls back to SpecNotSupportedByAnyResolverError (upstream's SPEC_NOT_SUPPORTED_BY_ANY_RESOLVER). Chain is empty until the per-protocol resolver PRs land.

Extended crate:

  • pacquet-resolving-resolver-base — previously verifier-only. Adds the dispatcher contract: WantedDependency, ResolveOptions, ResolveResult, LatestQuery, LatestInfo, PreferredVersions + selector types, WorkspacePackage(s), weight constants (DIRECT_DEP_SELECTOR_WEIGHT, EXISTING_VERSION_SELECTOR_WEIGHT), and the Resolver trait with boxed-future return types matching the existing ResolutionVerifier shape. Verifier types moved into their own submodule with no public-surface change.

Commit 2 — feat(resolving-npm-resolver): port pickPackage / pickPackageFromMeta

Ports pnpm's pickPackageFromMeta.ts and pickPackage.ts — the pure version-picking surface and the cache+fetch orchestration that wraps it. This is the piece that lets the chain's npm leg resolve a registry spec to a concrete (name, version, manifest) triple.

Two new modules under crates/resolving-npm-resolver/src/:

  • pick_package_from_meta — pure picker. pickPackageFromMeta, pickVersionByVersionRange (with deprecated-version fallback), pickLowestVersionByVersionRange, filterPkgMetadataByPublishDate (the minimumReleaseAge filter), PreferredVersionsPrioritizer's group-by-weight logic, and the error codes upstream surfaces (ERR_PNPM_UNPUBLISHED_PKG, ERR_PNPM_NO_VERSIONS, ERR_PNPM_MISSING_TIME). Introduces RegistryPackageSpec / RegistryPackageSpecType as the picker's input shape.

  • pick_package — cache+fetch orchestration. All four pre-fetch layers (in-memory PackageMetaCache, offline / preferOffline / pickLowestVersion disk read, version-spec fast path, publishedBy mtime shortcut), 304-aware fetch via fetch_full_metadata_cached, the include_latest_tag runner that picks max(spec, latest), the publishedBy-active fall-back-to-lowest path (pickRespectingMinReleaseAge), and the warn-and-skip ignore_missing_time_field path. Exposes PackageMetaCache as a trait with an InMemoryPackageMetaCache default impl plus a shared_in_memory_cache() factory.

Supporting changes to pacquet-registry:

  • PackageVersion gained deprecated: Option<String> (the deprecated-fallback in pickVersionByVersionRange needs it).
  • Package gained dist_tag(tag) / dist_tags() accessors, and dist_tags is now pub so the publishedBy filter can rebuild a packument with rewritten tags.

Scope notes

  • Chain is still empty after these two commits. Each per-protocol resolver is a separate Tier 2 item in Rust Roadmap #11633 and wires into the chain as it lands; the picker added here will plug into the npm leg in that follow-up.
  • DependencyManifest is a serde_json::Value alias for now. Swap once a typed in-memory manifest lands.
  • ResolveError = Box<dyn Error + Send + Sync> today so each resolver crate can keep its own typed error enum without forcing a shared enum prematurely.

Documented divergences in pick_package

  • Pacquet's metadata fetcher returns full metadata only; the abbreviated→full upgrade branches sit in place but never fire today. Adding an abbreviated fetcher later is a drop-in (Accept-header swap + meta-dir swap).
  • dry_run skips the in-memory cache write, but the underlying fetch_full_metadata_cached still warms the on-disk mirror. Restoring upstream's "no disk side effects on dry-run" needs the fetcher to grow a write-bypass flag.
  • Upstream's p-limit(1) per-mirror serialization is omitted; the atomic rename in save_meta covers write safety.

Test plan

  • cargo check -p pacquet-resolving-parse-wanted-dependency / -p pacquet-resolving-resolver-base / -p pacquet-resolving-default-resolver / -p pacquet-resolving-npm-resolver
  • cargo nextest run -p pacquet-resolving-npm-resolver — 105 tests pass (includes 21 new picker tests + 30 new orchestration tests, mockito-backed)
  • just check, just lint, just test — 1547 tests across the workspace, all pass
  • cargo fmt --check, taplo format --check — clean
  • Behavior parity for parse_wanted_dependency verified by running the upstream JS function on the same input corpus and confirming output matches byte-for-byte.
  • Per-protocol resolvers wire into the chain in follow-up PRs.

Part of #11633.


Written by an agent (Claude Code, claude-opus-4-7).

Summary by CodeRabbit

  • New Features

    • Per-version deprecation metadata exposed and usable via dist-tag lookups and iteration
    • Improved version selection: highest/lowest range picks, deprecated-version fallback, preferred selectors, and publish-date maturity filtering
    • Dependency-string parser extracting alias vs. bare specifier
    • Resolver chain with a default resolver, shared in-memory cache, optional on-disk mirror, offline/dry-run behaviors
  • Bug Fixes

    • Inclusive boundary for minimumReleaseAge (publishedBy) at the cutoff
  • Tests

    • New regression/unit tests for parsing, deprecation normalization, and version-picking
  • Chores

    • Workspace manifests updated; new resolver-related crates added

Review Change Stack

@coderabbitai

coderabbitai Bot commented May 19, 2026

Copy link
Copy Markdown

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: e00d666d-0d29-451a-8b73-5f42763f6029

📥 Commits

Reviewing files that changed from the base of the PR and between 0eaa86d and 52bf1e2.

📒 Files selected for processing (3)
  • pacquet/crates/registry/src/package_version.rs
  • pacquet/crates/resolving-npm-resolver/src/pick_package_from_meta.rs
  • pacquet/crates/resolving-resolver-base/src/resolve.rs
📜 Recent review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
  • GitHub Check: Run benchmark on ubuntu-latest
  • GitHub Check: Run benchmark on ubuntu-latest
  • GitHub Check: Lint and Test (windows-latest)
  • GitHub Check: Lint and Test (macos-latest)
  • GitHub Check: Code Coverage
🧰 Additional context used
📓 Path-based instructions (1)
pacquet/**/*.rs

📄 CodeRabbit inference engine (pacquet/AGENTS.md)

pacquet/**/*.rs: When porting a function that fires pnpm:<channel> events through globalLogger, logger.debug(), or streamParser.write(), mirror the call site, payload, and ordering so the reporter parses pacquet's NDJSON the same way it parses pnpm's.
Declare a newtype wrapper for branded string types. Do not collapse the brand into a plain String or &str.
If upstream always validates before construction, validate in pacquet's wrapper too. The wrapper must construct only via TryFrom<String> and/or FromStr. Do not provide an infallible public constructor.
If upstream never validates, just brand for type-safety. Expose an infallible From<String> (and From<&str> when convenient).
If upstream occasionally constructs without validation, expose from_str_unchecked as an escape hatch alongside the validating constructor.
Match upstream serde behavior for branded types that cross JSON, YAML, or INI boundaries. Use #[serde(try_from = "String")] for deserialization and #[serde(into = "String")] for serialization.
Use #[derive(derive_more::From)] and #[derive(derive_more::Into)] for mechanical conversion impls. Fall back to manual impl only when conversion needs custom logic.
String-literal unions should become enums, not newtype wrappers. Model closed sets of valid string values as enums.
Template literal types should be treated as branded strings with validation discipline from rules 2-5.
Choose owned vs. borrowed parameters to minimize copies. Widen to the most encompassing type (&Path over &PathBuf, &str over &String) when it doesn't force extra copies.
Prefer Arc::clone(&x) / Rc::clone(&x) over x.clone() for reference-counted types, so the cost is visible at the call site.
Follow Rust API Guidelines for naming conventions.
Do not use star imports inside module bodies. Write use super::{Foo, bar} instead of use super::*;. Two forms stay allowed: external-crate preludes like use rayon::prelude::*; and root-of-module re-...

Files:

  • pacquet/crates/registry/src/package_version.rs
  • pacquet/crates/resolving-npm-resolver/src/pick_package_from_meta.rs
  • pacquet/crates/resolving-resolver-base/src/resolve.rs
🔇 Additional comments (3)
pacquet/crates/registry/src/package_version.rs (1)

38-106: LGTM!

pacquet/crates/resolving-npm-resolver/src/pick_package_from_meta.rs (1)

552-563: LGTM!

pacquet/crates/resolving-resolver-base/src/resolve.rs (1)

22-34: LGTM!


📝 Walkthrough

Walkthrough

Adds resolver-base two-seam contracts and verifier types, a DefaultResolver chain, a wanted-dependency parser, registry metadata deprecation support, and npm package picking: an orchestration over in-memory cache, optional disk mirror, and fetcher plus pure packument pickers and comprehensive tests. Also adjusts publishedBy cutoff boundary to be inclusive and wires workspace crate deps.

Changes

Registry metadata & tests

Layer / File(s) Summary
PackageVersion deprecated field & serde normalizer
pacquet/crates/registry/src/package_version.rs
Adds deprecated: Option<String> with custom serde deserializer mapping boolean/string/null into the Option and skipping serialization when None.
Package dist-tags visibility & accessors
pacquet/crates/registry/src/package.rs
Exposes pub dist_tags and adds dist_tag(tag) and dist_tags() accessors.
Registry tests updated / regressions
pacquet/crates/registry/src/package/tests.rs, pacquet/crates/package-manager/src/build_snapshot/tests.rs
Test fixtures updated to include deprecated: None and new deserialization tests for deprecated boolean/string forms.

Resolver-base, DefaultResolver, and wanted-dependency parsing

Layer / File(s) Summary
Resolver-base two-seam contracts
pacquet/crates/resolving-resolver-base/src/lib.rs, .../src/resolve.rs, .../src/verifier.rs, pacquet/crates/resolving-resolver-base/Cargo.toml
Refactors into resolve and verifier modules; adds WantedDependency, Resolver trait, ResolveResult/LatestInfo models, verifier types/trait, selector-weight constants, and workspace serde dep.
Default resolver crate
pacquet/crates/resolving-default-resolver/*
New crate implementing DefaultResolver which walks a chain of Resolver trait objects and returns the first claimant; includes diagnostic error formatting and tests.
Parse wanted-dependency crate
pacquet/crates/resolving-parse-wanted-dependency/*
New crate parsing pnpm-style wanted-dependency strings into (alias, bare_specifier), re-exporting npm-name validation and including validation tests.

NPM package picking and version selection

Layer / File(s) Summary
Crate exports & workspace deps
pacquet/crates/resolving-npm-resolver/src/lib.rs, pacquet/crates/resolving-npm-resolver/Cargo.toml, Cargo.toml
Adds pick_package and pick_package_from_meta modules, re-exports their APIs, and updates workspace dependencies (dashmap, indexmap, new resolver crates).
Package picking orchestration
pacquet/crates/resolving-npm-resolver/src/pick_package.rs
pick_package orchestrator with PackageMetaCache trait and InMemoryPackageMetaCache, in-memory lookup, disk-mirror shortcuts, offline/prefer-offline behavior, pinned-version fast-path, conditional fetch with on-disk fallback, maturity-aware pick flow, bounded missing-time warnings, mirror persistence, and shared cache helper.
Pure version picker from packument
pacquet/crates/resolving-npm-resolver/src/pick_package_from_meta.rs
pick_package_from_meta selects versions from an existing packument: pins, dist-tags, semver ranges with preferred-selector weighting, deprecated fallback retry, lowest/highest pickers, and filter_pkg_metadata_by_publish_date.
Package picking test suites
pacquet/crates/resolving-npm-resolver/src/pick_package/tests.rs, pacquet/crates/resolving-npm-resolver/src/pick_package_from_meta/tests.rs
Async and unit tests covering cold/warm cache, offline/mirror behavior, pinned fast-paths and fallbacks, dry_run, pick_lowest, cache isolation by registry, invalid-name validation, publishedBy filtering, dist-tag rewriting, and many edge cases.

Workspace manifest and JS publishedBy boundary

Layer / File(s) Summary
publishedBy boundary & workspace wiring
.changeset/minimum-release-age-modified-shortcut-inclusive.md, Cargo.toml, resolving/npm-resolver/src/pickPackage.ts, resolving/npm-resolver/src/pickPackageFromMeta.ts, resolving/npm-resolver/test/publishedBy.test.ts
Changes the publishedBy/minimumReleaseAge shortcut to treat modified == publishedBy as eligible for the abbreviated fast path; updates TS logic and adds a Jest boundary test.

Sequence Diagrams

sequenceDiagram
  participant Client
  participant DefaultResolver as DefaultResolver
  participant ResolverA as ResolverA
  participant ResolverB as ResolverB
  Client->>DefaultResolver: resolve(wanted_dependency)
  DefaultResolver->>ResolverA: resolve(wanted_dependency, opts)
  ResolverA-->>DefaultResolver: Ok(None)
  DefaultResolver->>ResolverB: resolve(wanted_dependency, opts)
  ResolverB-->>DefaultResolver: Ok(Some(ResolveResult))
  DefaultResolver-->>Client: Ok(ResolveResult)
Loading
sequenceDiagram
  participant PickPackage as pick_package
  participant Cache as InMemoryCache
  participant Mirror as OnDiskMirror
  participant Fetcher as Fetcher
  participant Picker as pick_package_from_meta
  PickPackage->>Cache: get(key)
  alt cache hit
    Cache-->>PickPackage: meta
  else offline/prefer_offline read
    PickPackage->>Mirror: read(packument)
    Mirror-->>PickPackage: meta or error
  else fetch path
    PickPackage->>Fetcher: fetch_full_metadata_cached
    Fetcher-->>PickPackage: meta
    PickPackage->>Cache: set(key, meta)
  end
  PickPackage->>Picker: pick_matching_version(meta, opts)
  Picker-->>PickPackage: picked_version
  PickPackage-->>Client: PickPackageResult
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related issues

Possibly related PRs

  • pnpm/pnpm#11622: Also modifies publishedBy/minimumReleaseAge boundary logic in pickPackage.
  • pnpm/pnpm#11678: Related package-manager/test adjustments and directory fetcher changes that interact with snapshot behavior.

Poem

🐰 I hop through crates and chain each resolver,
I split the wanted string with a twitch and a noser,
Caches hum, mirrors keep,
Picks choose versions that sleep,
Boundary fixed—now home to my clover!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately captures the main objective: introducing resolver scaffold infrastructure and porting npm version-picking logic into pacquet.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch resolution

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions

github-actions Bot commented May 19, 2026

Copy link
Copy Markdown
Contributor

Micro-Benchmark Results

Linux

group                          main                                   pr
-----                          ----                                   --
tarball/download_dependency    1.00      7.5±0.20ms   577.7 KB/sec    1.05      7.9±0.55ms   549.3 KB/sec

@codecov-commenter

codecov-commenter commented May 19, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 78.24207% with 151 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.64%. Comparing base (3687b0e) to head (52bf1e2).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
.../crates/resolving-npm-resolver/src/pick_package.rs 56.68% 107 Missing ⚠️
...solving-npm-resolver/src/pick_package_from_meta.rs 93.15% 20 Missing ⚠️
pacquet/crates/registry/src/package_version.rs 40.00% 18 Missing ⚠️
pacquet/crates/registry/src/package.rs 50.00% 3 Missing ⚠️
...cquet/crates/resolving-default-resolver/src/lib.rs 93.61% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #11755      +/-   ##
==========================================
- Coverage   90.12%   89.64%   -0.48%     
==========================================
  Files         146      151       +5     
  Lines       16876    17571     +695     
==========================================
+ Hits        15209    15752     +543     
- Misses       1667     1819     +152     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions

github-actions Bot commented May 19, 2026

Copy link
Copy Markdown
Contributor

Integrated-Benchmark Report (Linux)

Scenario: Frozen Lockfile

Command Mean [s] Min [s] Max [s] Relative
pacquet@HEAD 2.375 ± 0.087 2.237 2.484 1.03 ± 0.05
pacquet@main 2.310 ± 0.067 2.250 2.450 1.00
pnpm 4.637 ± 0.044 4.570 4.705 2.01 ± 0.06
BENCHMARK_REPORT.json
{
  "results": [
    {
      "command": "pacquet@HEAD",
      "mean": 2.37524262104,
      "stddev": 0.08683046776834709,
      "median": 2.39692830224,
      "user": 2.6891260000000003,
      "system": 3.6050140399999995,
      "min": 2.23706149474,
      "max": 2.48431587174,
      "times": [
        2.43580794674,
        2.30536122974,
        2.44334405074,
        2.28453341774,
        2.39498566274,
        2.48431587174,
        2.4687581337399997,
        2.23706149474,
        2.39887094174,
        2.2993874607399998
      ]
    },
    {
      "command": "pacquet@main",
      "mean": 2.3095968343399997,
      "stddev": 0.06695026994490309,
      "median": 2.2958962722400003,
      "user": 2.7338534,
      "system": 3.57925094,
      "min": 2.2495446697399997,
      "max": 2.45011461574,
      "times": [
        2.2497285957399997,
        2.28096525374,
        2.45011461574,
        2.2495446697399997,
        2.3063347257399998,
        2.30268232774,
        2.29607273974,
        2.4069394257399996,
        2.29571980474,
        2.2578661847399997
      ]
    },
    {
      "command": "pnpm",
      "mean": 4.63662658664,
      "stddev": 0.04390894835615857,
      "median": 4.62701579824,
      "user": 7.8551850000000005,
      "system": 4.0854468399999995,
      "min": 4.56980973674,
      "max": 4.705020745740001,
      "times": [
        4.705020745740001,
        4.6855551567400004,
        4.68278204974,
        4.61359808074,
        4.65437214474,
        4.599640039740001,
        4.56980973674,
        4.60145631574,
        4.632039417740001,
        4.62199217874
      ]
    }
  ]
}

Scenario: Frozen Lockfile (Hot Cache)

Command Mean [ms] Min [ms] Max [ms] Relative
pacquet@HEAD 698.3 ± 15.3 680.2 726.3 1.00
pacquet@main 727.2 ± 15.3 708.3 758.0 1.04 ± 0.03
pnpm 2444.4 ± 121.0 2357.7 2735.2 3.50 ± 0.19
BENCHMARK_REPORT.json
{
  "results": [
    {
      "command": "pacquet@HEAD",
      "mean": 0.69832386544,
      "stddev": 0.01529857338184942,
      "median": 0.6926843787400001,
      "user": 0.38560116,
      "system": 1.5963023999999997,
      "min": 0.6802108467400001,
      "max": 0.7263271017400001,
      "times": [
        0.7161531857400001,
        0.7263271017400001,
        0.6802108467400001,
        0.7029666937400001,
        0.6967073917400001,
        0.71123092474,
        0.6880814057400001,
        0.6886613657400001,
        0.6880705637400001,
        0.68482917474
      ]
    },
    {
      "command": "pacquet@main",
      "mean": 0.72723730614,
      "stddev": 0.015263304104285655,
      "median": 0.7235843527400001,
      "user": 0.39388866,
      "system": 1.6119052999999997,
      "min": 0.7082683057400001,
      "max": 0.7580397567400001,
      "times": [
        0.7580397567400001,
        0.7082683057400001,
        0.7180942417400001,
        0.7409137517400001,
        0.7278770927400001,
        0.7204629277400001,
        0.7267057777400001,
        0.7408269767400001,
        0.7183286457400001,
        0.7128555847400001
      ]
    },
    {
      "command": "pnpm",
      "mean": 2.44443673794,
      "stddev": 0.1209714595248195,
      "median": 2.3941316812399998,
      "user": 2.98984066,
      "system": 2.1966240000000004,
      "min": 2.35771581174,
      "max": 2.73519135874,
      "times": [
        2.56916407774,
        2.39408199774,
        2.35771581174,
        2.73519135874,
        2.3863297437399997,
        2.39477149874,
        2.3941813647399997,
        2.3743936037399997,
        2.47848902774,
        2.36004889474
      ]
    }
  ]
}

@zkochan zkochan changed the title feat(pacquet): scaffold Stage 2 resolver seam feat(pacquet): Stage 2 resolver scaffold + npm version picking May 20, 2026
@zkochan zkochan changed the title feat(pacquet): Stage 2 resolver scaffold + npm version picking feat(pacquet): resolver scaffold + npm version picking May 20, 2026
@zkochan zkochan marked this pull request as ready for review May 20, 2026 01:27
Copilot AI review requested due to automatic review settings May 20, 2026 01:27

Copilot AI 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.

Pull request overview

Adds the Stage 2 resolver seam foundations to pacquet and ports pnpm’s npm version-picking logic so future protocol resolvers can route and select concrete versions consistently with pnpm.

Changes:

  • Introduces a dispatcher-oriented resolver contract (WantedDependencyResolveResult) plus a default-resolver chain that selects the first claiming resolver.
  • Adds a new parse-wanted-dependency crate (including an inline validate-npm-package-name@7 branch port) to split raw specs into (alias, bare_specifier).
  • Ports pnpm’s npm resolver version picker (pickPackage*) into Rust, including cache/disk/network orchestration and publishedBy/minimumReleaseAge behavior; extends registry packument structs to support this.

Reviewed changes

Copilot reviewed 22 out of 23 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
pacquet/crates/resolving-resolver-base/src/verifier.rs Extracts/defines verifier-side trait + violation types as a dedicated module.
pacquet/crates/resolving-resolver-base/src/resolve.rs Adds dispatcher-side resolver contract types and the Resolver trait.
pacquet/crates/resolving-resolver-base/src/lib.rs Re-exports both verifier and dispatcher seams from one crate.
pacquet/crates/resolving-resolver-base/src/tests.rs Adds tests for selector weights, defaults, and Resolver dyn dispatch.
pacquet/crates/resolving-resolver-base/Cargo.toml Adds serde dependency needed for new serialized types.
pacquet/crates/resolving-parse-wanted-dependency/src/lib.rs Implements parse_wanted_dependency split logic and public API.
pacquet/crates/resolving-parse-wanted-dependency/src/validate_npm_package_name.rs Inline port of npm validForOldPackages name validation rules.
pacquet/crates/resolving-parse-wanted-dependency/src/tests.rs Pins parsing/name-validation behavior with unit tests.
pacquet/crates/resolving-parse-wanted-dependency/Cargo.toml New crate manifest for parse-wanted-dependency port.
pacquet/crates/resolving-default-resolver/src/lib.rs Implements DefaultResolver chain and “spec not supported” error.
pacquet/crates/resolving-default-resolver/src/tests.rs Tests chain ordering and spec rendering parity with pnpm.
pacquet/crates/resolving-default-resolver/Cargo.toml New crate manifest for default-resolver dispatcher.
pacquet/crates/resolving-npm-resolver/src/pick_package_from_meta.rs Pure packument version-picking logic + publishedBy filtering port.
pacquet/crates/resolving-npm-resolver/src/pick_package_from_meta/tests.rs Extensive tests for picker behavior parity (latest/tag/range/deprecated/publishedBy).
pacquet/crates/resolving-npm-resolver/src/pick_package.rs Cache/disk/network orchestration port around the pure picker.
pacquet/crates/resolving-npm-resolver/src/pick_package/tests.rs Orchestration tests (offline/preferOffline/mirror fast paths/dry_run/etc.).
pacquet/crates/resolving-npm-resolver/src/lib.rs Exposes the new picker/orchestrator APIs from the npm resolver crate.
pacquet/crates/registry/src/package.rs Makes dist_tags public and adds dist_tag() / dist_tags() accessors.
pacquet/crates/registry/src/package/tests.rs Updates fixtures for new deprecated field and dist-tags visibility.
pacquet/crates/registry/src/package_version.rs Adds deprecated: Option<String> to per-version manifests.
pacquet/crates/package-manager/src/build_snapshot/tests.rs Updates snapshot test fixture for new deprecated field.
Cargo.toml Registers new pacquet resolver crates in workspace dependencies.
Cargo.lock Adds lock entries for new crates and updated deps.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread pacquet/crates/resolving-npm-resolver/src/pick_package_from_meta.rs
Comment thread pacquet/crates/resolving-npm-resolver/src/pick_package.rs Outdated
Comment thread pacquet/crates/resolving-npm-resolver/src/pick_package.rs Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🧹 Nitpick comments (2)
pacquet/crates/registry/src/package.rs (1)

71-88: ⚡ Quick win

Add direct unit tests for the new dist-tag accessors.

dist_tag() / dist_tags() are new public API; please add focused tests for missing-tag behavior and iterator mapping so this contract stays locked.

🤖 Prompt for 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.

In `@pacquet/crates/registry/src/package.rs` around lines 71 - 88, Add focused
unit tests covering the new public API methods dist_tag and dist_tags: write
tests that assert dist_tag("existing") returns the correct &str,
dist_tag("missing") returns None, and that dist_tags() yields pairs whose keys
and values match the underlying map (e.g., collect into Vec and compare). Target
tests to use the Package struct constructor/setup used in other tests to
populate dist_tags with multiple entries (including a missing-tag case) and
verify iterator mapping preserves each tag->version mapping. Ensure tests live
alongside existing package tests and are named to indicate they cover dist-tag
accessors.
pacquet/crates/resolving-resolver-base/src/resolve.rs (1)

22-28: ⚡ Quick win

Avoid Default for WantedDependency because it violates its own invariant.

The docs require at least one of alias or bare_specifier, but #[derive(Default)] creates an invalid empty instance. Prefer removing Default and exposing a validating constructor.

🤖 Prompt for 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.

In `@pacquet/crates/resolving-resolver-base/src/resolve.rs` around lines 22 - 28,
The WantedDependency struct currently derives Default which can create an
instance violating the invariant that at least one of alias or bare_specifier
must be Some; remove the #[derive(Default)] from WantedDependency and add a
validating constructor (e.g., WantedDependency::new or
WantedDependency::try_new) that enforces that alias or bare_specifier is present
(returning Result or panicking as preferred), update construction sites such as
the parse-wanted-dependency port and the deps-resolver manifest reader to use
this constructor, and adjust any callsites/tests that relied on Default to
construct instances accordingly.
🤖 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 `@pacquet/crates/resolving-npm-resolver/src/pick_package_from_meta.rs`:
- Around line 183-190: The check that fast-paths on abbreviated metadata uses an
exclusive cutoff (modified_date match arm with `Some(date) if date < cutoff`)
causing packages with `modified == cutoff` to be rejected; update the condition
in the match (the `modified_date` arm that currently uses `date < cutoff`) to be
inclusive (i.e., accept `date <= cutoff` or equivalent) so that entries with
`modified == cutoff` take the abbreviated-metadata path instead of returning
`PickPackageFromMetaError::MissingTime` for `meta`.

In `@pacquet/crates/resolving-npm-resolver/src/pick_package.rs`:
- Around line 265-267: The in-memory meta cache is keyed only by spec.name which
allows packuments from one registry to be reused for resolves against a
different registry; update the cache key usage to include the registry (e.g.,
combine ctx.meta_cache lookups and inserts to use a composite key derived from
spec.registry or spec.source along with spec.name) so cached entries are scoped
per-registry; change the lookup around ctx.meta_cache.get(&spec.name) and the
corresponding insert/return sites so pick_matching_version_final is always
passed metadata from the correct registry and PickPackageResult.meta refers to
the registry-scoped cached entry (also update the other occurrence at the second
site noted in the comment).
- Around line 356-366: The code treats any on-disk mirror as a successful
fallback even when pick_matching_version_final(&picker_opts, spec, &disk) yields
None, which masks the original fetch error; change the branch that uses
meta_cached_in_store.or_else(|| pkg_mirror.as_deref().and_then(load_meta)) so
that after calling pick_matching_version_final you check whether it returned
Some(picked) and only return Ok(PickPackageResult { meta: disk, picked_package:
picked }) in that case — if pick_matching_version_final returns None, propagate
the original fetch error (the local error variable) instead of returning an
Ok(None)-style result so callers see the real fetch failure.

---

Nitpick comments:
In `@pacquet/crates/registry/src/package.rs`:
- Around line 71-88: Add focused unit tests covering the new public API methods
dist_tag and dist_tags: write tests that assert dist_tag("existing") returns the
correct &str, dist_tag("missing") returns None, and that dist_tags() yields
pairs whose keys and values match the underlying map (e.g., collect into Vec and
compare). Target tests to use the Package struct constructor/setup used in other
tests to populate dist_tags with multiple entries (including a missing-tag case)
and verify iterator mapping preserves each tag->version mapping. Ensure tests
live alongside existing package tests and are named to indicate they cover
dist-tag accessors.

In `@pacquet/crates/resolving-resolver-base/src/resolve.rs`:
- Around line 22-28: The WantedDependency struct currently derives Default which
can create an instance violating the invariant that at least one of alias or
bare_specifier must be Some; remove the #[derive(Default)] from WantedDependency
and add a validating constructor (e.g., WantedDependency::new or
WantedDependency::try_new) that enforces that alias or bare_specifier is present
(returning Result or panicking as preferred), update construction sites such as
the parse-wanted-dependency port and the deps-resolver manifest reader to use
this constructor, and adjust any callsites/tests that relied on Default to
construct instances accordingly.
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: b2eb45d3-494c-4e7d-876d-311ce3949574

📥 Commits

Reviewing files that changed from the base of the PR and between ced20cb and b17032f.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (22)
  • Cargo.toml
  • pacquet/crates/package-manager/src/build_snapshot/tests.rs
  • pacquet/crates/registry/src/package.rs
  • pacquet/crates/registry/src/package/tests.rs
  • pacquet/crates/registry/src/package_version.rs
  • pacquet/crates/resolving-default-resolver/Cargo.toml
  • pacquet/crates/resolving-default-resolver/src/lib.rs
  • pacquet/crates/resolving-default-resolver/src/tests.rs
  • pacquet/crates/resolving-npm-resolver/src/lib.rs
  • pacquet/crates/resolving-npm-resolver/src/pick_package.rs
  • pacquet/crates/resolving-npm-resolver/src/pick_package/tests.rs
  • pacquet/crates/resolving-npm-resolver/src/pick_package_from_meta.rs
  • pacquet/crates/resolving-npm-resolver/src/pick_package_from_meta/tests.rs
  • pacquet/crates/resolving-parse-wanted-dependency/Cargo.toml
  • pacquet/crates/resolving-parse-wanted-dependency/src/lib.rs
  • pacquet/crates/resolving-parse-wanted-dependency/src/tests.rs
  • pacquet/crates/resolving-parse-wanted-dependency/src/validate_npm_package_name.rs
  • pacquet/crates/resolving-resolver-base/Cargo.toml
  • pacquet/crates/resolving-resolver-base/src/lib.rs
  • pacquet/crates/resolving-resolver-base/src/resolve.rs
  • pacquet/crates/resolving-resolver-base/src/tests.rs
  • pacquet/crates/resolving-resolver-base/src/verifier.rs
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
  • GitHub Check: Lint and Test (macos-latest)
  • GitHub Check: Lint and Test (ubuntu-latest)
  • GitHub Check: Lint and Test (windows-latest)
  • GitHub Check: Dylint
  • GitHub Check: Run benchmark on ubuntu-latest
  • GitHub Check: Agent
  • GitHub Check: Run benchmark on ubuntu-latest
  • GitHub Check: Code Coverage
  • GitHub Check: Compile & Lint
🧰 Additional context used
📓 Path-based instructions (1)
pacquet/**/*.rs

📄 CodeRabbit inference engine (pacquet/AGENTS.md)

pacquet/**/*.rs: When porting a function that fires pnpm:<channel> events through globalLogger, logger.debug(), or streamParser.write(), mirror the call site, payload, and ordering so the reporter parses pacquet's NDJSON the same way it parses pnpm's.
Declare a newtype wrapper for branded string types. Do not collapse the brand into a plain String or &str.
If upstream always validates before construction, validate in pacquet's wrapper too. The wrapper must construct only via TryFrom<String> and/or FromStr. Do not provide an infallible public constructor.
If upstream never validates, just brand for type-safety. Expose an infallible From<String> (and From<&str> when convenient).
If upstream occasionally constructs without validation, expose from_str_unchecked as an escape hatch alongside the validating constructor.
Match upstream serde behavior for branded types that cross JSON, YAML, or INI boundaries. Use #[serde(try_from = "String")] for deserialization and #[serde(into = "String")] for serialization.
Use #[derive(derive_more::From)] and #[derive(derive_more::Into)] for mechanical conversion impls. Fall back to manual impl only when conversion needs custom logic.
String-literal unions should become enums, not newtype wrappers. Model closed sets of valid string values as enums.
Template literal types should be treated as branded strings with validation discipline from rules 2-5.
Choose owned vs. borrowed parameters to minimize copies. Widen to the most encompassing type (&Path over &PathBuf, &str over &String) when it doesn't force extra copies.
Prefer Arc::clone(&x) / Rc::clone(&x) over x.clone() for reference-counted types, so the cost is visible at the call site.
Follow Rust API Guidelines for naming conventions.
Do not use star imports inside module bodies. Write use super::{Foo, bar} instead of use super::*;. Two forms stay allowed: external-crate preludes like use rayon::prelude::*; and root-of-module re-...

Files:

  • pacquet/crates/package-manager/src/build_snapshot/tests.rs
  • pacquet/crates/registry/src/package_version.rs
  • pacquet/crates/registry/src/package/tests.rs
  • pacquet/crates/resolving-parse-wanted-dependency/src/lib.rs
  • pacquet/crates/resolving-resolver-base/src/verifier.rs
  • pacquet/crates/registry/src/package.rs
  • pacquet/crates/resolving-resolver-base/src/tests.rs
  • pacquet/crates/resolving-resolver-base/src/resolve.rs
  • pacquet/crates/resolving-npm-resolver/src/lib.rs
  • pacquet/crates/resolving-parse-wanted-dependency/src/tests.rs
  • pacquet/crates/resolving-default-resolver/src/tests.rs
  • pacquet/crates/resolving-resolver-base/src/lib.rs
  • pacquet/crates/resolving-parse-wanted-dependency/src/validate_npm_package_name.rs
  • pacquet/crates/resolving-npm-resolver/src/pick_package_from_meta/tests.rs
  • pacquet/crates/resolving-npm-resolver/src/pick_package/tests.rs
  • pacquet/crates/resolving-default-resolver/src/lib.rs
  • pacquet/crates/resolving-npm-resolver/src/pick_package_from_meta.rs
  • pacquet/crates/resolving-npm-resolver/src/pick_package.rs
🔇 Additional comments (20)
pacquet/crates/registry/src/package_version.rs (1)

38-49: LGTM!

pacquet/crates/registry/src/package.rs (1)

16-16: LGTM!

pacquet/crates/package-manager/src/build_snapshot/tests.rs (1)

31-31: LGTM!

pacquet/crates/registry/src/package/tests.rs (1)

23-23: LGTM!

Also applies to: 44-44, 99-99

pacquet/crates/resolving-resolver-base/src/lib.rs (1)

1-37: LGTM!

pacquet/crates/resolving-resolver-base/src/resolve.rs (1)

1-21: LGTM!

Also applies to: 29-278

pacquet/crates/resolving-resolver-base/src/verifier.rs (1)

1-116: LGTM!

pacquet/crates/resolving-resolver-base/Cargo.toml (1)

16-16: LGTM!

pacquet/crates/resolving-resolver-base/src/tests.rs (1)

1-8: LGTM!

Also applies to: 107-187

pacquet/crates/resolving-parse-wanted-dependency/Cargo.toml (1)

1-17: LGTM!

pacquet/crates/resolving-parse-wanted-dependency/src/lib.rs (1)

1-85: LGTM!

pacquet/crates/resolving-parse-wanted-dependency/src/tests.rs (1)

1-140: LGTM!

pacquet/crates/resolving-parse-wanted-dependency/src/validate_npm_package_name.rs (1)

1-97: LGTM!

pacquet/crates/resolving-default-resolver/Cargo.toml (1)

1-27: LGTM!

pacquet/crates/resolving-default-resolver/src/lib.rs (1)

1-140: LGTM!

pacquet/crates/resolving-default-resolver/src/tests.rs (1)

1-167: LGTM!

pacquet/crates/resolving-npm-resolver/src/lib.rs (1)

22-49: LGTM!

Cargo.toml (1)

16-43: LGTM!

pacquet/crates/resolving-npm-resolver/src/pick_package/tests.rs (1)

1-390: LGTM!

pacquet/crates/resolving-npm-resolver/src/pick_package_from_meta/tests.rs (1)

1-526: LGTM!

Comment thread pacquet/crates/resolving-npm-resolver/src/pick_package_from_meta.rs
Comment thread pacquet/crates/resolving-npm-resolver/src/pick_package.rs Outdated
Comment thread pacquet/crates/resolving-npm-resolver/src/pick_package.rs
zkochan added a commit that referenced this pull request May 20, 2026
Address review comments on #11755:

- **Cache parsed `Range` values** (Copilot). `semver_satisfies_loose`,
  `max_satisfying`, `min_satisfying` reparse `Range` on every call;
  in hot loops like `prioritize_preferred_versions` that's O(n)
  repeated parse work per packument. Mirror upstream's
  `semverRangeCache` with a process-global `DashMap<String,
  Option<Arc<Range>>>` so each range string is parsed at most once
  per process.

- **Recover from mutex poisoning** in `InMemoryPackageMetaCache`
  (Copilot). Other pacquet code (e.g. `build_modules.rs`) already
  uses `unwrap_or_else(|err| err.into_inner())` — the cache is a
  plain `HashMap` of cloneable values, so no broken invariants
  outlive a poisoned lock. Swap `.expect(...)` for the recovery
  pattern. Same for the `warn_missing_time_once` set.

- **Use `IndexSet` for the missing-time warning set** (Copilot).
  Replaces `Vec<String>` membership scan + `Vec::remove(0)`
  eviction with O(1) `contains` and O(n)-but-cheap-shift
  `shift_remove_index(0)`. Insertion order is preserved (matching
  upstream's JS `Set` iteration order), and the cap stays at 1024.

- **Scope the in-memory cache key by registry** (coderabbit). The
  same package name in two registries (private + public, scoped
  override, etc.) was colliding on `spec.name` alone; a packument
  fetched against one registry could short-circuit a later resolve
  against a different registry, returning the wrong metadata. Switch
  to `<registry>\x00<name>` cache keys. Adds
  `in_memory_cache_does_not_leak_across_registries` as a regression
  test pinning the new shape.

Two coderabbit suggestions deliberately not applied — both would
diverge from upstream pnpm:

- `<` vs `<=` in the `modified` shortcut: upstream pnpm uses strict
  `<` ([pickPackage.ts](https://github.com/pnpm/pnpm/blob/f657b5cb44/resolving/npm-resolver/src/pickPackage.ts#L372)
  and [pickPackageFromMeta.ts](https://github.com/pnpm/pnpm/blob/f657b5cb44/resolving/npm-resolver/src/pickPackageFromMeta.ts#L47))
  to keep the modified-shortcut conservative — if `modified == cutoff`,
  a version published exactly at the cutoff might exist and the
  fast path can't be taken. The per-version filter (`<=`) is a
  separate concern.

- Returning the picker's `None` on a fetch failure with a stale
  mirror: upstream pnpm explicitly returns
  `pickMatchingVersionFinal(...)`'s result verbatim in the
  fetch-error catch block ([pickPackage.ts](https://github.com/pnpm/pnpm/blob/f657b5cb44/resolving/npm-resolver/src/pickPackage.ts#L420-L431)),
  including when it's `null`. Re-throwing the fetch error would
  diverge from upstream's "swallow + serve stale" semantics.
@coderabbitai coderabbitai Bot added the area: config dependencies Changes related to configDependencies. label May 20, 2026
@zkochan zkochan removed the area: config dependencies Changes related to configDependencies. label May 20, 2026
zkochan added a commit that referenced this pull request May 20, 2026
The `minimumReleaseAge` shortcut on abbreviated metadata used strict
`<` against the cutoff (and equivalently `>=` to reject), so a
packument whose package-level `modified` equalled the cutoff fell
off the fast path. That's an unnecessary divergence from the
per-version filter in `filterPkgMetadataByPublishDate`, which uses
inclusive `<=` to treat a version published exactly at the cutoff
as mature.

Since `modified` is an upper bound on every version's publish time,
`modified == publishedBy` already implies every version passes the
per-version `<=` filter. The shortcut can now accept the boundary
case directly instead of triggering a full-metadata re-fetch (or a
`MISSING_TIME` error when full metadata isn't permitted).

Lands in both stacks under the parity rule in pacquet's AGENTS.md:

- **TypeScript pnpm** — `resolving/npm-resolver/src/pickPackage.ts`
  (`maybeUpgradeAbbreviatedMetaForReleaseAge`, and the in-band
  upgrade branch) and `resolving/npm-resolver/src/pickPackageFromMeta.ts`
  (the abbreviated `assertMetaHasTime` gate). Each call site flips
  `>=` to `>` (or `<` to `<=`) and adds a comment pinning the
  rationale.

- **Pacquet** — `pacquet/crates/resolving-npm-resolver/src/pick_package_from_meta.rs`
  flips `<` to `<=` in the abbreviated branch.

Tests:

- New `pick_from_meta_modified_shortcut_inclusive_at_cutoff` in
  pacquet (verified to catch the regression by reverting the
  comparison locally — the test fails, then passes when restored).
- New "use abbreviated metadata when modified date equals
  publishedBy" in `publishedBy.test.ts` upstream.

Changeset bumps `@pnpm/resolving.npm-resolver` and `pnpm` patch.

Addresses review feedback from coderabbit on #11755.
Copilot AI review requested due to automatic review settings May 20, 2026 01:52
zkochan added 9 commits May 20, 2026 03:52
Lays down the seam future per-protocol resolvers (npm, git, tarball,
local, jsr, runtimes, named-registry, workspace) will plug into.

New crates:

- `pacquet-resolving-parse-wanted-dependency` ports pnpm's
  [`@pnpm/resolving.parse-wanted-dependency`](https://github.com/pnpm/pnpm/blob/3687b0e180/resolving/parse-wanted-dependency/src/index.ts)
  with an inline port of `validate-npm-package-name@7`'s
  `validForOldPackages` branch. Splits raw manifest specs like
  `foo@1.2.3`, `@scope/foo@^1`, and `foo@npm:lodash@^4` into
  `(alias, bareSpecifier)` halves; passes git/tarball/range strings
  through unchanged. Behavior verified byte-for-byte against the
  upstream JS for the test corpus.

- `pacquet-resolving-default-resolver` ports the protocol-routing
  dispatcher at the top of
  [`resolving/default-resolver/src/index.ts`](https://github.com/pnpm/pnpm/blob/3687b0e180/resolving/default-resolver/src/index.ts).
  `DefaultResolver` walks a `Vec<Box<dyn Resolver>>` chain in order
  and returns the first resolver's `Ok(Some(_))` claim, falling back
  to a `SpecNotSupportedByAnyResolverError` (upstream's
  `SPEC_NOT_SUPPORTED_BY_ANY_RESOLVER`). Chain is empty until the
  per-protocol resolver PRs land.

Extends `pacquet-resolving-resolver-base` (previously verifier-only)
with the dispatcher contract: `WantedDependency`, `ResolveOptions`,
`ResolveResult`, `LatestQuery`, `LatestInfo`, `PreferredVersions` and
related selector types, `WorkspacePackage(s)`, weight constants, and
the `Resolver` trait + boxed-future return types. Verifier types
moved into their own submodule with no public-surface change.

Part of #11633 Stage 2 — Tier 1 foundations.
Ports pnpm's pickPackageFromMeta and pickPackage from
resolving/npm-resolver/src/ — the pure version-picking surface and
the cache+fetch orchestration that wraps it. Plugs into the Stage 2
resolver scaffolding so the chain's npm leg can resolve a registry
spec to a concrete (name, version, manifest) triple.

Two new modules under `crates/resolving-npm-resolver/src/`:

- `pick_package_from_meta` — the pure picker. Ports
  [`pickPackageFromMeta`](https://github.com/pnpm/pnpm/blob/f657b5cb44/resolving/npm-resolver/src/pickPackageFromMeta.ts),
  `pickVersionByVersionRange` (with the deprecated-version fallback),
  `pickLowestVersionByVersionRange`, `filterPkgMetadataByPublishDate`
  (the `minimumReleaseAge` filter), the
  `PreferredVersionsPrioritizer` group-by-weight logic, and the
  error codes upstream surfaces (`ERR_PNPM_UNPUBLISHED_PKG`,
  `ERR_PNPM_NO_VERSIONS`, `ERR_PNPM_MISSING_TIME`). Introduces
  `RegistryPackageSpec` / `RegistryPackageSpecType` as the picker's
  input shape; the parser that produces these is its own port.

- `pick_package` — the cache+fetch orchestration. Ports
  [`pickPackage`](https://github.com/pnpm/pnpm/blob/f657b5cb44/resolving/npm-resolver/src/pickPackage.ts)
  with all four pre-fetch layers (in-memory `PackageMetaCache`,
  offline / preferOffline / pickLowestVersion disk read, version-spec
  fast path, publishedBy mtime shortcut), the 304-aware fetch via
  `fetch_full_metadata_cached`, the `include_latest_tag` runner that
  picks `max(spec, latest)`, the publishedBy-active fall-back-to-
  lowest path (`pickRespectingMinReleaseAge`), and the warn-and-skip
  `ignore_missing_time_field` path. Exposes `PackageMetaCache` as a
  trait with an `InMemoryPackageMetaCache` default impl plus a
  `shared_in_memory_cache()` factory.

Supporting changes:

- `pacquet_registry::PackageVersion` gained
  `deprecated: Option<String>` (the deprecated-fallback in
  pickVersionByVersionRange needs it; the field was missing).
- `pacquet_registry::Package` gained `dist_tag(tag)` / `dist_tags()`
  accessors, and `dist_tags` is now `pub` so the publishedBy filter
  can rebuild a packument with rewritten tags.

Documented divergences from upstream:

- Pacquet's metadata fetcher returns full metadata only; the
  abbreviated→full upgrade branches are in place but never fire
  today. Adding an abbreviated fetcher later is a drop-in.
- `dry_run` skips the in-memory cache write but the underlying
  fetcher still warms the on-disk mirror. Restoring upstream's
  "no disk side effects on dry-run" needs the fetcher to grow a
  write-bypass flag.
- Upstream's `p-limit(1)` per-mirror serialization is omitted; the
  atomic rename in `save_meta` covers write safety.

Tests: 21 new picker tests + 30 new orchestration tests
(mockito-backed). Full workspace nextest run passes (1547 tests).

Part of #11633 Stage 2 — npm resolver.
CI's `Doc` job (`RUSTDOCFLAGS="-D warnings"`) and `Dylint` job
(`-D perfectionist::derive-ordering`) caught issues missed by
`just ready`'s default profile:

- Disambiguate two-meaning intra-doc links the `Doc` job rejected as
  ambiguous: `fetch_full_metadata_cached()` (function vs module) and
  `crate::pick_package()` (function vs module).
- Qualify two unresolved links: `MissingTime` →
  `PickPackageFromMetaError::MissingTime`, and `LockfileResolution` →
  `pacquet_lockfile::LockfileResolution` in the resolver-base lib
  docs.
- Reorder four `#[derive(...)]` lists in
  `resolving-resolver-base/src/resolve.rs` so `Default` sits next to
  `Debug` in the prefix block (the `prefix_then_alphabetical`
  Perfectionist rule the `Dylint` job enforces). `Debug, Clone,
  Default, …` → `Debug, Default, Clone, …` and similar.
CI's `Dylint` job runs additional Perfectionist lints that
`just lint` (stable clippy) doesn't see. Fixing the leftover
violations:

- `single-letter-generic`: rename single-letter type parameters that
  don't carry meaning by themselves. `C: PackageMetaCache` →
  `Cache: PackageMetaCache`, the picker-fn `F` →
  `PickFn` / `PickOne`, the `AsRef<str>` versions slice `S` →
  `Raw`.
- `single-letter-generic` (parameter form): rename `a` / `b` in
  `pick_max` to `lhs` / `rhs`.
- `arc-clone-not-direct`: swap `meta.mutex.clone()` for
  `Arc::clone(&meta.mutex)` so the refcount bump is visible at the
  call site (matches the project style guide).
- `unicode-ellipsis-in-comments`: replace `…` with `...` in the
  default-resolver test that landed in the scaffold commit.
- `prefer-raw-string`: rewrite four `"\"…\" isn't supported…"`
  literals in the default-resolver tests as `r#""…" isn't…"#` raw
  strings.
CI's `Format` job (`cargo fmt --check`) and `Dylint` job
(`-D perfectionist::trailing-comma-in-single-line-macro`) caught two
spots where a multi-line `assert!` / `assert_eq!` collapsed to a
single line during the last fmt pass and kept its trailing comma.
Drop the trailing comma so both checks are happy.
…param

Dylint's `perfectionist::single-letter-generic` rule also catches
single-letter closure parameters. `|c|` in `is_url_friendly` →
`|ch|` so the rule passes.
Address review comments on #11755:

- **Cache parsed `Range` values** (Copilot). `semver_satisfies_loose`,
  `max_satisfying`, `min_satisfying` reparse `Range` on every call;
  in hot loops like `prioritize_preferred_versions` that's O(n)
  repeated parse work per packument. Mirror upstream's
  `semverRangeCache` with a process-global `DashMap<String,
  Option<Arc<Range>>>` so each range string is parsed at most once
  per process.

- **Recover from mutex poisoning** in `InMemoryPackageMetaCache`
  (Copilot). Other pacquet code (e.g. `build_modules.rs`) already
  uses `unwrap_or_else(|err| err.into_inner())` — the cache is a
  plain `HashMap` of cloneable values, so no broken invariants
  outlive a poisoned lock. Swap `.expect(...)` for the recovery
  pattern. Same for the `warn_missing_time_once` set.

- **Use `IndexSet` for the missing-time warning set** (Copilot).
  Replaces `Vec<String>` membership scan + `Vec::remove(0)`
  eviction with O(1) `contains` and O(n)-but-cheap-shift
  `shift_remove_index(0)`. Insertion order is preserved (matching
  upstream's JS `Set` iteration order), and the cap stays at 1024.

- **Scope the in-memory cache key by registry** (coderabbit). The
  same package name in two registries (private + public, scoped
  override, etc.) was colliding on `spec.name` alone; a packument
  fetched against one registry could short-circuit a later resolve
  against a different registry, returning the wrong metadata. Switch
  to `<registry>\x00<name>` cache keys. Adds
  `in_memory_cache_does_not_leak_across_registries` as a regression
  test pinning the new shape.

Two coderabbit suggestions deliberately not applied — both would
diverge from upstream pnpm:

- `<` vs `<=` in the `modified` shortcut: upstream pnpm uses strict
  `<` ([pickPackage.ts](https://github.com/pnpm/pnpm/blob/f657b5cb44/resolving/npm-resolver/src/pickPackage.ts#L372)
  and [pickPackageFromMeta.ts](https://github.com/pnpm/pnpm/blob/f657b5cb44/resolving/npm-resolver/src/pickPackageFromMeta.ts#L47))
  to keep the modified-shortcut conservative — if `modified == cutoff`,
  a version published exactly at the cutoff might exist and the
  fast path can't be taken. The per-version filter (`<=`) is a
  separate concern.

- Returning the picker's `None` on a fetch failure with a stale
  mirror: upstream pnpm explicitly returns
  `pickMatchingVersionFinal(...)`'s result verbatim in the
  fetch-error catch block ([pickPackage.ts](https://github.com/pnpm/pnpm/blob/f657b5cb44/resolving/npm-resolver/src/pickPackage.ts#L420-L431)),
  including when it's `null`. Re-throwing the fetch error would
  diverge from upstream's "swallow + serve stale" semantics.
The `minimumReleaseAge` shortcut on abbreviated metadata used strict
`<` against the cutoff (and equivalently `>=` to reject), so a
packument whose package-level `modified` equalled the cutoff fell
off the fast path. That's an unnecessary divergence from the
per-version filter in `filterPkgMetadataByPublishDate`, which uses
inclusive `<=` to treat a version published exactly at the cutoff
as mature.

Since `modified` is an upper bound on every version's publish time,
`modified == publishedBy` already implies every version passes the
per-version `<=` filter. The shortcut can now accept the boundary
case directly instead of triggering a full-metadata re-fetch (or a
`MISSING_TIME` error when full metadata isn't permitted).

Lands in both stacks under the parity rule in pacquet's AGENTS.md:

- **TypeScript pnpm** — `resolving/npm-resolver/src/pickPackage.ts`
  (`maybeUpgradeAbbreviatedMetaForReleaseAge`, and the in-band
  upgrade branch) and `resolving/npm-resolver/src/pickPackageFromMeta.ts`
  (the abbreviated `assertMetaHasTime` gate). Each call site flips
  `>=` to `>` (or `<` to `<=`) and adds a comment pinning the
  rationale.

- **Pacquet** — `pacquet/crates/resolving-npm-resolver/src/pick_package_from_meta.rs`
  flips `<` to `<=` in the abbreviated branch.

Tests:

- New `pick_from_meta_modified_shortcut_inclusive_at_cutoff` in
  pacquet (verified to catch the regression by reverting the
  comparison locally — the test fails, then passes when restored).
- New "use abbreviated metadata when modified date equals
  publishedBy" in `publishedBy.test.ts` upstream.

Changeset bumps `@pnpm/resolving.npm-resolver` and `pnpm` patch.

Addresses review feedback from coderabbit on #11755.

Copilot AI 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.

Pull request overview

Copilot reviewed 27 out of 28 changed files in this pull request and generated 2 comments.

Comment thread pacquet/crates/resolving-npm-resolver/src/pick_package_from_meta.rs Outdated
Comment thread pacquet/crates/resolving-resolver-base/src/resolve.rs
@coderabbitai coderabbitai Bot added the area: config dependencies Changes related to configDependencies. label May 20, 2026
@zkochan zkochan added area: resolution and removed area: config dependencies Changes related to configDependencies. labels May 20, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
pacquet/crates/resolving-npm-resolver/src/pick_package_from_meta/tests.rs (1)

205-224: ⚡ Quick win

Strengthen the selector test assertion to verify the actual winner.

assert!(...is_some()) won’t catch regressions in preferred-selector weighting behavior. This test should assert the picked version explicitly (or rename the test if it’s only a crash-safety check).

♻️ Proposed tightening
-    assert!(pick_version_by_version_range(&opts).is_some());
+    assert_eq!(
+        pick_version_by_version_range(&opts).as_deref(),
+        Some("1.0.0"),
+        "tag selector should bias toward the tagged in-range version",
+    );
🤖 Prompt for 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.

In `@pacquet/crates/resolving-npm-resolver/src/pick_package_from_meta/tests.rs`
around lines 205 - 224, The test preferred_versions_tag_selector_wins currently
only asserts pick_version_by_version_range(&opts).is_some(); change it to assert
the exact picked version to prevent regressions: call
pick_version_by_version_range(&opts), unwrap the Some result and compare it to
the expected in-range winner (the "latest" tag -> "1.2.0" in the test data).
Update the assertion to check the returned version string/value equals "1.2.0"
(using the same return shape as pick_version_by_version_range) so the test
validates selector weighting, referencing the test function
preferred_versions_tag_selector_wins, the
VersionSelectors/VersionSelectorEntry/VersionSelectorType setup, and
pick_version_by_version_range/PickVersionByVersionRangeOptions.
🤖 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.

Nitpick comments:
In `@pacquet/crates/resolving-npm-resolver/src/pick_package_from_meta/tests.rs`:
- Around line 205-224: The test preferred_versions_tag_selector_wins currently
only asserts pick_version_by_version_range(&opts).is_some(); change it to assert
the exact picked version to prevent regressions: call
pick_version_by_version_range(&opts), unwrap the Some result and compare it to
the expected in-range winner (the "latest" tag -> "1.2.0" in the test data).
Update the assertion to check the returned version string/value equals "1.2.0"
(using the same return shape as pick_version_by_version_range) so the test
validates selector weighting, referencing the test function
preferred_versions_tag_selector_wins, the
VersionSelectors/VersionSelectorEntry/VersionSelectorType setup, and
pick_version_by_version_range/PickVersionByVersionRangeOptions.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 50514015-f9d0-4327-8aec-fd46c3484604

📥 Commits

Reviewing files that changed from the base of the PR and between e5bb53b and d3470ac.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (27)
  • .changeset/minimum-release-age-modified-shortcut-inclusive.md
  • Cargo.toml
  • pacquet/crates/package-manager/src/build_snapshot/tests.rs
  • pacquet/crates/registry/src/package.rs
  • pacquet/crates/registry/src/package/tests.rs
  • pacquet/crates/registry/src/package_version.rs
  • pacquet/crates/resolving-default-resolver/Cargo.toml
  • pacquet/crates/resolving-default-resolver/src/lib.rs
  • pacquet/crates/resolving-default-resolver/src/tests.rs
  • pacquet/crates/resolving-npm-resolver/Cargo.toml
  • pacquet/crates/resolving-npm-resolver/src/lib.rs
  • pacquet/crates/resolving-npm-resolver/src/pick_package.rs
  • pacquet/crates/resolving-npm-resolver/src/pick_package/tests.rs
  • pacquet/crates/resolving-npm-resolver/src/pick_package_from_meta.rs
  • pacquet/crates/resolving-npm-resolver/src/pick_package_from_meta/tests.rs
  • pacquet/crates/resolving-parse-wanted-dependency/Cargo.toml
  • pacquet/crates/resolving-parse-wanted-dependency/src/lib.rs
  • pacquet/crates/resolving-parse-wanted-dependency/src/tests.rs
  • pacquet/crates/resolving-parse-wanted-dependency/src/validate_npm_package_name.rs
  • pacquet/crates/resolving-resolver-base/Cargo.toml
  • pacquet/crates/resolving-resolver-base/src/lib.rs
  • pacquet/crates/resolving-resolver-base/src/resolve.rs
  • pacquet/crates/resolving-resolver-base/src/tests.rs
  • pacquet/crates/resolving-resolver-base/src/verifier.rs
  • resolving/npm-resolver/src/pickPackage.ts
  • resolving/npm-resolver/src/pickPackageFromMeta.ts
  • resolving/npm-resolver/test/publishedBy.test.ts
✅ Files skipped from review due to trivial changes (3)
  • .changeset/minimum-release-age-modified-shortcut-inclusive.md
  • pacquet/crates/package-manager/src/build_snapshot/tests.rs
  • pacquet/crates/resolving-default-resolver/Cargo.toml
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
  • GitHub Check: Run benchmark on ubuntu-latest
  • GitHub Check: Lint and Test (windows-latest)
  • GitHub Check: Lint and Test (macos-latest)
  • GitHub Check: Run benchmark on ubuntu-latest
  • GitHub Check: Code Coverage
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ts,tsx}: Follow Standard Style with trailing commas, prefer functions over classes, declare functions after they are used (rely on hoisting), and use a single options object for functions with more than two or three arguments
Sort imports in three groups: standard libraries, external dependencies (alphabetically), then relative imports
Write code that explains itself through clear naming and types — do not write comments that merely restate what the code already says; use comments only for non-obvious reasons, hidden invariants, or workarounds

Files:

  • resolving/npm-resolver/test/publishedBy.test.ts
  • resolving/npm-resolver/src/pickPackageFromMeta.ts
  • resolving/npm-resolver/src/pickPackage.ts
**/*.test.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Use util.types.isNativeError() instead of instanceof Error when type-checking errors in Jest tests, as instanceof checks can fail across VM realms

Files:

  • resolving/npm-resolver/test/publishedBy.test.ts
pacquet/**/*.rs

📄 CodeRabbit inference engine (pacquet/AGENTS.md)

pacquet/**/*.rs: When porting a function that fires pnpm:<channel> events through globalLogger, logger.debug(), or streamParser.write(), mirror the call site, payload, and ordering so the reporter parses pacquet's NDJSON the same way it parses pnpm's.
Declare a newtype wrapper for branded string types. Do not collapse the brand into a plain String or &str.
If upstream always validates before construction, validate in pacquet's wrapper too. The wrapper must construct only via TryFrom<String> and/or FromStr. Do not provide an infallible public constructor.
If upstream never validates, just brand for type-safety. Expose an infallible From<String> (and From<&str> when convenient).
If upstream occasionally constructs without validation, expose from_str_unchecked as an escape hatch alongside the validating constructor.
Match upstream serde behavior for branded types that cross JSON, YAML, or INI boundaries. Use #[serde(try_from = "String")] for deserialization and #[serde(into = "String")] for serialization.
Use #[derive(derive_more::From)] and #[derive(derive_more::Into)] for mechanical conversion impls. Fall back to manual impl only when conversion needs custom logic.
String-literal unions should become enums, not newtype wrappers. Model closed sets of valid string values as enums.
Template literal types should be treated as branded strings with validation discipline from rules 2-5.
Choose owned vs. borrowed parameters to minimize copies. Widen to the most encompassing type (&Path over &PathBuf, &str over &String) when it doesn't force extra copies.
Prefer Arc::clone(&x) / Rc::clone(&x) over x.clone() for reference-counted types, so the cost is visible at the call site.
Follow Rust API Guidelines for naming conventions.
Do not use star imports inside module bodies. Write use super::{Foo, bar} instead of use super::*;. Two forms stay allowed: external-crate preludes like use rayon::prelude::*; and root-of-module re-...

Files:

  • pacquet/crates/registry/src/package_version.rs
  • pacquet/crates/registry/src/package.rs
  • pacquet/crates/resolving-parse-wanted-dependency/src/lib.rs
  • pacquet/crates/resolving-default-resolver/src/lib.rs
  • pacquet/crates/resolving-resolver-base/src/resolve.rs
  • pacquet/crates/resolving-resolver-base/src/verifier.rs
  • pacquet/crates/registry/src/package/tests.rs
  • pacquet/crates/resolving-parse-wanted-dependency/src/tests.rs
  • pacquet/crates/resolving-resolver-base/src/lib.rs
  • pacquet/crates/resolving-default-resolver/src/tests.rs
  • pacquet/crates/resolving-parse-wanted-dependency/src/validate_npm_package_name.rs
  • pacquet/crates/resolving-npm-resolver/src/pick_package_from_meta/tests.rs
  • pacquet/crates/resolving-resolver-base/src/tests.rs
  • pacquet/crates/resolving-npm-resolver/src/lib.rs
  • pacquet/crates/resolving-npm-resolver/src/pick_package.rs
  • pacquet/crates/resolving-npm-resolver/src/pick_package/tests.rs
  • pacquet/crates/resolving-npm-resolver/src/pick_package_from_meta.rs
🧠 Learnings (1)
📚 Learning: 2026-05-14T09:04:00.133Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11622
File: resolving/npm-resolver/test/publishedBy.test.ts:350-354
Timestamp: 2026-05-14T09:04:00.133Z
Learning: In the pnpm/pnpm repository, ESLint is the authoritative style linter. Do not raise review findings for missing trailing commas in multiline function calls (e.g., `fs.writeFileSync(...)`) when this repo’s ESLint configuration does not report them and lint passes. Prefer deferring to the ESLint results for this specific trailing-comma rule rather than enforcing it manually in code review.

Applied to files:

  • resolving/npm-resolver/test/publishedBy.test.ts
  • resolving/npm-resolver/src/pickPackageFromMeta.ts
  • resolving/npm-resolver/src/pickPackage.ts
🔇 Additional comments (24)
resolving/npm-resolver/test/publishedBy.test.ts (1)

195-217: LGTM!

pacquet/crates/resolving-npm-resolver/Cargo.toml (1)

21-23: LGTM!

pacquet/crates/resolving-parse-wanted-dependency/Cargo.toml (1)

1-17: LGTM!

pacquet/crates/registry/src/package_version.rs (1)

38-49: LGTM!

pacquet/crates/resolving-resolver-base/Cargo.toml (1)

16-16: LGTM!

Cargo.toml (1)

16-44: LGTM!

resolving/npm-resolver/src/pickPackageFromMeta.ts (1)

47-57: LGTM!

resolving/npm-resolver/src/pickPackage.ts (1)

372-377: LGTM!

Also applies to: 474-479

pacquet/crates/registry/src/package.rs (1)

16-16: LGTM!

Also applies to: 71-88

pacquet/crates/resolving-parse-wanted-dependency/src/lib.rs (1)

1-85: LGTM!

pacquet/crates/resolving-default-resolver/src/lib.rs (1)

1-140: LGTM!

pacquet/crates/resolving-resolver-base/src/resolve.rs (1)

1-278: LGTM!

pacquet/crates/resolving-resolver-base/src/verifier.rs (1)

1-117: LGTM!

pacquet/crates/registry/src/package/tests.rs (1)

23-23: LGTM!

Also applies to: 44-44, 99-99

pacquet/crates/resolving-parse-wanted-dependency/src/tests.rs (1)

1-140: LGTM!

pacquet/crates/resolving-resolver-base/src/lib.rs (1)

1-37: LGTM!

pacquet/crates/resolving-default-resolver/src/tests.rs (1)

1-168: LGTM!

pacquet/crates/resolving-parse-wanted-dependency/src/validate_npm_package_name.rs (1)

1-97: LGTM!

pacquet/crates/resolving-npm-resolver/src/pick_package_from_meta/tests.rs (1)

1-204: LGTM!

Also applies to: 225-550

pacquet/crates/resolving-resolver-base/src/tests.rs (1)

1-188: LGTM!

pacquet/crates/resolving-npm-resolver/src/lib.rs (1)

22-23: LGTM!

Also applies to: 40-49

pacquet/crates/resolving-npm-resolver/src/pick_package.rs (1)

1-685: LGTM!

pacquet/crates/resolving-npm-resolver/src/pick_package/tests.rs (1)

1-493: LGTM!

pacquet/crates/resolving-npm-resolver/src/pick_package_from_meta.rs (1)

1-620: LGTM!

zkochan added 2 commits May 20, 2026 04:07
The npm registry occasionally serves `"deprecated": false` (a
boolean) for never-deprecated versions, even though the upstream
TypeScript type declares the field as a string. JavaScript silently
stores the boolean and the upstream `if (info.deprecated)`
truthiness check happens to do the right thing across both shapes.
Rust serde is strict, so a real install of any workload containing
react/react-dom/scheduler/create-react-class tripped the verifier:

> Failed to decode metadata from https://registry.npmjs.org/react:
> invalid type: boolean `false`, expected a string at line 1 column 324915

Route the field through a custom deserializer that normalizes the
wire shape to `Option<String>`:

- `string s` → `Some(s)` (deprecation reason).
- `false`    → `None`    (not deprecated).
- `true`     → `Some("")` (deprecated without a recorded reason).
- missing    → `None`    (via the existing `#[serde(default)]`).

`#[serde(skip_serializing_if = "Option::is_none")]` stays, so
round-trips through pacquet's own mirror don't emit a synthetic
`"deprecated": false` either.

Caught by running `pacquet install --frozen-lockfile` against the
integrated-benchmark fixture (`pacquet/tasks/integrated-benchmark/src/fixtures/`),
which the picker port commit started failing on CI. With this fix
the install succeeds end-to-end (exit 0).

Three regression tests pinning the wire-shape: `deprecated:false`,
`deprecated:true`, and the regular reason-string case. All three
verified by `cargo nextest run -p pacquet-registry deprecated`.
- `cached_range`: use `entry.value().clone()` to make the
  `dashmap::Ref` → `Option<Arc<Range>>` projection explicit, with a
  comment explaining the auto-deref + Clone the previous form was
  relying on. No behavior change.

- `WantedDependency` doc: clarify that `Default` is only kept so call
  sites can use the `..WantedDependency::default()` spread syntax;
  the at-least-one-of-(alias, bare_specifier) invariant is upheld by
  construction sites, not by the type system, and resolvers walking
  an empty WantedDependency should return `Ok(None)` to let the
  chain fall through to the SpecNotSupported terminal.
@coderabbitai coderabbitai Bot added the area: config dependencies Changes related to configDependencies. label May 20, 2026
Copilot AI review requested due to automatic review settings May 20, 2026 02:10
Dylint's `perfectionist::single-letter-generic` rejected the
conventional serde names (`D` for Deserializer, `E` for Error,
`D2` for the nested Deserializer in `visit_some`) in the custom
`deserialize_deprecated_field` helper. Rename to `Deser`, `Err`,
and `Nested` respectively.

Copilot AI 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.

Pull request overview

Copilot reviewed 27 out of 28 changed files in this pull request and generated no new comments.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants