install: dedupe a transitive wide range onto a root/workspace range across majors

src/install/PackageManager/PackageManagerEnqueue.rs

@@ -2134,14 +2134,13 @@ fn get_or_put_resolved_package_with_find_result(
     )?)?;
 
     debug_assert!(package.meta.id != invalid_package_id);
-    // Record exact-version pins so `Lockfile::get_package_id`'s
-    // order-independence guard can tell them apart from range-resolved
-    // entries (which it treats as network-order artefacts).
-    if version.tag == dependency::version::Tag::Npm && version.npm().version.is_exact() {
-        // SAFETY: `this_ptr` is the sole live `&mut PackageManager` here;
-        // `lockfile.exact_pinned` is disjoint from `package` (returned
-        // by-value above).
-        unsafe { &mut *(*this_ptr).lockfile }.mark_exact_pin(package.meta.id);
+    // SAFETY: `this_ptr` is the sole live `&mut PackageManager` here and
+    // `package` was returned by value above, so nothing aliases the lockfile.
+    let lockfile = unsafe { &mut *(*this_ptr).lockfile };
+    if (version.tag == dependency::version::Tag::Npm && version.npm().version.is_exact())
+        || lockfile.is_workspace_dependency(dependency_id)
+    {
+        lockfile.mark_local_pin(package.meta.id);
     }
     // Use scopeguard so success_fn runs on every
     // return below (including the `?` paths). The guard owns the raw pointer so the

src/install/lockfile.rs

@@ -203,14 +203,11 @@ pub struct Lockfile {
     /// Runtime-only — never serialised.
     pub(crate) loaded_package_count: PackageID,
 
-    /// `bit[id] == true` ⇔ package `id` was appended for a dependency whose
-    /// version range was an exact `=X.Y.Z` (i.e. the user — root or workspace
-    /// — pinned this exact version somewhere in the tree). `get_package_id`'s
-    /// order-independence guard never blocks deduping to one of these: an
-    /// exact pin is a deliberate choice, not an artifact of which manifest
-    /// happened to land first. Runtime-only — never serialised; sized lazily
-    /// in `mark_exact_pin`.
-    pub(crate) exact_pinned: DynamicBitSet,
+    /// Packages appended for a root/workspace package.json dependency or an
+    /// exact `=X.Y.Z` dependency; exempt from the guard in `get_package_id`.
+    ///
+    /// Runtime-only — never serialised.
+    pub(crate) local_pinned: DynamicBitSet,
 }
 
 pub(crate) type PackageList = self::package::List<u64>;
@@ -1971,7 +1968,7 @@ impl Lockfile {
             // session-appended, so the order-independence guard in
             // `get_package_id` applies from id 0.
             loaded_package_count: 0,
-            exact_pinned: DynamicBitSet::default(),
+            local_pinned: DynamicBitSet::default(),
         }
     }
 
@@ -1983,15 +1980,14 @@ impl Lockfile {
         self.loaded_package_count = self.packages.len() as PackageID;
     }
 
-    /// Record that package `id` was appended via an exact-version dependency
-    /// (`=X.Y.Z`). See the `exact_pinned` field doc.
+    /// See `local_pinned`.
     #[inline]
