fix(package-manager): link optionalDependencies siblings into slot (#526)

`create_symlink_layout` only iterated `snapshot.dependencies`, so a
package whose CPU/OS-specific siblings live entirely under
`optionalDependencies` (e.g. `@typescript/native-preview`,
`@reflink/reflink`, every `*-darwin-arm64` / `*-linux-x64` family)
ended up with a slot `node_modules/<scope>/` containing only the
parent package — no platform binary sibling. Consumers that do
`require.resolve('@typescript/native-preview-darwin-arm64')` from
inside `getExePath.js` walked parent directories and found nothing,
so `tsgo --version` (and every other tool that delegates to a
platform variant) crashed with `Unable to resolve … missing the
package on disk`.

Port upstream's `dependencies ∪ optionalDependencies` merge — the
graph builder at
https://github.com/pnpm/pnpm/blob/da65e6262/deps/graph-builder/src/lockfileToDepGraph.ts#L150-L156
unifies both maps into one `allDeps` for each node's children, and
`linkAllModules` then symlinks every child with two short-circuits:
`alias === depNode.name` (a snapshot referencing itself) and
`!pkg.installable && pkg.optional` (a non-materialized optional).
See https://github.com/pnpm/pnpm/blob/da65e6262/installing/deps-installer/src/install/link.ts#L521-L549.

`create_symlink_layout` now takes both maps and a `SkippedSnapshots`
reference, merges them, and applies both short-circuits. The skip
set is threaded through `InstallPackageBySnapshot` (cold batch) and
the warm-batch `CreateVirtualDirBySnapshot` call site in
`CreateVirtualStore::run`, so a target dropped by the installability
pass, by `--no-optional`, or by a swallowed optional fetch failure
gets no dangling symlink.

Five new unit tests in `create_symlink_layout/tests.rs` cover the
matching-optional happy path, the skipped-optional dangling-link
guard, the self-name guard for entries listed in either bucket, the
both-buckets-absent no-op, and the alias-resolve path (aliased deps
still link the alias filename while resolving the slot via the
target's name). End-to-end verification: `pacquet install
--frozen-lockfile` followed by `tsgo --version` in the pnpm v11
repo now succeeds; the matching `native-preview-darwin-arm64`
sibling shows up in the slot's `node_modules/@typescript/`.

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

pacquet/crates/package-manager/src/create_symlink_layout.rs

@@ -1,37 +1,69 @@
-use crate::{SymlinkPackageError, VirtualStoreLayout, symlink_package};
+use crate::{SkippedSnapshots, SymlinkPackageError, VirtualStoreLayout, symlink_package};
 use pacquet_lockfile::{PkgName, SnapshotDepRef};
 use std::{collections::HashMap, path::Path};
 
 /// Create symlink layout of dependencies for a package in a virtual dir.
 ///
-/// For npm-aliased dependencies (e.g. `string-width-cjs: string-width@4.2.3`),
-/// the symlink filename under `node_modules/` uses the entry key (the alias),
-/// while the virtual-store lookup uses the aliased target.
+/// Mirrors upstream's `linkAllModules` child-selection rules at
+/// <https://github.com/pnpm/pnpm/blob/f2981a316/installing/deps-installer/src/install/link.ts#L521-L549>
+/// and the underlying `dependencies` ∪ `optionalDependencies` merge in
+/// `lockfileToDepGraph` at
+/// <https://github.com/pnpm/pnpm/blob/f2981a316/deps/graph-builder/src/lockfileToDepGraph.ts#L150-L156>.
 ///
-/// Child target paths come from the install-scoped [`VirtualStoreLayout`]:
-/// `layout.slot_dir(&target)` returns either
+/// For each entry in `dependencies ∪ optional_dependencies`:
+/// - Skip when the alias matches the slot's own package name
+///   (`alias === depNode.name`), so a package that lists itself as
+///   a dep doesn't get a circular self-link inside its own slot.
+/// - Skip when the target snapshot is in `skipped` — its slot was
+///   never materialized (platform-mismatched optional, `--no-optional`
+///   excluded, or a swallowed optional fetch failure). Mirrors
+///   upstream's `!pkg || (!pkg.installable && pkg.optional)` guard.
+/// - Otherwise create a symlink at
+///   `<virtual_node_modules_dir>/<alias>` pointing to the target's
+///   slot under `<layout.slot_dir(target)>/node_modules/<target-name>`.
+///
+/// For npm-aliased dependencies (e.g.
+/// `string-width-cjs: string-width@4.2.3`), the symlink filename
+/// under `node_modules/` uses the entry key (the alias), while the
+/// virtual-store lookup uses the aliased target.
+///
+/// Child target paths come from the install-scoped
+/// [`VirtualStoreLayout`]: `layout.slot_dir(&target)` returns either
 /// `<virtual_store_dir>/<flat-name>` (legacy) or
-/// `<global_virtual_store_dir>/<scope>/<name>/<version>/<hash>` (GVS), so
-/// the caller doesn't have to branch on which mode is in effect.
+/// `<global_virtual_store_dir>/<scope>/<name>/<version>/<hash>` (GVS),
+/// so the caller doesn't have to branch on which mode is in effect.
 ///
-/// `virtual_node_modules_dir` does not have to exist — `symlink_package` calls
-/// `fs::create_dir_all` on the symlink path's parent before each link. Callers
-/// that already know the directory exists (e.g. `CreateVirtualStore::run`,
-/// which `mkdir`s it just before calling this function) just pay redundant
-/// stat syscalls, which is cheap and matches pnpm's own redundant-mkdir shape.
+/// `virtual_node_modules_dir` does not have to exist —
+/// `symlink_package` calls `fs::create_dir_all` on the symlink path's
+/// parent before each link. Callers that already know the directory
+/// exists (e.g. `CreateVirtualStore::run`, which `mkdir`s it just
+/// before calling this function) just pay redundant stat syscalls,
+/// which is cheap and matches pnpm's own redundant-mkdir shape.
 pub fn create_symlink_layout(
-    dependencies: &HashMap<PkgName, SnapshotDepRef>,
+    dependencies: Option<&HashMap<PkgName, SnapshotDepRef>>,
+    optional_dependencies: Option<&HashMap<PkgName, SnapshotDepRef>>,
+    self_name: &PkgName,
+    skipped: &SkippedSnapshots,
     layout: &VirtualStoreLayout,
     virtual_node_modules_dir: &Path,
 ) -> Result<(), SymlinkPackageError> {
-    // Serial iteration: the symlink work per snapshot is small (a handful of
-    // entries), so fanning out to rayon here would just add task-scheduling
-    // overhead without a wider work queue to amortise it against. The
-    // single-caller policy upstream is to run this stage single-threaded on a
-    // `spawn_blocking` worker (see `CreateVirtualStore::run`), mirroring
-    // pnpm's `symlinkAllModules` in `worker/src/start.ts`.
-    dependencies.iter().try_for_each(|(alias_name, dep_ref)| {
+    // Serial iteration: the symlink work per snapshot is small (a
+    // handful of entries), so fanning out to rayon here would just add
+    // task-scheduling overhead without a wider work queue to amortise
+    // it against. The single-caller policy upstream is to run this
+    // stage single-threaded on a `spawn_blocking` worker (see
+    // `CreateVirtualStore::run`), mirroring pnpm's `symlinkAllModules`
+    // in `worker/src/start.ts`.
+    let deps = dependencies.into_iter().flatten();
+    let opt_deps = optional_dependencies.into_iter().flatten();
+    deps.chain(opt_deps).try_for_each(|(alias_name, dep_ref)| {
+        if alias_name == self_name {
+            return Ok(());
+        }
         let target = dep_ref.resolve(alias_name);
+        if skipped.contains(&target) {
+            return Ok(());
+        }
         let target_name_str = target.name.to_string();
         let alias_name_str = alias_name.to_string();
         symlink_package(
@@ -40,3 +72,6 @@ pub fn create_symlink_layout(
         )
     })
 }
+
+#[cfg(test)]
+mod tests;

pacquet/crates/package-manager/src/create_symlink_layout/tests.rs

@@ -0,0 +1,229 @@
+use crate::{SkippedSnapshots, VirtualStoreLayout, create_symlink_layout};
+use pacquet_lockfile::{PackageKey, PkgName, SnapshotDepRef};
+use pretty_assertions::assert_eq;
+use std::{collections::HashMap, fs, path::PathBuf};
+use tempfile::tempdir;
+
+fn pkg_name(input: &str) -> PkgName {
+    PkgName::parse(input).expect("valid pkg name")
+}
+
+fn dep_ref(input: &str) -> SnapshotDepRef {
+    input.parse().expect("valid snapshot dep ref")
+}
+
+/// A symlink in the slot's `node_modules` matches the alias and points
+/// to `<layout.slot_dir(target)>/node_modules/<target-name>`. Trivial
+/// path-shape assertion that anchors the rest of the test cases.
+fn assert_symlink_shape(
+    virtual_node_modules_dir: &std::path::Path,
+    alias: &str,
+    layout: &VirtualStoreLayout,
+    target_key: &PackageKey,
+) {
+    let symlink_path = virtual_node_modules_dir.join(alias);
+    let read = fs::read_link(&symlink_path)
+        .unwrap_or_else(|err| panic!("read_link {symlink_path:?}: {err}"));
+    let expected =
+        layout.slot_dir(target_key).join("node_modules").join(target_key.name.to_string());
+    assert_eq!(read, expected);
+}
+
+/// `optionalDependencies` siblings whose target slot is **not** in
+/// `skipped` get linked alongside the regular `dependencies` siblings.
+/// This is the v11 install path: a snapshot like
+/// `@typescript/native-preview` lists every platform variant under
+/// `optionalDependencies`, and the installability pass leaves the
+/// host-matching variant out of `skipped`. Without this, downstream
+/// `getExePath`-style lookups fail because the matching binary slot
+/// is missing from the consumer's slot `node_modules`.
+#[test]
+fn links_matching_optional_sibling_alongside_regular_deps() {
+    let tmp = tempdir().expect("tempdir");
+    let virtual_store_dir = tmp.path().to_path_buf();
+    let layout = VirtualStoreLayout::legacy(virtual_store_dir.clone());
+
+    let mut deps: HashMap<PkgName, SnapshotDepRef> = HashMap::new();
+    deps.insert(pkg_name("plain-dep"), dep_ref("1.0.0"));
+
+    let mut optional: HashMap<PkgName, SnapshotDepRef> = HashMap::new();
+    optional.insert(pkg_name("matching-optional"), dep_ref("2.0.0"));
+
+    let skipped = SkippedSnapshots::default();
+
+    let virtual_node_modules_dir = tmp.path().join("self/node_modules");
+    fs::create_dir_all(&virtual_node_modules_dir).unwrap();
+
+    create_symlink_layout(
+        Some(&deps),
+        Some(&optional),
+        &pkg_name("self"),
+        &skipped,
+        &layout,
+        &virtual_node_modules_dir,
+    )
+    .expect("create_symlink_layout should succeed");
+
+    assert_symlink_shape(
+        &virtual_node_modules_dir,
+        "plain-dep",
+        &layout,
+        &"plain-dep@1.0.0".parse().unwrap(),
+    );
+    assert_symlink_shape(
+        &virtual_node_modules_dir,
+        "matching-optional",
+        &layout,
+        &"matching-optional@2.0.0".parse().unwrap(),
+    );
+}
+
+/// Optional siblings whose target snapshot is in `skipped` are
+/// *not* linked — their slot was never materialized, so a symlink
+/// would dangle. Mirrors upstream's `!pkg.installable && pkg.optional`
+/// short-circuit at
+/// <https://github.com/pnpm/pnpm/blob/f2981a316/installing/deps-installer/src/install/link.ts#L540>.
+#[test]
+fn skips_optional_siblings_that_are_in_skipped() {
+    let tmp = tempdir().expect("tempdir");
+    let virtual_store_dir = tmp.path().to_path_buf();
+    let layout = VirtualStoreLayout::legacy(virtual_store_dir);
+
+    let mut optional: HashMap<PkgName, SnapshotDepRef> = HashMap::new();
+    optional.insert(pkg_name("matching-optional"), dep_ref("2.0.0"));
+    optional.insert(pkg_name("mismatched-optional"), dep_ref("3.0.0"));
+
+    let mut skipped_set = std::collections::HashSet::<PackageKey>::new();
+    skipped_set.insert("mismatched-optional@3.0.0".parse().unwrap());
+    let skipped = SkippedSnapshots::from_set(skipped_set);
+
+    let virtual_node_modules_dir = tmp.path().join("self/node_modules");
+    fs::create_dir_all(&virtual_node_modules_dir).unwrap();
+
+    create_symlink_layout(
+        None,
+        Some(&optional),
+        &pkg_name("self"),
+        &skipped,
+        &layout,
+        &virtual_node_modules_dir,
+    )
+    .expect("create_symlink_layout should succeed");
+
+    // `symlink_metadata` reports the link itself, not the target —
+    // crucial for this assertion because the slot the link points to
+    // is never created in this test (the symlink is intentionally
+    // dangling). `Path::exists()` would follow the link and return
+    // false despite the link existing.
+    assert!(
+        fs::symlink_metadata(virtual_node_modules_dir.join("matching-optional"))
+            .is_ok_and(|m| m.is_symlink()),
+        "matching optional sibling must be linked",
+    );
+    assert!(
+        fs::symlink_metadata(virtual_node_modules_dir.join("mismatched-optional")).is_err(),
+        "skipped optional sibling must not be linked (would dangle)",
+    );
+}
+
+/// A dep whose alias matches the slot's own package name is a
+/// self-link that Node's resolver doesn't need (and upstream
+/// explicitly excludes via `alias === depNode.name` at
+/// <https://github.com/pnpm/pnpm/blob/f2981a316/installing/deps-installer/src/install/link.ts#L540>).
+/// Tests both buckets — `dependencies` and `optionalDependencies` —
+/// because either can list the self-name in the wild.
+#[test]
+fn skips_dep_entries_whose_alias_matches_self_name() {
+    let tmp = tempdir().expect("tempdir");
+    let virtual_store_dir = tmp.path().to_path_buf();
+    let layout = VirtualStoreLayout::legacy(virtual_store_dir);
+
+    let mut deps: HashMap<PkgName, SnapshotDepRef> = HashMap::new();
+    deps.insert(pkg_name("self"), dep_ref("1.0.0"));
+
+    let mut optional: HashMap<PkgName, SnapshotDepRef> = HashMap::new();
+    optional.insert(pkg_name("self"), dep_ref("1.0.0"));
+
+    let skipped = SkippedSnapshots::default();
+    let virtual_node_modules_dir = tmp.path().join("self/node_modules");
+    fs::create_dir_all(&virtual_node_modules_dir).unwrap();
+
+    create_symlink_layout(
+        Some(&deps),
+        Some(&optional),
+        &pkg_name("self"),
+        &skipped,
+        &layout,
+        &virtual_node_modules_dir,
+    )
+    .expect("create_symlink_layout should succeed");
+
+    let entries: Vec<PathBuf> = fs::read_dir(&virtual_node_modules_dir)
+        .unwrap()
+        .map(|entry| entry.unwrap().path())
+        .collect();
+    assert!(entries.is_empty(), "self-named entries must not become symlinks; got {entries:?}");
+}
+
+/// Both `dependencies` and `optional_dependencies` absent is a
+/// no-op — the empty-snapshot fast path matches what the legacy
+/// `Option<&HashMap>` signature used to do for `dependencies = None`.
+#[test]
+fn both_dep_maps_absent_is_a_noop() {
+    let tmp = tempdir().expect("tempdir");
+    let virtual_store_dir = tmp.path().to_path_buf();
+    let layout = VirtualStoreLayout::legacy(virtual_store_dir);
+    let skipped = SkippedSnapshots::default();
+    let virtual_node_modules_dir = tmp.path().join("self/node_modules");
+    fs::create_dir_all(&virtual_node_modules_dir).unwrap();
+
+    create_symlink_layout(
+        None,
+        None,
+        &pkg_name("self"),
+        &skipped,
+        &layout,
+        &virtual_node_modules_dir,
+    )
+    .expect("create_symlink_layout should succeed with no deps");
+
+    let entries: Vec<_> = fs::read_dir(&virtual_node_modules_dir).unwrap().collect();
+    assert!(entries.is_empty(), "no symlinks should be created when both dep maps are absent");
+}
+
+/// Aliased `dependencies` entries — `<alias>: <target-name>@<version>`
+/// shape — still link the alias filename in the slot's `node_modules`
+/// while resolving the slot via the target's name. Guards the
+/// `dep_ref.resolve(alias_name)` behavior since the merge change
+/// rewrote the iteration shape; the alias path mustn't regress.
+#[test]
+fn alias_dep_links_under_alias_but_resolves_via_target() {
+    let tmp = tempdir().expect("tempdir");
+    let virtual_store_dir = tmp.path().to_path_buf();
+    let layout = VirtualStoreLayout::legacy(virtual_store_dir);
+
+    let mut deps: HashMap<PkgName, SnapshotDepRef> = HashMap::new();
+    deps.insert(pkg_name("string-width-cjs"), dep_ref("string-width@4.2.3"));
+
+    let skipped = SkippedSnapshots::default();
+    let virtual_node_modules_dir = tmp.path().join("self/node_modules");
+    fs::create_dir_all(&virtual_node_modules_dir).unwrap();
+
+    create_symlink_layout(
+        Some(&deps),
+        None,
+        &pkg_name("self"),
+        &skipped,
+        &layout,
+        &virtual_node_modules_dir,
+    )
+    .expect("create_symlink_layout should succeed");
+
+    let symlink_path = virtual_node_modules_dir.join("string-width-cjs");
+    let read = fs::read_link(&symlink_path).expect("read_link");
+    let expected = layout
+        .slot_dir(&"string-width@4.2.3".parse().unwrap())
+        .join("node_modules")
+        .join("string-width");
+    assert_eq!(read, expected);
+}

pacquet/crates/package-manager/src/create_virtual_dir_by_snapshot.rs

@@ -1,6 +1,6 @@
 use crate::{
-    ImportIndexedDirError, ImportIndexedDirOpts, SymlinkPackageError, VirtualStoreLayout,
-    create_symlink_layout, import_indexed_dir,
+    ImportIndexedDirError, ImportIndexedDirOpts, SkippedSnapshots, SymlinkPackageError,
+    VirtualStoreLayout, create_symlink_layout, import_indexed_dir,
 };
 use derive_more::{Display, Error};
 use miette::Diagnostic;
@@ -47,6 +47,14 @@ pub struct CreateVirtualDirBySnapshot<'a> {
     pub package_id: &'a str,
     pub package_key: &'a PackageKey,
     pub snapshot: &'a SnapshotEntry,
+    /// Snapshots whose slots were not materialized on this host —
+    /// platform-mismatched optionals, `--no-optional` exclusions, and
+    /// swallowed optional fetch failures. `create_symlink_layout`
+    /// uses this to skip dangling symlinks to absent slots. Mirrors
+    /// upstream's `!pkg.installable && pkg.optional` short-circuit in
+    /// `linkAllModules` at
+    /// <https://github.com/pnpm/pnpm/blob/f2981a316/installing/deps-installer/src/install/link.ts#L540>.
+    pub skipped: &'a SkippedSnapshots,
 }
 
 /// Error type of [`CreateVirtualDirBySnapshot`].
@@ -79,6 +87,7 @@ impl<'a> CreateVirtualDirBySnapshot<'a> {
             package_id: _package_id,
             package_key,
             snapshot,
+            skipped,
         } = self;
 
         let virtual_node_modules_dir = layout.slot_dir(package_key).join("node_modules");
@@ -109,12 +118,16 @@ impl<'a> CreateVirtualDirBySnapshot<'a> {
                 )
                 .map_err(CreateVirtualDirError::ImportIndexedDir)
             },
-            || match snapshot.dependencies.as_ref() {
-                Some(dependencies) => {
-                    create_symlink_layout(dependencies, layout, &virtual_node_modules_dir)
-                        .map_err(CreateVirtualDirError::SymlinkPackage)
-                }
-                None => Ok(()),
+            || {
+                create_symlink_layout(
+                    snapshot.dependencies.as_ref(),
+                    snapshot.optional_dependencies.as_ref(),
+                    &package_key.name,
+                    skipped,
+                    layout,
+                    &virtual_node_modules_dir,
+                )
+                .map_err(CreateVirtualDirError::SymlinkPackage)
             },
         );
         cas_result?;

pacquet/crates/package-manager/src/create_virtual_dir_by_snapshot/tests.rs

@@ -61,6 +61,7 @@ async fn run_emits_imported_event_after_import_indexed_dir() {
     // `.run()` directly here. The function itself is sync — only
     // the caller's runtime flavor matters.
     let layout = crate::VirtualStoreLayout::legacy(virtual_store_dir.clone());
+    let skipped = crate::SkippedSnapshots::default();
     CreateVirtualDirBySnapshot {
         layout: &layout,
         cas_paths: &cas_paths,
@@ -70,6 +71,7 @@ async fn run_emits_imported_event_after_import_indexed_dir() {
         package_id: "react@18.0.0",
         package_key: &package_key,
         snapshot: &snapshot,
+        skipped: &skipped,
     }
     .run::<RecordingReporter>()
     .expect("empty-cas-paths run should succeed");

pacquet/crates/package-manager/src/create_virtual_store.rs

@@ -647,6 +647,7 @@ impl<'a> CreateVirtualStore<'a> {
                         package_id: &package_id,
                         package_key: snapshot_key,
                         snapshot,
+                        skipped,
                     }
                     .run::<R>()
                     .map_err(|e| {
@@ -725,6 +726,7 @@ impl<'a> CreateVirtualStore<'a> {
                         metadata,
                         snapshot,
                         allow_build_policy,
+                        skipped,
                         node_linker,
                     }
                     .run::<R>()

pacquet/crates/package-manager/src/install_package_by_snapshot.rs

@@ -68,6 +68,13 @@ pub struct InstallPackageBySnapshot<'a> {
     /// [`crate::InstallFrozenLockfile::run`] and threaded through
     /// [`crate::CreateVirtualStore`].
     pub allow_build_policy: &'a AllowBuildPolicy,
+    /// Snapshots whose slots were not materialized on this host —
+    /// threaded into [`CreateVirtualDirBySnapshot`] so the per-slot
+    /// `create_symlink_layout` step can skip optional siblings whose
+    /// target slot is absent (platform mismatch, `--no-optional`
+    /// exclusion, or swallowed optional fetch failure). See
+    /// [`crate::SkippedSnapshots`] for how it is built.
+    pub skipped: &'a crate::SkippedSnapshots,
     /// Selects between the isolated and hoisted install layouts.
     /// `Isolated` runs [`CreateVirtualDirBySnapshot`] at the end of
     /// the per-snapshot fetch to populate the virtual-store slot;
@@ -200,6 +207,7 @@ impl<'a> InstallPackageBySnapshot<'a> {
             metadata,
             snapshot,
             allow_build_policy,
+            skipped,
             node_linker,
         } = self;
 
@@ -434,6 +442,7 @@ impl<'a> InstallPackageBySnapshot<'a> {
                 package_id: &package_id,
                 package_key,
                 snapshot,
+                skipped,
             }
             .run::<R>()
             .map_err(InstallPackageBySnapshotError::CreateVirtualDir)?;