fix(resolver): refresh metadata during dedupe (#17215)

pnpm dedupe resolves a fresh lockfile without an existing wanted_lockfile
and with prefer_highest_version: true, so locked versions sit in
preferred_versions with VersionPreference::Highest while update remains
UpdateBehavior::Off. Under that combination, MirrorShortcuts::decide
allowed dominant_lockfile_version and DiskFirst::decide allowed reading
the on-disk metadata mirror without a conditional registry request,
leaving stale packument metadata (such as deprecation messages) in place
even when pnpm dedupe --check on a cold cache reported a diff.

Thread refresh_metadata through ResolutionRefreshOptions into
MetadataPickRequest for dedupe runs, and disable unverified on-disk
mirror shortcuts when refresh_metadata is set and the network policy is
not cache-first. Conditional ETag / Last-Modified headers from the mirror
are still sent so unchanged packuments get a 304 response.

Closes pnpm/pnpm#17208.

---------

Co-authored-by: Zoltan Kochan <z@kochan.io>

.changeset/bright-pandas-refresh.md

@@ -0,0 +1,5 @@
+---
+"pacquet": patch
+---
+
+`pnpm dedupe` now refreshes cached registry metadata while re-resolving locked versions [#17208](https://github.com/pnpm/pnpm/issues/17208).

pnpm/crates/cli/tests/suite/dedupe.rs

@@ -6,6 +6,7 @@ use pnpm_testing_utils::{
     command_env::CommandTestExt,
 };
 use std::{fs, path::Path, process::Command};
+use walkdir::WalkDir;
 
 fn pacquet_at(workspace: &Path) -> Command {
     Command::cargo_bin("pnpm")
@@ -14,6 +15,29 @@ fn pacquet_at(workspace: &Path) -> Command {
         .without_ambient_pnpm_config()
 }
 
+fn add_registry_deprecation(storage: &Path, package: &str, version: &str) {
+    WalkDir::new(storage)
+        .into_iter()
+        .filter_map(Result::ok)
+        .filter(|entry| entry.file_type().is_file())
+        .find_map(|entry| {
+            let contents = fs::read(entry.path()).ok()?;
+            let mut packument: serde_json::Value = serde_json::from_slice(&contents).ok()?;
+            if packument.get("name")?.as_str()? != package {
+                return None;
+            }
+            let versions = packument.get_mut("versions")?.as_object_mut()?;
+            let package_version = versions.get_mut(version)?.as_object_mut()?;
+            package_version.insert(
+                "deprecated".to_string(),
+                serde_json::json!("This package is deprecated."),
+            );
+            fs::write(entry.path(), serde_json::to_vec(&packument).ok()?).ok()?;
+            Some(())
+        })
+        .expect("find registry package metadata");
+}
+
 #[test]
 fn dedupe_preserves_auto_installed_peer_with_a_newer_major_in_another_importer() {
     let CommandTempCwd { root, workspace, npmrc_info, .. } =
@@ -194,6 +218,64 @@ fn dedupe_writes_lockfile() {
     drop((root, mock_instance));
 }
 
+#[test]
+fn dedupe_refreshes_cached_registry_metadata_for_locked_versions() {
+    let CommandTempCwd {
+        pacquet,
+        root,
+        workspace,
+        npmrc_info,
+        ..
+    } = CommandTempCwd::init().add_recording_mocked_registry();
+    let AddMockedRegistry { mock_instance, .. } = npmrc_info;
+    mock_instance.remove_version_field("@pnpm.e2e/deprecated", "1.0.0", "deprecated");
+    fs::write(
+        workspace.join("package.json"),
+        serde_json::json!({
+            "dependencies": {
+                "@pnpm.e2e/deprecated": "1.0.0",
+            },
+        })
+        .to_string(),
+    )
+    .expect("write package.json");
+
+    pacquet
+        .with_args(["install", "--lockfile-only"])
+        .assert()
+        .success();
+    let _ = mock_instance.take_requests();
+
+    let lockfile_path = workspace.join("pnpm-lock.yaml");
+    let lockfile = fs::read_to_string(&lockfile_path).expect("read pnpm-lock.yaml");
+    eprintln!("{lockfile}");
+    assert!(!lockfile.contains("deprecated: This package is deprecated."));
+    add_registry_deprecation(
+        &root.path().join("registry-storage"),
+        "@pnpm.e2e/deprecated",
+        "1.0.0",
+    );
+
+    pacquet_at(&workspace)
+        .with_args(["dedupe", "--lockfile-only"])
+        .assert()
+        .success();
+
+    let dedupe_requests = mock_instance.take_requests();
+    dbg!(&dedupe_requests);
+    assert!(
+        dedupe_requests
+            .iter()
+            .any(|request| {
+                request.if_none_match.is_some() || request.if_modified_since.is_some()
+            }),
+    );
+    let deduped_lockfile = fs::read_to_string(&lockfile_path).expect("read deduped lockfile");
+    eprintln!("{deduped_lockfile}");
+    assert!(deduped_lockfile.contains("deprecated: This package is deprecated."));
+    drop((root, mock_instance));
+}
+
 #[test]
 fn dedupe_materializes_node_modules_unless_lockfile_only() {
     let CommandTempCwd {

pnpm/crates/package-manager/src/install_with_fresh_lockfile/resolution.rs

@@ -164,6 +164,10 @@ impl<'a, Reporter: self::Reporter + 'static> ResolutionContext<'a, Reporter> {
             workspace_packages: self.setup.workspace_packages.clone(),
             update_checksums: self.install.update_checksums,
             update_behavior: resolver_update_behavior(&self.owned.resolution.update_seed_policy),
+            refresh_metadata: matches!(
+                self.owned.resolution.update_seed_policy,
+                super::UpdateSeedPolicy::KeepAllResolveAll,
+            ),
         }
     }
 

pnpm/crates/package-manager/src/install_with_fresh_lockfile/resolve.rs

@@ -83,6 +83,7 @@ pub(super) struct SharedResolveOptions<'a> {
     /// See [`super::FreshInputs::update_checksums`].
     pub update_checksums: bool,
     pub update_behavior: pnpm_resolving_resolver_base::UpdateBehavior,
+    pub refresh_metadata: bool,
 }
 
 impl SharedResolveOptions<'_> {
@@ -108,6 +109,7 @@ impl SharedResolveOptions<'_> {
             refresh: pnpm_resolving_resolver_base::ResolutionRefreshOptions {
                 update_checksums: self.update_checksums,
                 update: self.update_behavior,
+                refresh_metadata: self.refresh_metadata,
                 ..Default::default()
             },
             policy: pnpm_resolving_resolver_base::ResolutionPolicyOptions {

pnpm/crates/resolving-npm-resolver/src/pick_package.rs

@@ -174,7 +174,11 @@ pub async fn pick_package<Cache: PackageMetaCache>(
     }
 
     // 2. Offline / pickLowestVersion / preferOffline disk read.
-    let disk_first = match DiskFirst::decide(ctx.cache_policy.network, opts.pick_lowest_version) {
+    let disk_first = match DiskFirst::decide(
+        ctx.cache_policy.network,
+        opts.pick_lowest_version,
+        opts.request.refresh_metadata,
+    ) {
         DiskFirst::Never => false,
         DiskFirst::Always => true,
         DiskFirst::UnlessUncacheable => state.mirror_may_answer(ctx).await,

pnpm/crates/resolving-npm-resolver/src/pick_package/options.rs

@@ -119,8 +119,8 @@ pub struct MetadataPickRequest {
     /// the install is a pure dry-run (`--lockfile-only`, frozen
     /// lockfile, etc.).
     pub dry_run: bool,
-    /// `pnpm update` must see versions published since the mirror was
-    /// written, so it does not reuse an ETag-less mirror.
+    /// Revalidates the metadata mirror against the registry instead of
+    /// serving from unverified mirror shortcuts.
     pub refresh_metadata: bool,
     /// `true` forces this pick to use the full packument because
     /// the dependency carries `optionalDependencies`-specific

pnpm/crates/resolving-npm-resolver/src/pick_package/route.rs

@@ -97,6 +97,9 @@ impl MirrorShortcuts {
         if !mirror_may_answer(network, mirror) {
             return None;
         }
+        if request.refresh_metadata && !network.reads_cache_first() {
+            return None;
+        }
         let stable_range = inputs.range_pick_is_stable();
         let mirror_has_no_etag =
             mirror.is_some_and(|headers| headers.etag.as_deref().is_none_or(str::is_empty));
@@ -105,10 +108,7 @@ impl MirrorShortcuts {
                 && !request.include_latest_tag
                 && !request.update_checksums,
             dominant_lockfile_version: stable_range,
-            unvalidated_mirror: stable_range
-                && !request.refresh_metadata
-                && public_scope
-                && mirror_has_no_etag,
+            unvalidated_mirror: stable_range && public_scope && mirror_has_no_etag,
         })
     }
 }
@@ -135,6 +135,9 @@ impl CacheHitCheck {
         if network.forbids_registry() || registry_verified {
             return CacheHitCheck::Serve;
         }
+        if request.refresh_metadata && !network.prefers_reachable_cache() {
+            return CacheHitCheck::Revalidate;
+        }
         if !picked {
             return CacheHitCheck::Revalidate;
         }
@@ -176,9 +179,15 @@ pub(super) enum DiskFirst {
 }
 
 impl DiskFirst {
-    pub(super) fn decide(network: NetworkPolicy, pick_lowest_version: bool) -> Self {
+    pub(super) fn decide(
+        network: NetworkPolicy,
+        pick_lowest_version: bool,
+        refresh_metadata: bool,
+    ) -> Self {
         if network.reads_cache_first() {
             DiskFirst::Always
+        } else if refresh_metadata {
+            DiskFirst::Never
         } else if pick_lowest_version {
             DiskFirst::UnlessUncacheable
         } else {

pnpm/crates/resolving-npm-resolver/src/pick_package/route/tests.rs

@@ -120,7 +120,7 @@ fn a_package_excluded_from_the_release_age_keeps_the_range_shortcuts() {
 }
 
 #[test]
-fn only_a_public_mirror_without_an_etag_skips_revalidation() {
+fn an_unvalidated_mirror_skips_cache_shortcut_when_metadata_must_be_refreshed() {
     let untagged = headers(None, false);
     let empty_etag = headers(Some(""), false);
     let tagged = headers(Some(r#""etag""#), false);
@@ -134,18 +134,15 @@ fn only_a_public_mirror_without_an_etag_skips_revalidation() {
             RouteInputs { public_scope: false, ..range_pick(&NO_POLICY, Some(&untagged)) },
             false,
         ),
-        (
-            "pnpm update",
-            with_request(range_pick(&NO_POLICY, Some(&untagged)), |request| {
-                request.refresh_metadata = true;
-            }),
-            false,
-        ),
     ];
     for (name, inputs, unvalidated_mirror) in cases {
         let shortcuts = MirrorShortcuts::decide(&inputs).expect("the mirror may answer");
         assert_eq!(shortcuts.unvalidated_mirror, unvalidated_mirror, "{name}");
     }
+    let refresh = with_request(range_pick(&NO_POLICY, Some(&untagged)), |request| {
+        request.refresh_metadata = true;
+    });
+    assert_eq!(MirrorShortcuts::decide(&refresh), None);
 }
 
 #[test]
@@ -157,12 +154,21 @@ fn a_pinned_version_may_be_read_from_the_mirror_unless_fresh_metadata_is_needed(
         ("pick lowest", with_request(base, |request| request.pick_lowest_version = true), true),
         ("latest tag", with_request(base, |request| request.include_latest_tag = true), false),
         ("update checksums", with_request(base, |request| request.update_checksums = true), false),
+        (
+            "offline refresh metadata",
+            with_request(RouteInputs { network: NetworkPolicy::Offline, ..base }, |request| {
+                request.refresh_metadata = true;
+            }),
+            true,
+        ),
         (
             "tag",
             with_request(base, |request| request.spec_type = RegistryPackageSpecType::Tag),
             false,
         ),
     ];
+    let refresh = with_request(base, |request| request.refresh_metadata = true);
+    assert_eq!(MirrorShortcuts::decide(&refresh), None);
     for (name, inputs, exact_version) in cases {
         let shortcuts = MirrorShortcuts::decide(&inputs).expect("the mirror may answer");
         assert_eq!(shortcuts.exact_version, exact_version, "{name}");
@@ -228,18 +234,67 @@ fn an_unvalidated_mirror_is_fresh_within_its_max_age_either_way() {
 
 #[test]
 fn the_disk_is_read_first_when_the_cache_comes_first_or_the_pick_is_lowest() {
+    struct Case {
+        network: NetworkPolicy,
+        pick_lowest_version: bool,
+        refresh_metadata: bool,
+        disk_first: DiskFirst,
+    }
     let cases = [
-        (NetworkPolicy::Online, false, DiskFirst::Never),
-        (NetworkPolicy::Online, true, DiskFirst::UnlessUncacheable),
-        (NetworkPolicy::PreferCache, false, DiskFirst::Always),
-        (NetworkPolicy::PreferCache, true, DiskFirst::Always),
-        (NetworkPolicy::Offline, false, DiskFirst::Always),
+        Case {
+            network: NetworkPolicy::Online,
+            pick_lowest_version: false,
+            refresh_metadata: false,
+            disk_first: DiskFirst::Never,
+        },
+        Case {
+            network: NetworkPolicy::Online,
+            pick_lowest_version: true,
+            refresh_metadata: false,
+            disk_first: DiskFirst::UnlessUncacheable,
+        },
+        Case {
+            network: NetworkPolicy::Online,
+            pick_lowest_version: true,
+            refresh_metadata: true,
+            disk_first: DiskFirst::Never,
+        },
+        Case {
+            network: NetworkPolicy::PreferCache,
+            pick_lowest_version: false,
+            refresh_metadata: false,
+            disk_first: DiskFirst::Always,
+        },
+        Case {
+            network: NetworkPolicy::PreferCache,
+            pick_lowest_version: true,
+            refresh_metadata: false,
+            disk_first: DiskFirst::Always,
+        },
+        Case {
+            network: NetworkPolicy::PreferCache,
+            pick_lowest_version: true,
+            refresh_metadata: true,
+            disk_first: DiskFirst::Always,
+        },
+        Case {
+            network: NetworkPolicy::Offline,
+            pick_lowest_version: false,
+            refresh_metadata: false,
+            disk_first: DiskFirst::Always,
+        },
     ];
-    for (network, pick_lowest_version, disk_first) in cases {
+    for Case {
+        network,
+        pick_lowest_version,
+        refresh_metadata,
+        disk_first,
+    } in cases
+    {
         assert_eq!(
-            DiskFirst::decide(network, pick_lowest_version),
+            DiskFirst::decide(network, pick_lowest_version, refresh_metadata),
             disk_first,
-            "network={network:?} pick_lowest_version={pick_lowest_version}",
+            "network={network:?} pick_lowest_version={pick_lowest_version} refresh_metadata={refresh_metadata}",
         );
     }
 }
@@ -299,6 +354,13 @@ fn an_unverified_cache_hit_serves_only_a_pick_a_stale_mirror_cannot_change() {
             CacheHitCheck::Serve,
         ),
         ("version", version_pick(&release_age, None), CacheHitCheck::Serve),
+        (
+            "refresh metadata on a version",
+            with_request(version_pick(&NO_POLICY, None), |request| {
+                request.refresh_metadata = true;
+            }),
+            CacheHitCheck::Revalidate,
+        ),
         ("stable range", base, CacheHitCheck::StableRangePick),
         (
             "latest tag",
@@ -310,6 +372,11 @@ fn an_unverified_cache_hit_serves_only_a_pick_a_stale_mirror_cannot_change() {
             with_request(base, |request| request.update_checksums = true),
             CacheHitCheck::Revalidate,
         ),
+        (
+            "refresh metadata",
+            with_request(base, |request| request.refresh_metadata = true),
+            CacheHitCheck::Revalidate,
+        ),
         ("release age", RouteInputs { policy: &release_age, ..base }, CacheHitCheck::Revalidate),
         ("no-downgrade", RouteInputs { policy: &no_downgrade, ..base }, CacheHitCheck::Revalidate),
         (

pnpm/crates/resolving-resolver-base/src/resolve.rs

@@ -432,6 +432,7 @@ pub struct ResolutionRefreshOptions {
     /// [`Self::current_pkg`] as it is.
     pub repick_current_version: bool,
     pub update: UpdateBehavior,
+    pub refresh_metadata: bool,
     /// True only when this specific package matches the user's update
     /// target (e.g. `pnpm up <name>`). Unlike `update`, this is false for
     /// unrelated packages that get re-resolved as a side effect of an
@@ -451,12 +452,12 @@ pub struct ResolutionRefreshOptions {
 }
 
 impl ResolutionRefreshOptions {
-    /// Whether the resolution must see versions published since the
-    /// metadata mirror was written: an update is running, or this package
-    /// is its target.
+    /// Whether the resolution must revalidate cached metadata against the
+    /// registry: an update or deduplication is running, or this package is
+    /// its target.
     #[must_use]
     pub fn refreshes_metadata(&self) -> bool {
-        self.update != UpdateBehavior::Off || self.update_requested
+        self.update != UpdateBehavior::Off || self.update_requested || self.refresh_metadata
     }
 
     /// The locked runtime a runtime resolver returns as it is: a