-    pub(crate) fn mark_exact_pin(&mut self, id: PackageID) {
+    pub(crate) fn mark_local_pin(&mut self, id: PackageID) {
         let i = id as usize;
-        if self.exact_pinned.bit_length() <= i {
-            bun_core::handle_oom(self.exact_pinned.resize(i + 1, false));
+        if self.local_pinned.bit_length() <= i {
+            bun_core::handle_oom(self.local_pinned.resize(i + 1, false));
         }
-        self.exact_pinned.set(i);
+        self.local_pinned.set(i);
     }
 
     pub(crate) fn get_package_id(
@@ -2030,7 +2026,7 @@ impl Lockfile {
         let buf = self.buffers.string_bytes.as_slice();
 
         let loaded_watermark = self.loaded_package_count;
-        let exact_pinned = &self.exact_pinned;
+        let local_pinned = &self.local_pinned;
         let try_satisfies_dedupe = |id: PackageID| -> bool {
             let existing = &resolutions[id as usize];
             if existing.tag != ResolutionTag::Npm {
@@ -2043,22 +2039,10 @@ impl Lockfile {
             if !npm_v.satisfies(existing_ver, buf, buf) {
                 return false;
             }
-            // Order-independence guard. We refuse to dedupe a wide range to a
-            // *lower* existing entry only when ALL of the following hold:
-            //   - the entry was appended in this resolve session
-            //     (lockfile-loaded entries are the user's existing pin),
-            //   - the entry was NOT appended for an exact-`=X.Y.Z` dependency
-            //     (an exact pin anywhere in the tree is a deliberate choice,
-            //     not a network-order artefact — `dragon test 2` /
-            //     "dependency from root satisfies range from dependency"),
-            //   - the manifest's best-match is a *different major* (within a
-            //     major, deduping to an older patch is the long-standing
-            //     behaviour and the worst case is still ^-compatible).
-            // What this leaves is exactly the cross-parent network-order
-            // flake: a wide range (`*`, `>=X`) collapsing onto a sibling's
-            // *range-resolved* lower major depending on whose manifest landed
-            // first ("text lockfile is hoisted").
-            if id >= loaded_watermark && !exact_pinned.is_set_allow_out_of_bound(id as usize, false)
+            // A wide range must not collapse onto a lower major that a sibling's
+            // manifest merely happened to append first. Lockfile-loaded and
+            // `local_pinned` entries do not depend on manifest order.
+            if id >= loaded_watermark && !local_pinned.is_set_allow_out_of_bound(id as usize, false)
             {
                 if let Some(floor) = resolved_npm_floor {
                     if existing_ver.order(floor, buf, buf) == Ordering::Less

test/cli/install/bun-install-registry.test.ts

@@ -4568,6 +4568,113 @@ describe("hoisting", async () => {
       lockfile,
     );
   });
+
+  test("transitive wide range dedupes onto root range across majors", async () => {
+    // Root declares `hoist-lockfile-shared: ^1.0.1` (resolves to 1.0.2).
+    // `hoist-lockfile-1` depends on `hoist-lockfile-shared: *` (best-match 2.0.2).
+    // The `*` satisfies 1.0.2, so it must dedupe onto the root's copy instead of
+    // nesting a second install at 2.0.2 (npm also dedupes here).
+    await write(
+      packageJson,
+      JSON.stringify({
+        name: "foo",
+        dependencies: {
+          "hoist-lockfile-1": "1.0.0",
+          "hoist-lockfile-shared": "^1.0.1",
+        },
+      }),
+    );
+
+    const { stdout, stderr, exited } = spawn({
+      cmd: [bunExe(), "install"],
+      cwd: packageDir,
+      stderr: "pipe",
+      stdout: "pipe",
+      env,
+    });
+
+    const [out, err, exitCode] = await Promise.all([stdout.text(), stderr.text(), exited]);
+    expect(err).toContain("Saved lockfile");
+    expect(err).not.toContain("error:");
+    expect(out.replace(/\s*\[[0-9\.]+m?s\]\s*$/, "").split(/\r?\n/)).toEqual([
+      expect.stringContaining("bun install v1."),
+      "",
+      "+ hoist-lockfile-1@1.0.0",
+      expect.stringContaining("+ hoist-lockfile-shared@1.0.2"),
+      "",
+      "2 packages installed",
+    ]);
+    expect(await file(join(packageDir, "node_modules", "hoist-lockfile-shared", "package.json")).json()).toMatchObject({
+      name: "hoist-lockfile-shared",
+      version: "1.0.2",
+    });
+    expect(await exists(join(packageDir, "node_modules", "hoist-lockfile-1", "node_modules"))).toBeFalse();
+    expect(exitCode).toBe(0);
+    assertManifestsPopulated(join(packageDir, ".bun-cache"), registryUrl());
+
+    // Second install from the saved lockfile must not rewrite it.
+    await rm(join(packageDir, "node_modules"), { recursive: true, force: true });
+    const second = spawn({
+      cmd: [bunExe(), "install"],
+      cwd: packageDir,
+      stderr: "pipe",
+      stdout: "pipe",
+      env,
+    });
+    const [out2, err2, exitCode2] = await Promise.all([second.stdout.text(), second.stderr.text(), second.exited]);
+    expect(err2).not.toContain("Saved lockfile");
+    expect(err2).not.toContain("error:");
+    expect(out2).toContain("2 packages installed");
+    expect(await exists(join(packageDir, "node_modules", "hoist-lockfile-1", "node_modules"))).toBeFalse();
+    expect(exitCode2).toBe(0);
+  });
+
+  test("transitive wide range dedupes onto workspace range across majors", async () => {
+    // Same as above but the narrow range lives in a workspace package.json.
+    await write(
+      packageJson,
+      JSON.stringify({
+        name: "foo",
+        workspaces: ["pkg-a"],
+      }),
+    );
+    await mkdir(join(packageDir, "pkg-a"));
+    await write(
+      join(packageDir, "pkg-a", "package.json"),
+      JSON.stringify({
+        name: "pkg-a",
+        version: "1.0.0",
+        dependencies: {
+          "hoist-lockfile-1": "1.0.0",
+          "hoist-lockfile-shared": "^1.0.1",
+        },
+      }),
+    );
+
+    const { stdout, stderr, exited } = spawn({
+      cmd: [bunExe(), "install"],
+      cwd: packageDir,
+      stderr: "pipe",
+      stdout: "pipe",
+      env,
+    });
+
+    const [out, err, exitCode] = await Promise.all([stdout.text(), stderr.text(), exited]);
+    expect(err).toContain("Saved lockfile");
+    expect(err).not.toContain("error:");
+    expect(out.replace(/\s*\[[0-9\.]+m?s\]\s*$/, "").split(/\r?\n/)).toEqual([
+      expect.stringContaining("bun install v1."),
+      "",
+      "3 packages installed",
+    ]);
+    expect(await file(join(packageDir, "node_modules", "hoist-lockfile-shared", "package.json")).json()).toMatchObject({
+      name: "hoist-lockfile-shared",
+      version: "1.0.2",
+    });
+    expect(await exists(join(packageDir, "node_modules", "hoist-lockfile-1", "node_modules"))).toBeFalse();
+    expect(exitCode).toBe(0);
+    assertManifestsPopulated(join(packageDir, ".bun-cache"), registryUrl());
+  });
 });
 
 describe("transitive file dependencies", () => {