fix(deps-resolver): close two Astro lockfile divergences (#13374)

Resolving the Astro workspace with pacquet produced a `pnpm-lock.yaml`
that differed from pnpm 11's by 306 lines. Two independent bugs account
for 143 of them.

`dedupe_injected_deps` splits a resolved package id into its name and
its `file:` payload on the first `@`. A scoped id
(`@test/pkg@file:packages/…`) starts with `@`, so the split returned the
scope tail and the `file:` strip failed — the injected workspace dep was
never recognized and stayed a `file:` snapshot where pnpm records
`link:`. Split on the whole `@file:` separator instead.

`extract_children` walked every entry of `dependencies` and
`optionalDependencies`, including the ones the manifest declares as
bundled. npm ships those inside the package's own tarball, and pnpm's
`getNonDevWantedDependencies` filters them out before resolution; the
missing filter pulled extra packages into the lockfile (`napi-wasm`
under `@parcel/watcher-wasm`, the whole `@napi-rs`/`@emnapi` set under
`@tailwindcss/oxide-wasm32-wasi`). Both spellings are honored, and
`true` means every entry of `dependencies`, matching npm. The bundled
set is computed once and shared by both `collect_deps` calls, so an
alias declared in both groups is dropped from both — upstream filters
the merged `{...optionalDependencies, ...dependencies}` map and reaches
the same result. The alias validation still runs over bundled entries
so an unusable alias is rejected either way, as it is upstream.

A third divergence from the same reproduction — a project's own
`peerDependencies` reaching its lockfile importer entry without
`autoInstallPeers` — landed separately as pnpm/pnpm#13372 while this was
in review, so it is no longer part of this change.

The remaining divergences — the `parentPkgAliases` arm of upstream's
peer-shadowed-dependency omission, and local `file:*.tgz` tarballs
resolving without a manifest — are described in the issue; both need
design work beyond this change.

Related to pnpm/pnpm#13334.

.changeset/astro-lockfile-resolution-parity.md

@@ -0,0 +1,8 @@
+---
+"pacquet": patch
+---
+
+Two `pnpm install` resolution fixes that made large workspaces such as [Astro](https://github.com/withastro/astro) produce a different `pnpm-lock.yaml` than pnpm 11 [#13334](https://github.com/pnpm/pnpm/issues/13334):
+
+- A scoped workspace package referenced through the `file:` protocol (`"@test/pkg": "file:./pkg"`) is recorded as a `link:` again instead of being copied in as a `file:` snapshot.
+- `bundledDependencies` / `bundleDependencies` are no longer resolved as dependencies of their own. npm ships them inside the package's tarball, so installing them again added packages the lockfile should not contain (for example `napi-wasm` under `@parcel/watcher-wasm`).

pnpm/crates/resolving-deps-resolver/src/dedupe_injected_deps.rs

@@ -129,16 +129,17 @@ fn child_matches_target(
 /// The resolver's `pkg.id` for a `file:<path>` workspace pick is
 /// emitted as `<name>@file:<path>` once the manifest name is in scope
 /// (see `build_pkg_id_with_patch_hash`) and as the bare `file:<path>`
-/// before that — accept both shapes.
+/// before that — accept both shapes. Splitting on the whole `@file:`
+/// separator rather than on `@` alone keeps scoped names
+/// (`@scope/name@file:<path>`) matching, whose leading `@` would
+/// otherwise be taken for the separator.
 fn injected_workspace_target(
     node: &crate::dependencies_graph::DependenciesGraphNode,
     workspace_project_ids: &HashSet<String>,
 ) -> Option<String> {
     let raw = node.resolved_package_id.as_str();
-    let path = raw.strip_prefix("file:").or_else(|| {
-        let after_at = raw.split_once('@').map(|(_, rest)| rest)?;
-        after_at.strip_prefix("file:")
-    })?;
+    let path =
+        raw.strip_prefix("file:").or_else(|| raw.split_once("@file:").map(|(_, path)| path))?;
     workspace_project_ids.contains(path).then(|| path.to_string())
 }
 

pnpm/crates/resolving-deps-resolver/src/dedupe_injected_deps/tests.rs

@@ -63,6 +63,36 @@ fn rewrites_childless_injected_dep_to_link() {
     assert!(graph.is_empty(), "unreachable file: snapshot should be pruned");
 }
 
+// Regression test for pnpm/pnpm#13334: the resolver prefixes a `file:`
+// workspace pick with the manifest name, so a scoped package's id reads
+// `@scope/name@file:<path>`. Its leading `@` must not be mistaken for the
+// name/id separator, or the dep never dedupes back to `link:`.
+#[test]
+fn rewrites_scoped_injected_dep_to_link() {
+    let lockfile_dir = PathBuf::from("/ws");
+    let host_root = lockfile_dir.join("fixtures/host");
+    let pkg_root = host_root.join("pkg");
+
+    let mut graph: DependenciesGraph = std::collections::HashMap::new();
+    let injected = DepPath::from("@test/pkg@file:fixtures/host/pkg".to_string());
+    graph.insert(injected.clone(), make_node("@test/pkg@file:fixtures/host/pkg", BTreeMap::new()));
+
+    let mut direct: DirectByImporter = BTreeMap::new();
+    direct
+        .insert("fixtures/host".to_string(), BTreeMap::from([("@test/pkg".to_string(), injected)]));
+    direct.insert("fixtures/host/pkg".to_string(), BTreeMap::new());
+
+    let mut roots = BTreeMap::new();
+    roots.insert("fixtures/host".to_string(), host_root);
+    roots.insert("fixtures/host/pkg".to_string(), pkg_root);
+
+    dedupe_injected_deps(&mut graph, &mut direct, &roots, &lockfile_dir);
+
+    let after = direct.get("fixtures/host").unwrap().get("@test/pkg").unwrap();
+    assert_eq!(after.as_str(), "link:pkg");
+    assert!(graph.is_empty(), "unreachable file: snapshot should be pruned");
+}
+
 #[test]
 fn leaves_injected_dep_when_children_differ() {
     let lockfile_dir = PathBuf::from("/ws");

pnpm/crates/resolving-deps-resolver/src/resolve_dependency_tree.rs

@@ -3064,25 +3064,52 @@ fn render_specifier(wanted: &WantedDependency) -> String {
 /// `optionalDependencies`. The walker propagates this through
 /// `current_is_optional` so [`ResolvedPackage::optional`] reflects
 /// whether every path to the node went through an optional edge.
+///
+/// Names the manifest bundles are dropped: npm ships them inside the
+/// package's own tarball, so resolving them again would install a
+/// second copy the package never loads.
 fn extract_children(
     result: &pacquet_resolving_resolver_base::ResolveResult,
 ) -> Result<Vec<ChildSpec>, ResolveDependencyTreeError> {
     let Some(manifest) = result.manifest.as_ref() else { return Ok(Vec::new()) };
     let parent = render_parent(result);
+    let bundled = bundled_dependency_names(manifest);
     let mut out = Vec::new();
-    collect_deps(manifest, "dependencies", false, &parent, &mut out)?;
-    collect_deps(manifest, "optionalDependencies", true, &parent, &mut out)?;
+    collect_deps(manifest, "dependencies", false, &parent, &bundled, &mut out)?;
+    collect_deps(manifest, "optionalDependencies", true, &parent, &bundled, &mut out)?;
     for (name, specifier) in engines_runtime_dependencies(manifest, "engines", "dependencies") {
         out.push((name.to_string(), specifier, false));
     }
     Ok(out)
 }
 
+/// The dependency names a manifest declares as bundled, read from
+/// `bundledDependencies` with `bundleDependencies` as the fallback
+/// spelling. `true` stands for "every entry in `dependencies`".
+fn bundled_dependency_names(manifest: &Value) -> HashSet<&str> {
+    let bundled = ["bundledDependencies", "bundleDependencies"]
+        .into_iter()
+        .find_map(|key| manifest.get(key).filter(|value| !value.is_null()));
+    match bundled {
+        Some(Value::Bool(true)) => manifest
+            .get("dependencies")
+            .and_then(Value::as_object)
+            .map(|map| map.keys().map(String::as_str).collect())
+            .unwrap_or_default(),
+        Some(Value::Array(names)) => names.iter().filter_map(Value::as_str).collect(),
+        _ => HashSet::new(),
+    }
+}
+
+/// Dependency names are validated before the bundled filter runs, so a
+/// manifest with an unusable alias is rejected whether or not the
+/// package bundles that alias.
 fn collect_deps(
     manifest: &Value,
     key: &str,
     optional: bool,
     parent: &str,
+    bundled: &HashSet<&str>,
     out: &mut Vec<ChildSpec>,
 ) -> Result<(), ResolveDependencyTreeError> {
     let Some(map) = manifest.get(key).and_then(Value::as_object) else { return Ok(()) };
@@ -3094,6 +3121,9 @@ fn collect_deps(
                     alias: name.clone(),
                 });
             }
+            if bundled.contains(name.as_str()) {
+                continue;
+            }
             out.push((name.clone(), range_str.to_string(), optional));
         }
     }

pnpm/crates/resolving-deps-resolver/src/resolve_dependency_tree/tests.rs

@@ -41,6 +41,88 @@ fn dependency_engines_runtime_is_walked_as_a_runtime_dependency() {
     );
 }
 
+fn manifest_result(manifest: serde_json::Value) -> ResolveResult {
+    ResolveResult {
+        id: PkgResolutionId::from("parent@1.0.0"),
+        name_ver: None,
+        latest: None,
+        published_at: None,
+        manifest: Some(std::sync::Arc::new(manifest)),
+        resolution: LockfileResolution::Directory(DirectoryResolution {
+            directory: "parent".to_string(),
+        }),
+        resolved_via: "npm-registry".to_string(),
+        normalized_bare_specifier: None,
+        alias: Some("parent".to_string()),
+        policy_violation: None,
+    }
+}
+
+// Regression test for pnpm/pnpm#13334: npm ships bundled dependencies
+// inside the package's own tarball, so they must not be resolved as
+// edges of their own.
+#[test]
+fn bundled_dependencies_are_not_walked() {
+    let result = manifest_result(serde_json::json!({
+        "name": "parent",
+        "version": "1.0.0",
+        "dependencies": { "bundled-dep": "^1.0.0", "regular-dep": "^2.0.0" },
+        "optionalDependencies": { "bundled-optional": "^3.0.0" },
+        "bundledDependencies": ["bundled-dep", "bundled-optional"],
+    }));
+    assert_eq!(
+        extract_children(&result).unwrap(),
+        vec![("regular-dep".to_string(), "^2.0.0".to_string(), false)],
+    );
+}
+
+#[test]
+fn bundle_dependencies_spelling_is_honored() {
+    let result = manifest_result(serde_json::json!({
+        "name": "parent",
+        "version": "1.0.0",
+        "dependencies": { "bundled-dep": "^1.0.0", "regular-dep": "^2.0.0" },
+        "bundleDependencies": ["bundled-dep"],
+    }));
+    assert_eq!(
+        extract_children(&result).unwrap(),
+        vec![("regular-dep".to_string(), "^2.0.0".to_string(), false)],
+    );
+}
+
+#[test]
+fn bundled_dependencies_true_bundles_every_dependency() {
+    let result = manifest_result(serde_json::json!({
+        "name": "parent",
+        "version": "1.0.0",
+        "dependencies": { "one": "^1.0.0", "two": "^2.0.0" },
+        "optionalDependencies": { "three": "^3.0.0" },
+        "bundledDependencies": true,
+    }));
+    assert_eq!(
+        extract_children(&result).unwrap(),
+        vec![("three".to_string(), "^3.0.0".to_string(), true)],
+    );
+}
+
+// `bundledDependencies: true` names the `dependencies` keys, and upstream
+// filters the merged `{...optionalDependencies, ...dependencies}` map, so an
+// alias listed in both groups is dropped from both.
+#[test]
+fn bundled_dependencies_true_also_drops_the_optional_duplicate() {
+    let result = manifest_result(serde_json::json!({
+        "name": "parent",
+        "version": "1.0.0",
+        "dependencies": { "both": "^1.0.0" },
+        "optionalDependencies": { "both": "^1.0.0", "optional-only": "^3.0.0" },
+        "bundledDependencies": true,
+    }));
+    assert_eq!(
+        extract_children(&result).unwrap(),
+        vec![("optional-only".to_string(), "^3.0.0".to_string(), true)],
+    );
+}
+
 fn key(raw: &str) -> PkgNameVerPeer {
     raw.parse().expect("parse snapshot key")
 }