install: fail a lifecycle script of a package that a required and an optional dependency share

src/install/PackageInstaller.rs

@@ -1929,7 +1929,6 @@ impl<'a> PackageInstaller<'a> {
 
                     let dep =
                         &self.lockfile().buffers.dependencies.as_slice()[dependency_id as usize];
-                    let dep_behavior = dep.behavior;
                     let truncated_dep_name_hash: TruncatedPackageNameHash =
                         dep.name_hash as TruncatedPackageNameHash;
                     let (is_trusted, is_trusted_through_update_request) = 'brk: {
@@ -1990,12 +1989,17 @@ impl<'a> PackageInstaller<'a> {
                                 break 'enqueue_lifecycle_scripts;
                             }
 
+                            let optional = !self.required_packages.contains(
+                                self.manager(),
+                                dependency_id,
+                                package_id,
+                            );
                             if self.enqueue_lifecycle_scripts(
                                 alias.slice(string_buf!()),
                                 log_level,
                                 &mut folder_path,
                                 package_id,
-                                dep_behavior.contains(crate::dependency::Behavior::OPTIONAL),
+                                optional,
                                 resolution,
                             ) {
                                 if is_trusted_through_update_request {
@@ -2230,7 +2234,6 @@ impl<'a> PackageInstaller<'a> {
             }
 
             let dep = &self.lockfile().buffers.dependencies.as_slice()[dependency_id as usize];
-            let dep_behavior = dep.behavior;
             let truncated_dep_name_hash: TruncatedPackageNameHash =
                 dep.name_hash as TruncatedPackageNameHash;
             let (is_trusted, is_trusted_through_update_request, add_to_lockfile) = 'brk: {
@@ -2299,12 +2302,16 @@ impl<'a> PackageInstaller<'a> {
                         break 'enqueue_lifecycle_scripts;
                     }
 
+                    let optional =
+                        !self
+                            .required_packages
+                            .contains(self.manager(), dependency_id, package_id);
                     if self.enqueue_lifecycle_scripts(
                         alias.slice(string_buf!()),
                         log_level,
                         &mut folder_path,
                         package_id,
-                        dep_behavior.contains(crate::dependency::Behavior::OPTIONAL),
+                        optional,
                         resolution,
                     ) {
                         let (trusted_name, trusted_name_hash) =

src/install/PackageManager/runTasks.rs

@@ -22,7 +22,6 @@ use super::{
     Command, PackageInstaller, PackageManager, ProgressStrings, Subcommand, TaskCallbackList,
 };
 use super::{directories, enqueue};
-use crate::dependency::Behavior;
 use crate::isolated_install::installer as store_installer;
 use crate::isolated_install::store::{EntryColumns as _, NodeColumns as _};
 use crate::lifecycle_script_runner::InstallCtx;
@@ -381,8 +380,11 @@ fn run_tasks_erased(
                     let entry_id = task.entry_id;
                     let node_id = installer.store.entries.items_node_id()[entry_id.get() as usize];
                     let dep_id = installer.store.nodes.items_dep_id()[node_id.get() as usize];
-                    let dep = &installer.lockfile().buffers.dependencies[dep_id as usize];
-                    let optional = dep.behavior.contains(Behavior::OPTIONAL);
+                    let pkg_id = installer.store.nodes.items_pkg_id()[node_id.get() as usize];
+                    let optional =
+                        !installer
+                            .required_packages
+                            .contains(installer.manager(), dep_id, pkg_id);
                     // SAFETY: `list` is the per-entry scripts slot owned by
                     // `store.entries.items_scripts()[entry_id]`; this Task is
                     // its sole consumer (see Installer.rs Yield::RunScripts).

src/install/isolated_install.rs

@@ -2047,6 +2047,11 @@ pub(crate) fn install_isolated_packages(
                 None
             },
             store: &store,
+            required_packages: RequiredPackages::new(
+                workspace_filters,
+                install_root_dependencies,
+                packages_to_install,
+            ),
             tasks,
             waiters_head: vec![store::entry::Id::INVALID; store.entries.len()].into_boxed_slice(),
             next_waiter: vec![store::entry::Id::INVALID; store.entries.len()].into_boxed_slice(),
@@ -2100,12 +2105,6 @@ pub(crate) fn install_isolated_packages(
             );
         }
 
-        let mut required_packages = RequiredPackages::new(
-            workspace_filters,
-            install_root_dependencies,
-            packages_to_install,
-        );
-
         // add the pending task count upfront
         installer
             .manager_mut()
@@ -2416,7 +2415,9 @@ pub(crate) fn install_isolated_packages(
 
                     let dep = &lockfile_ro.buffers.dependencies[dep_id as usize];
                     let is_required =
-                        required_packages.contains(installer.manager(), dep_id, pkg_id);
+                        installer
+                            .required_packages
+                            .contains(installer.manager(), dep_id, pkg_id);
 
                     match pkg_res_tag {
                         ResolutionTag::Npm => {

src/install/isolated_install/Installer.rs

@@ -14,6 +14,7 @@ use bun_sys::{FdDirExt as _, FdExt as _};
 
 use crate::bin_real;
 use crate::lockfile::package;
+use crate::lockfile::tree::RequiredPackages;
 use crate::lockfile_real::PackageIDSlice;
 use crate::package_install::{Method as InstallMethod, Summary as InstallSummary};
 use crate::package_manager_real::Command;
@@ -88,6 +89,9 @@ pub struct Installer<'a> {
 
     pub(crate) store: &'a Store,
 
+    /// Main-thread only.
+    pub(crate) required_packages: RequiredPackages<'a>,
+
     pub(crate) task_queue: UnboundedQueue<Task>, // intrusive via .next
     pub(crate) tasks: Box<[Task]>,
 

test/cli/install/bun-install-lifecycle-scripts.test.ts

@@ -210,6 +210,124 @@ test.concurrent("trustedDependencies matches the resolved package name, not the
   expect(await exited).toBe(0);
 });
 
+// `no-deps-scripted-to-fail` has an `install` script that exits 1. The linker places it once,
+// under the root's optional dependency. `no-deps-scripted-to-deeply-fail` requires it, so the
+// failed script fails the install.
+for (const linker of ["hoisted", "isolated"] as const) {
+  test.concurrent(
+    `a failed script of a package that an optional and a required dependency share (${linker})`,
+    async () => {
+      using ctx = await setupTest();
+      const { packageDir, packageJson, env } = ctx;
+
+      await writeFile(
+        packageJson,
+        JSON.stringify({
+          name: "foo",
+          version: "1.0.0",
+          dependencies: { "no-deps-scripted-to-deeply-fail": "1.0.0" },
+          optionalDependencies: { "no-deps-scripted-to-fail": "1.0.0" },
+          trustedDependencies: ["no-deps-scripted-to-fail"],
+        }),
+      );
+
+      await using proc = spawn({
+        cmd: [bunExe(), "install", `--linker=${linker}`],
+        cwd: packageDir,
+        stdout: "pipe",
+        stdin: "ignore",
+        stderr: "pipe",
+        env,
+      });
+      const [out, err, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+      expect(out).toContain("bun install v1.");
+      expect(splitErrLines(err).filter(line => line.startsWith("error:"))).toEqual([
+        'error: install script from "no-deps-scripted-to-fail" exited with 1',
+      ]);
+      expect(exitCode).toBe(1);
+    },
+  );
+
+  test.concurrent(`a failed script of a package that only an optional dependency needs (${linker})`, async () => {
+    using ctx = await setupTest();
+    const { packageDir, packageJson, env } = ctx;
+
+    await writeFile(
+      packageJson,
+      JSON.stringify({
+        name: "foo",
+        version: "1.0.0",
+        dependencies: { "no-deps": "1.0.0" },
+        optionalDependencies: { "no-deps-scripted-to-fail": "1.0.0" },
+        trustedDependencies: ["no-deps-scripted-to-fail"],
+      }),
+    );
+
+    await using proc = spawn({
+      cmd: [bunExe(), "install", `--linker=${linker}`],
+      cwd: packageDir,
+      stdout: "pipe",
+      stdin: "ignore",
+      stderr: "pipe",
+      env,
+    });
+    const [out, err, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+    expect(err).not.toContain("error:");
+    expect(out).not.toContain("Blocked");
+    expect(exitCode).toBe(0);
+    expect(await exists(join(packageDir, "node_modules", "no-deps", "package.json"))).toBeTrue();
+    expect(await exists(join(packageDir, "node_modules", "no-deps-scripted-to-fail"))).toBeFalse();
+  });
+}
+
+// The package is already installed and blocked. The second install trusts it and runs its script.
+// Only the hoisted linker runs the scripts of a package that a later install trusts.
+test.concurrent(
+  "a failed script of an installed package that an optional and a required dependency share (hoisted)",
+  async () => {
+    using ctx = await setupTest();
+    const { packageDir, packageJson, env } = ctx;
+
+    const pkg = {
+      name: "foo",
+      version: "1.0.0",
+      dependencies: { "no-deps-scripted-to-deeply-fail": "1.0.0" },
+      optionalDependencies: { "no-deps-scripted-to-fail": "1.0.0" },
+    };
+    await writeFile(packageJson, JSON.stringify(pkg));
+    {
+      await using proc = spawn({
+        cmd: [bunExe(), "install", "--linker=hoisted"],
+        cwd: packageDir,
+        stdout: "pipe",
+        stdin: "ignore",
+        stderr: "pipe",
+        env,
+      });
+      const [out, err, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+      expect(err).not.toContain("error:");
+      expect(out).toContain("Blocked 1 postinstall");
+      expect(exitCode).toBe(0);
+    }
+
+    await writeFile(packageJson, JSON.stringify({ ...pkg, trustedDependencies: ["no-deps-scripted-to-fail"] }));
+    await using proc = spawn({
+      cmd: [bunExe(), "install", "--linker=hoisted"],
+      cwd: packageDir,
+      stdout: "pipe",
+      stdin: "ignore",
+      stderr: "pipe",
+      env,
+    });
+    const [out, err, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+    expect(out).toContain("bun install v1.");
+    expect(splitErrLines(err).filter(line => line.startsWith("error:"))).toEqual([
+      'error: install script from "no-deps-scripted-to-fail" exited with 1',
+    ]);
+    expect(exitCode).toBe(1);
+  },
+);
+
 test.concurrent(
   "trustedDependencies added on a later install still matches the resolved package name, not the dependency alias",
   async () => {