From 9f257470149bfd5576e97247988120e3df1d0cc8 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 18 Sep 2026 01:31:29 +0000 Subject: [PATCH 1/4] install: fail a lifecycle script of a package that a required dependency shares with an optional one The optional flag of a package's lifecycle scripts came from the one dependency that placed the package. optionalDependencies sort first, so a package that the root lists as optional and that a required dependency also needs ran its scripts as optional. A failed script deleted the package and the install exited 0. Both linkers now ask RequiredPackages whether any linked dependency without the OPTIONAL bit resolves to the package. The isolated installer keeps the RequiredPackages in the Installer so the RunScripts arm of run_tasks can ask it too. --- src/install/PackageInstaller.rs | 15 ++-- src/install/PackageManager/runTasks.rs | 8 ++- src/install/isolated_install.rs | 15 ++-- src/install/isolated_install/Installer.rs | 4 ++ .../bun-install-lifecycle-scripts.test.ts | 68 +++++++++++++++++++ 5 files changed, 96 insertions(+), 14 deletions(-) diff --git a/src/install/PackageInstaller.rs b/src/install/PackageInstaller.rs index ca441264d8ab..084146bb1041 100644 --- a/src/install/PackageInstaller.rs +++ b/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) = diff --git a/src/install/PackageManager/runTasks.rs b/src/install/PackageManager/runTasks.rs index 4282cf698fc8..c4a76286c03a 100644 --- a/src/install/PackageManager/runTasks.rs +++ b/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). diff --git a/src/install/isolated_install.rs b/src/install/isolated_install.rs index 55c02f20153e..ff288b8e498b 100644 --- a/src/install/isolated_install.rs +++ b/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 => { diff --git a/src/install/isolated_install/Installer.rs b/src/install/isolated_install/Installer.rs index 1ee1715dbf53..2c2b10245947 100644 --- a/src/install/isolated_install/Installer.rs +++ b/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, // intrusive via .next pub(crate) tasks: Box<[Task]>, diff --git a/test/cli/install/bun-install-lifecycle-scripts.test.ts b/test/cli/install/bun-install-lifecycle-scripts.test.ts index 3ccbfb3b4c3f..97c07e677c76 100644 --- a/test/cli/install/bun-install-lifecycle-scripts.test.ts +++ b/test/cli/install/bun-install-lifecycle-scripts.test.ts @@ -210,6 +210,74 @@ 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 [err, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]); + 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 [err, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]); + expect(err).not.toContain("error:"); + 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(); + }); +} + test.concurrent( "trustedDependencies added on a later install still matches the resolved package name, not the dependency alias", async () => { From d3bb0f4759d8cf8f124d7ca25cc5263da1818b12 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 18 Sep 2026 01:53:51 +0000 Subject: [PATCH 2/4] install: test the hoisted already-installed path for a shared scripted package --- .../bun-install-lifecycle-scripts.test.ts | 46 +++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/test/cli/install/bun-install-lifecycle-scripts.test.ts b/test/cli/install/bun-install-lifecycle-scripts.test.ts index 97c07e677c76..972843d47b57 100644 --- a/test/cli/install/bun-install-lifecycle-scripts.test.ts +++ b/test/cli/install/bun-install-lifecycle-scripts.test.ts @@ -278,6 +278,52 @@ for (const linker of ["hoisted", "isolated"] as const) { }); } +// 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 [err, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]); + expect(err).not.toContain("error:"); + 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 [err, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]); + 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 () => { From 83bb55bf0d287e94608ba2b18a0f3a49f88d7b24 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 18 Sep 2026 02:00:59 +0000 Subject: [PATCH 3/4] install: read stdout in the new lifecycle script tests and assert the blocked first install --- .../install/bun-install-lifecycle-scripts.test.ts | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/test/cli/install/bun-install-lifecycle-scripts.test.ts b/test/cli/install/bun-install-lifecycle-scripts.test.ts index 972843d47b57..49b2ffb3c141 100644 --- a/test/cli/install/bun-install-lifecycle-scripts.test.ts +++ b/test/cli/install/bun-install-lifecycle-scripts.test.ts @@ -239,7 +239,8 @@ for (const linker of ["hoisted", "isolated"] as const) { stderr: "pipe", env, }); - const [err, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]); + 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', ]); @@ -270,8 +271,9 @@ for (const linker of ["hoisted", "isolated"] as const) { stderr: "pipe", env, }); - const [err, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]); + 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(); @@ -302,8 +304,9 @@ test.concurrent( stderr: "pipe", env, }); - const [err, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]); + 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); } @@ -316,7 +319,8 @@ test.concurrent( stderr: "pipe", env, }); - const [err, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]); + 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', ]); From 5727d399ea6b24385e7f4f0da487c837153d56a8 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 18 Sep 2026 02:23:20 +0000 Subject: [PATCH 4/4] ci: retrigger