install: keep the packages bun.lock holds only through optional peers when an install re-resolves

src/install/PackageManager/install_with_manager.rs

@@ -90,8 +90,9 @@ pub fn install_with_manager(
 
     // Snapshot the loaded-from-lockfile package count so
     // `Lockfile::get_package_id` can tell loaded pins apart from packages
-    // appended by manifest fetches in this resolve session.
-    manager.lockfile.mark_loaded_packages();
+    // appended by manifest fetches in this resolve session, and which loaded
+    // packages a non-peer dependency holds, for `clean_with_logger`.
+    manager.lockfile.mark_loaded_packages()?;
 
     let (config_version, changed_config_version) = load_result.choose_config_version();
     manager.options.config_version = Some(config_version);

src/install/dedupe.rs

@@ -605,7 +605,8 @@ pub(crate) fn effective_version(
     Some(version)
 }
 
-// Optional-peer edges are followed too: with an in-sync package.json `clean` runs with `keep_optional_peer_targets`.
+// Optional-peer edges are followed too: `clean` keeps a target the loaded lockfile held through them alone
+// (`Lockfile::held_at_load`), and a version that survives here keeps every edge it had, so anything else they reach stays held.
 fn reachable(lockfile: &Lockfile, resolutions: &[PackageID]) -> DynamicBitSet {
     crate::lockfile::reachable::packages(
         lockfile,

src/install/lockfile.rs

@@ -203,6 +203,19 @@ pub struct Lockfile {
     /// Runtime-only — never serialised.
     pub(crate) loaded_package_count: PackageID,
 
+    /// `bit[id] == true` ⇔ a dependency other than an optional peer resolved
+    /// to package `id` in the lockfile as loaded. An optional peer slot never
+    /// keeps such a package alive through `clean_with_logger` (it is bound in
+    /// `Cloner::flush` if something else still reaches it), so a package whose
+    /// last real dependent leaves package.json is dropped. A loaded package
+    /// with the bit unset was held by optional peers alone; 1.3.x wrote such
+    /// entries and they are kept, otherwise every resolve would prune them and
+    /// rewrite the file on any unrelated change. Sized to
+    /// `loaded_package_count`, so packages appended during this resolve read
+    /// as held (a fresh resolve never creates peer-only entries). Set by
+    /// `mark_loaded_packages`; runtime-only — never serialised.
+    pub(crate) held_at_load: DynamicBitSet,
+
     /// `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
@@ -969,9 +982,6 @@ impl Lockfile {
 
         let mut package_id_mapping = vec![invalid_package_id; old.packages.len()];
         let clone_queue_ = PendingResolutions::new();
-        // A frozen install never saves, so dropping peer-held targets could only fail its check.
-        let keep_optional_peer_targets =
-            manager.options.enable.frozen_lockfile() || !manager.summary.changes_resolutions();
         // Explicit `&mut *` reborrows so `old`/`manager`/`new` are
         // released back to this scope once `cloner` is dropped.
         let mut cloner = Cloner {
@@ -980,7 +990,6 @@ impl Lockfile {
             mapping: &mut package_id_mapping,
             clone_queue: clone_queue_,
             optional_peers: PendingResolutions::new(),
-            keep_optional_peer_targets,
             log,
             old_preinstall_state,
             manager: &mut *manager,
@@ -1207,7 +1216,6 @@ pub struct Cloner<'a> {
     pub(crate) clone_queue: PendingResolutions,
     /// Bound in `flush`, once `clone_queue` has decided which targets survive.
     pub(crate) optional_peers: PendingResolutions,
-    pub(crate) keep_optional_peer_targets: bool,
     pub lockfile: &'a mut Lockfile,
     pub(crate) old: &'a mut Lockfile,
     pub(crate) mapping: &'a mut [PackageID],
@@ -1986,16 +1994,32 @@ impl Lockfile {
             // session-appended, so the order-independence guard in
             // `get_package_id` applies from id 0.
             loaded_package_count: 0,
+            held_at_load: DynamicBitSet::default(),
             exact_pinned: DynamicBitSet::default(),
         }
     }
 
-    /// Snapshot `packages.len()` as the "loaded from lockfile" watermark.
-    /// Call exactly once after `load_from_cwd` (including npm/pnpm/yarn
-    /// migration) before any manifest-driven `append_package`.
-    #[inline]
-    pub(crate) fn mark_loaded_packages(&mut self) {
-        self.loaded_package_count = self.packages.len() as PackageID;
+    /// Snapshot `packages.len()` as the "loaded from lockfile" watermark and
+    /// which of those packages a non-peer dependency resolves to
+    /// (`held_at_load`). Call exactly once after `load_from_cwd` (including
+    /// npm/pnpm/yarn migration), before the differ, an update or a dedupe
+    /// re-points any resolution and before any manifest-driven
+    /// `append_package`.
+    pub(crate) fn mark_loaded_packages(&mut self) -> Result<(), AllocError> {
+        let packages_len = self.packages.len();
+        self.loaded_package_count = packages_len as PackageID;
+        let mut held = DynamicBitSet::init_empty(packages_len)?;
+        // A load that failed partway leaves `resolutions` shorter than `dependencies`;
+        // that lockfile is replaced by `init_empty` before anything reads the set.
+        let dependencies = self.buffers.dependencies.as_slice();
+        let resolutions = self.buffers.resolutions.as_slice();
+        for (dependency, &package_id) in dependencies.iter().zip(resolutions) {
+            if !dependency.behavior.is_optional_peer() && (package_id as usize) < packages_len {
+                held.set(package_id as usize);
+            }
+        }
+        self.held_at_load = held;
+        Ok(())
     }
 
     /// Record that package `id` was appended via an exact-version dependency

src/install/lockfile/Package.rs

@@ -616,8 +616,14 @@ impl Package<u64> {
                 resolve_id: new_package.resolutions.off + PackageID::try_from(i).expect("int cast"),
             };
 
-            // Peer slots must not keep their target alive; bound in `Cloner::flush`.
-            if old_dependencies[i].behavior.is_optional_peer() && !cloner.keep_optional_peer_targets
+            // A peer slot must not keep a target alive that something else held
+            // when the lockfile was loaded; it is bound in `Cloner::flush` if that
+            // holder survived. A target the loaded lockfile held through optional
+            // peers alone is cloned like a dependency (see `Lockfile::held_at_load`).
+            if old_dependencies[i].behavior.is_optional_peer()
+                && old
+                    .held_at_load
+                    .is_set_allow_out_of_bound(*old_resolution as usize, true)
             {
                 cloner.optional_peers.push(pending);
                 continue;

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

@@ -1450,6 +1450,51 @@ it("--frozen-lockfile keeps a package that an older lockfile lists only as an op
   expect(await exists(join(packageDir, "node_modules", "no-deps", "package.json"))).toBeTrue();
 });
 
+// Same entries, but the install that re-resolves and saves: a package.json change
+// unrelated to them (here a dependency bun.lock already has) must not prune them
+// either. Only a package whose real dependent leaves is dropped (the tests above).
+it("a re-resolving install keeps the packages an older lockfile holds through optional peers alone", async () => {
+  const { packageDir, packageJson } = await registry.createTestDir({
+    bunfigOpts: { saveTextLockfile: true, linker: "hoisted" },
+  });
+  const run = makeInstallRunner(packageDir);
+  const noDepsEntry = '"no-deps": ["no-deps@1.0.0"';
+  const locked = { "optional-peer-deps": "1.0.0", "uses-a-dep-1": "1.0.0" };
+  await Promise.all([
+    // a-dep is already in bun.lock through uses-a-dep-1, so nothing resolves differently.
+    write(packageJson, JSON.stringify({ name: "foo", dependencies: { ...locked, "a-dep": "1.0.1" } })),
+    write(
+      join(packageDir, "bun.lock"),
+      JSON.stringify({
+        lockfileVersion: 1,
+        configVersion: 1,
+        workspaces: { "": { name: "foo", dependencies: locked } },
+        packages: {
+          "a-dep": ["a-dep@1.0.1", "", {}, ""],
+          // only optional-peer-deps's optional peer refers to this entry
+          "no-deps": ["no-deps@1.0.0", "", {}, ""],
+          "optional-peer-deps": [
+            "optional-peer-deps@1.0.0",
+            "",
+            { peerDependencies: { "no-deps": "*" }, optionalPeers: ["no-deps"] },
+            "",
+          ],
+          "uses-a-dep-1": ["uses-a-dep-1@1.0.0", "", { dependencies: { "a-dep": "1.0.1" } }, ""],
+        },
+      }),
+    ),
+  ]);
+
+  await run(["install"]);
+  const saved = await file(join(packageDir, "bun.lock")).text();
+  expect(saved).toContain('"a-dep": "1.0.1"');
+  expect(saved).toContain(noDepsEntry);
+  expect(await exists(join(packageDir, "node_modules", "no-deps", "package.json"))).toBeTrue();
+
+  await run(["install", "--frozen-lockfile"]);
+  expect(await file(join(packageDir, "bun.lock")).text()).toBe(saved);
+});
+
 // The optional-peer-hoist-* fixtures are described in
 // registry/packages/create-optional-peer-hoist-packages.ts. In short: consumer
 // has an optional peer on target, and deep -> deep-child reaches target@1.0.0