From e801417550928b8269124fc377c1a538dc219a7f Mon Sep 17 00:00:00 2001 From: Colin McDonnell <3084745+colinhacks@users.noreply.github.com> Date: Sat, 15 Aug 2026 09:40:19 -0700 Subject: [PATCH 1/7] fix(install): an optional dependency's build failure no longer fails the install MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Back-port of 92ed4cc78f from sandbox/integration onto main, narrowed to drop its dependence on d6c1d94a35's `failed` short-circuit so it lands standalone. A package reachable only through `optionalDependencies` is one the project declared it can live without, so npm (`_handleOptionalFailure` during reify) and pnpm (`buildDependency`'s catch) both treat a failed build for it as non-fatal. run_dep_lifecycle_scripts never consulted optionality at all. Optionality is derived from the graph's edges via the new non-mutating `optional_only_packages`, not from `LockedPackage::optional` — that field is documented as always false outside the pnpm parse/write path, so reading it would leave this inert on a frozen install off an npm/bun/yarn lockfile. A package with even one fully-required path stays required. A required package's failure still returns immediately, exactly as before; only the optional carve-out is added. Task-level panics stay fatal. Verified on a local-tarball fixture, macOS arm64, Node 26.5.0: optional build fails -> exit 0 + WARN_NUB_OPTIONAL_BUILD_FAILED (was 1) required build fails -> exit 1 (unchanged) reachable both ways -> exit 1 (unchanged) Refs #660 --- vendor/aube/crates/aube-codes/src/warnings.rs | 7 + .../aube/crates/aube-resolver/src/platform.rs | 88 +++++++++++-- .../aube/src/commands/install/lifecycle.rs | 51 ++++++-- vendor/aube/crates/aube/tests/e2e.rs | 120 ++++++++++++++++++ 4 files changed, 246 insertions(+), 20 deletions(-) diff --git a/vendor/aube/crates/aube-codes/src/warnings.rs b/vendor/aube/crates/aube-codes/src/warnings.rs index 49a4dd5ed..1673fe270 100644 --- a/vendor/aube/crates/aube-codes/src/warnings.rs +++ b/vendor/aube/crates/aube-codes/src/warnings.rs @@ -19,6 +19,7 @@ pub const WARN_AUBE_HOOK_PACKAGE_ADDED: &str = "WARN_AUBE_HOOK_PACKAGE_ADDED"; // ── install lifecycle ─────────────────────────────────────────────── pub const WARN_AUBE_IGNORED_BUILD_SCRIPTS: &str = "WARN_AUBE_IGNORED_BUILD_SCRIPTS"; pub const WARN_AUBE_DEFAULT_TRUST_BUILDS: &str = "WARN_AUBE_DEFAULT_TRUST_BUILDS"; +#[rustfmt::skip] pub const WARN_AUBE_OPTIONAL_BUILD_FAILED: &str = "WARN_AUBE_OPTIONAL_BUILD_FAILED"; #[rustfmt::skip] pub const WARN_AUBE_NODE_GYP_BOOTSTRAP_FAILED: &str = "WARN_AUBE_NODE_GYP_BOOTSTRAP_FAILED"; #[rustfmt::skip] pub const WARN_AUBE_SUSPICIOUS_LIFECYCLE_SCRIPT: &str = "WARN_AUBE_SUSPICIOUS_LIFECYCLE_SCRIPT"; #[rustfmt::skip] pub const WARN_AUBE_WINDOWS_JOB_OBJECT_UNAVAILABLE: &str = "WARN_AUBE_WINDOWS_JOB_OBJECT_UNAVAILABLE"; @@ -205,6 +206,12 @@ pub const ALL: &[CodeMeta] = &[ description: "The `defaultTrust` floor let listed packages run build scripts without an explicit `allowBuilds` entry. Disclosure, not an error — set `defaultTrust=false` or an explicit `allowBuilds: false` entry to opt out.", exit_code: None, }, + CodeMeta { + name: WARN_AUBE_OPTIONAL_BUILD_FAILED, + category: category::INSTALL_LIFECYCLE, + description: "A package reachable only through `optionalDependencies` failed to build. The install continues without it (npm and pnpm both treat an optional build failure as non-fatal). Anything importing it at runtime will fail — declare it a regular dependency if it is actually required.", + exit_code: None, + }, CodeMeta { name: WARN_AUBE_NODE_GYP_BOOTSTRAP_FAILED, category: category::INSTALL_LIFECYCLE, diff --git a/vendor/aube/crates/aube-resolver/src/platform.rs b/vendor/aube/crates/aube-resolver/src/platform.rs index 0e7c43276..5f3de2f5c 100644 --- a/vendor/aube/crates/aube-resolver/src/platform.rs +++ b/vendor/aube/crates/aube-resolver/src/platform.rs @@ -364,16 +364,35 @@ pub fn filter_graph( /// `optionalDependencies`). pnpm derives this during resolution; aube /// recomputes it as a post-resolve pass so freshly resolved lockfiles /// carry the same markers pnpm writes instead of an empty `{}` snapshot. -/// -/// Algorithm: seed a `required` set from every non-optional direct -/// dependency of every importer, then walk each required package's -/// *non-optional* edges. A package's non-optional edges are its -/// `dependencies` minus its `optional_dependencies`, because the pnpm -/// parser mirrors active optional edges into `dependencies`. Any package -/// not reached this way is optional. A single fully-required path keeps a -/// package required even when other paths to it are optional, matching -/// pnpm. pub fn mark_optional_packages(graph: &mut aube_lockfile::LockfileGraph) { + let optional = optional_only_packages(graph); + for (dep_path, pkg) in graph.packages.iter_mut() { + pkg.optional = optional.contains(dep_path); + } +} + +/// The dep_paths [`mark_optional_packages`] would mark `optional: true`, +/// derived from the graph's own edges without mutating it. +/// +/// Exists because `LockedPackage::optional` is only populated on two paths — +/// the pnpm reader and the fresh-resolve pass above — so a frozen install off +/// an npm / bun / yarn lockfile carries `optional: false` on every package even +/// though the graph still describes the optional edges. A consumer whose +/// behavior must not silently vary by incumbent lockfile format asks here +/// instead of reading the field. +/// +/// Algorithm: seed a `required` set from every non-optional direct dependency of +/// every importer, then walk each required package's *non-optional* edges. A +/// package's non-optional edges are its `dependencies` minus its +/// `optional_dependencies`, because the pnpm parser mirrors active optional +/// edges into `dependencies`. Any package not reached this way is optional. A +/// single fully-required path keeps a package required even when other paths to +/// it are optional, matching pnpm and npm (whose `calcDepFlags` states the same +/// invariant: "a node still flagged optional must only be reachable via optional +/// edges"). +pub fn optional_only_packages( + graph: &aube_lockfile::LockfileGraph, +) -> aube_util::collections::FxSet { use crate::FxHashSet; use aube_lockfile::DepType; @@ -410,9 +429,12 @@ pub fn mark_optional_packages(graph: &mut aube_lockfile::LockfileGraph) { } } } - for (dep_path, pkg) in graph.packages.iter_mut() { - pkg.optional = !required.contains(dep_path); - } + graph + .packages + .keys() + .filter(|dep_path| !required.contains(*dep_path)) + .cloned() + .collect() } /// Populate each package's `transitive_peer_dependencies` the way pnpm @@ -750,6 +772,48 @@ mod tests { assert!(is_opt("opt-root@1.0.0")); } + // The install's build-failure classification reads this set directly rather + // than `LockedPackage::optional`, so the graph — not the stamped field — has + // to be what decides. Every package here is left `optional: false` (the + // state a frozen install off an npm / bun / yarn lockfile arrives in): if + // the classification ever regressed to reading the field, this test would + // find nothing optional at all. + #[test] + fn optional_only_packages_reads_edges_not_the_stamped_flag() { + use aube_lockfile::DepType; + let mut graph = aube_lockfile::LockfileGraph::default(); + graph.importers.insert( + ".".to_string(), + vec![ + dep("host", DepType::Production), + dep("also-required", DepType::Production), + ], + ); + graph.packages.extend([ + pkg("host", &["native", "dual"], &["native", "dual"]), + pkg("also-required", &["dual"], &[]), + pkg("native", &["native-child"], &[]), + pkg("native-child", &[], &[]), + pkg("dual", &[], &[]), + ]); + assert!( + graph.packages.values().all(|p| !p.optional), + "fixture must leave the stamped flag unset" + ); + + let optional = optional_only_packages(&graph); + + // Reachable only under `host`'s optional edge — and so is its own child. + assert!(optional.contains("native@1.0.0")); + assert!(optional.contains("native-child@1.0.0")); + // The control that proves this can't swallow a required package's build + // failure: `dual` hangs off an optional edge from `host` AND a required + // edge from `also-required`, so one fully-required path keeps it required. + assert!(!optional.contains("dual@1.0.0")); + assert!(!optional.contains("host@1.0.0")); + assert!(!optional.contains("also-required@1.0.0")); + } + fn pkg_with_peers( name: &str, deps: &[&str], diff --git a/vendor/aube/crates/aube/src/commands/install/lifecycle.rs b/vendor/aube/crates/aube/src/commands/install/lifecycle.rs index 2fcb74572..f89975c6b 100644 --- a/vendor/aube/crates/aube/src/commands/install/lifecycle.rs +++ b/vendor/aube/crates/aube/src/commands/install/lifecycle.rs @@ -430,6 +430,9 @@ pub(crate) async fn run_dep_lifecycle_scripts( package_dir: std::path::PathBuf, manifest: aube_manifest::PackageJson, cache_entry: Option, + /// Graph key, kept so the optional-only classification below resolves + /// per job without re-walking the graph. + dep_path: String, } let mut jobs: Vec = Vec::new(); @@ -559,6 +562,7 @@ pub(crate) async fn run_dep_lifecycle_scripts( package_dir, manifest: dep_manifest, cache_entry, + dep_path: dep_path.clone(), }); } @@ -566,6 +570,17 @@ pub(crate) async fn run_dep_lifecycle_scripts( return Ok(0); } + // A package reachable ONLY through `optionalDependencies` is one the + // project declared it can live without, so a failed build for it is + // non-fatal — npm (`_handleOptionalFailure` during reify) and pnpm + // (`buildDependency`'s catch, which logs `reason: 'build_failure'` and + // returns) both continue the install. Computed from the graph's edges + // rather than read off `LockedPackage::optional`, which only the pnpm + // reader and the fresh-resolve pass populate; reading the field would make + // this silently inert on a frozen install off an npm/bun/yarn lockfile. + // A package with even one fully-required path stays required. + let optional_only = aube_resolver::platform::optional_only_packages(graph); + // Name what the floor let through — the floor must never be a // silent allow path. One line, not per-package, so big graphs // don't drown the install output. Emitted at `warn` with a stable @@ -652,13 +667,19 @@ pub(crate) async fn run_dep_lifecycle_scripts( let should_save_side_effects_cache = side_effects_cache.should_save(); let overwrite_side_effects_cache = side_effects_cache.overwrite_existing(); let jail_policy = std::sync::Arc::new((*jail_policy).clone()); - let mut set: tokio::task::JoinSet> = tokio::task::JoinSet::new(); + // `(optional, spec, outcome)` rather than a bare result: the drain loop has + // to know whether the package that failed was optional-only, and an error + // surfacing from `join_next` carries no identity of its own. + let mut set: tokio::task::JoinSet<(bool, String, miette::Result)> = + tokio::task::JoinSet::new(); for job in jobs { let sem = semaphore.clone(); let project_dir = project_dir.clone(); let modules_dir_name = modules_dir_name.clone(); let node_gyp_bin_dir = node_gyp_bin_dir.clone(); let jail_policy = jail_policy.clone(); + let job_optional = optional_only.contains(&job.dep_path); + let job_spec = format!("{}@{}", job.name, job.version); let task = crate::dep_chain::scope_current(async move { let _permit = sem.acquire().await.unwrap(); if should_restore_side_effects_cache && let Some(cache_entry) = job.cache_entry.clone() @@ -812,17 +833,31 @@ pub(crate) async fn run_dep_lifecycle_scripts( }); let task = crate::runtime::scope_current(task); let task = aube_scripts::scope_current(task); - set.spawn(task); + set.spawn(async move { (job_optional, job_spec, task.await) }); } let mut ran = 0usize; while let Some(res) = set.join_next().await { - // `?` on the outer `Result` propagates a real task-level panic - // (tokio's `JoinError`); `?` on the inner `miette::Result` - // propagates a script failure. Either way, the function - // returns, `set` is dropped, and the remaining in-flight - // scripts are aborted before they can scribble on disk. - ran += res.into_diagnostic()??; + // A `JoinError` here is a task-level panic — an aube bug, not a package + // whose build failed — so it stays fatal even for an optional package. + let (optional, spec, outcome) = res.into_diagnostic()?; + match outcome { + Ok(count) => ran += count, + // An optional-only package's build failure is not the install's + // error: npm (`_handleOptionalFailure` during reify) and pnpm + // (`buildDependency`'s catch) both continue. Warned per package + // rather than swallowed — a native addon that silently never built + // is miserable to trace back from the runtime import error. + Err(error) if optional => { + tracing::warn!( + code = aube_codes::warnings::WARN_AUBE_OPTIONAL_BUILD_FAILED, + "{spec} is an optional dependency and failed to build; continuing without it: {error}" + ); + } + // A required package's failure returns immediately, as before: + // `set` is dropped and the not-yet-started jobs never run. + Err(error) => return Err(error), + } } Ok(ran) } diff --git a/vendor/aube/crates/aube/tests/e2e.rs b/vendor/aube/crates/aube/tests/e2e.rs index 4e240bb40..20ef77458 100644 --- a/vendor/aube/crates/aube/tests/e2e.rs +++ b/vendor/aube/crates/aube/tests/e2e.rs @@ -335,3 +335,123 @@ fn approve_builds_surfaces_and_runs_a_local_source_dep() { .success() .stdout(predicates::str::contains("No ignored builds")); } + +// --- optional-dependency build failures are non-fatal --------------------- +// +// A package reachable only through `optionalDependencies` is one the project +// declared it can live without, so npm (`_handleOptionalFailure`) and pnpm +// (`buildDependency`'s catch) both let its build fail without failing the +// install. Both tests use `file:` directory deps, so they are fully offline. + +#[test] +fn a_failing_optional_dependency_build_does_not_fail_the_install() { + let _guard = e2e_lock(); + let sbx = Sandbox::new(); + sbx.write_file( + "required-dep/package.json", + r#"{ + "name": "required-dep", + "version": "1.0.0", + "scripts": { "postinstall": "node -e \"require('fs').writeFileSync('REQUIRED_BUILT_MARKER','ok')\"" } + }"#, + ); + sbx.write_file( + "optional-dep/package.json", + r#"{ + "name": "optional-dep", + "version": "1.0.0", + "scripts": { "postinstall": "exit 1" } + }"#, + ); + sbx.write_manifest( + r#"{ + "name": "e2e-optional-build", + "version": "0.0.0", + "dependencies": { "required-dep": "file:./required-dep" }, + "optionalDependencies": { "optional-dep": "file:./optional-dep" } + }"#, + ); + + // The skip must be recorded, not hidden — a native addon that silently + // never built is miserable to trace back from the runtime import error. + sbx.cmd() + .args(["install", "--dangerously-allow-all-builds"]) + .assert() + .success() + .stderr(predicates::str::contains("optional-dep@1.0.0")); + + // Non-fatal must also mean non-blocking: the failure raises no + // short-circuit, so a sibling's build still runs to completion. + assert!( + marker_exists_under(&sbx.project.join("node_modules"), "REQUIRED_BUILT_MARKER"), + "a sibling package's build must still run after an optional build fails" + ); +} + +#[test] +fn a_failing_required_dependency_build_still_fails_the_install() { + // The control for the test above: if the optional carve-out ever widened to + // swallow every build failure, this is what would stop passing. + let _guard = e2e_lock(); + let sbx = Sandbox::new(); + sbx.write_file( + "required-dep/package.json", + r#"{ + "name": "required-dep", + "version": "1.0.0", + "scripts": { "postinstall": "exit 1" } + }"#, + ); + sbx.write_manifest( + r#"{ + "name": "e2e-required-build", + "version": "0.0.0", + "dependencies": { "required-dep": "file:./required-dep" } + }"#, + ); + + sbx.cmd() + .args(["install", "--dangerously-allow-all-builds"]) + .assert() + .failure() + .stderr(predicates::str::contains("required-dep")); +} + +#[test] +fn an_optional_dependency_that_is_also_required_still_fails_the_install() { + // `optional-dep` is declared BOTH optional (by the root) and as a plain + // dependency of `required-dep`, so one fully-required path reaches it and + // npm/pnpm both treat it as required. This is the case where a + // reachability bug would silently swallow a real failure. + let _guard = e2e_lock(); + let sbx = Sandbox::new(); + sbx.write_file( + "required-dep/package.json", + r#"{ + "name": "required-dep", + "version": "1.0.0", + "dependencies": { "optional-dep": "file:../optional-dep" } + }"#, + ); + sbx.write_file( + "optional-dep/package.json", + r#"{ + "name": "optional-dep", + "version": "1.0.0", + "scripts": { "postinstall": "exit 1" } + }"#, + ); + sbx.write_manifest( + r#"{ + "name": "e2e-dual-path-build", + "version": "0.0.0", + "dependencies": { "required-dep": "file:./required-dep" }, + "optionalDependencies": { "optional-dep": "file:./optional-dep" } + }"#, + ); + + sbx.cmd() + .args(["install", "--dangerously-allow-all-builds"]) + .assert() + .failure(); +} From 8b36a2669dde9cccac4bfaa846b2ae872ee4726a Mon Sep 17 00:00:00 2001 From: Colin McDonnell <3084745+colinhacks@users.noreply.github.com> Date: Sat, 15 Aug 2026 10:40:40 -0700 Subject: [PATCH 2/7] test(install): pin the optional-build assertions to the warning code and the lifecycle error MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-up. Three assertion and doc gaps, no behavior change. The dual-path control asserted a bare `.failure()`. `required-dep` reaches `optional-dep` through `file:../optional-dep`, which points outside its own directory, so a resolution failure would have satisfied the assertion while the reachability regression it guards went unnoticed. It now pins the lifecycle error and the package spec. The non-fatal test asserted only the spec string, which the underlying build error also carries — so it would have passed with the warning gone entirely. It now pins WARN_AUBE_OPTIONAL_BUILD_FAILED. The literal is safe because aube-codes' `every_const_value_matches_its_name` holds each constant equal to its name, and the crate is not a dev-dependency of the e2e target. The new code is added to docs/error-codes.data.json, the second of the two places a code's description lives. The one-shot skip is now recorded where it happens. Returning `Ok` lets `run_finalize_phase` reach `state::write_state`, which the pre-fix `?` never did, and `delta::fingerprint` carries nothing about build success — so a later up-to-date install skips the package and the warning does not repeat. Measured: a second `pnpm install` over a failed optional build is likewise silent, while npm re-runs the build. The retry the pre-fix code appeared to give was only a side effect of failing the install, which is the defect being fixed. aube e2e: 13 passed, 0 failed. Refs #660 --- .../aube/src/commands/install/lifecycle.rs | 15 +++++++++++++++ vendor/aube/crates/aube/tests/e2e.rs | 16 +++++++++++++++- vendor/aube/docs/error-codes.data.json | 6 ++++++ 3 files changed, 36 insertions(+), 1 deletion(-) diff --git a/vendor/aube/crates/aube/src/commands/install/lifecycle.rs b/vendor/aube/crates/aube/src/commands/install/lifecycle.rs index f89975c6b..75cb81348 100644 --- a/vendor/aube/crates/aube/src/commands/install/lifecycle.rs +++ b/vendor/aube/crates/aube/src/commands/install/lifecycle.rs @@ -848,6 +848,21 @@ pub(crate) async fn run_dep_lifecycle_scripts( // (`buildDependency`'s catch) both continue. Warned per package // rather than swallowed — a native addon that silently never built // is miserable to trace back from the runtime import error. + // + // Deliberately ONE-SHOT, which is the trade this fix makes. + // Returning `Ok` lets `run_finalize_phase` reach + // `state::write_state`, which the pre-fix `?` short-circuit never + // did. `delta::fingerprint` hashes resolve identity only and + // records nothing about build success, so the next install finds + // no delta and skips the package at the `selected_dep_paths` guard + // above — the warning does not repeat. That matches pnpm exactly + // (measured: a second `pnpm install` over a failed optional build + // is silent), though npm does re-run the build every install. The + // retry the pre-fix code appeared to offer was only a side effect + // of failing the install outright, which is the defect being fixed + // here. Re-warning across installs would need the package + // persisted into install state and re-emitted from the up-to-date + // short-circuit, the way `unreviewed_builds` already is. Err(error) if optional => { tracing::warn!( code = aube_codes::warnings::WARN_AUBE_OPTIONAL_BUILD_FAILED, diff --git a/vendor/aube/crates/aube/tests/e2e.rs b/vendor/aube/crates/aube/tests/e2e.rs index 20ef77458..e9c6efc32 100644 --- a/vendor/aube/crates/aube/tests/e2e.rs +++ b/vendor/aube/crates/aube/tests/e2e.rs @@ -374,10 +374,17 @@ fn a_failing_optional_dependency_build_does_not_fail_the_install() { // The skip must be recorded, not hidden — a native addon that silently // never built is miserable to trace back from the runtime import error. + // Pinned on the stable warning CODE, not the spec string: the underlying + // build error names the package too, so a spec-only assertion would pass + // even if the skip stopped being warned about at all. The literal is safe + // because `aube-codes`' `every_const_value_matches_its_name` self-test + // holds each constant's value equal to its name, and the crate is not a + // dev-dependency here. sbx.cmd() .args(["install", "--dangerously-allow-all-builds"]) .assert() .success() + .stderr(predicates::str::contains("WARN_AUBE_OPTIONAL_BUILD_FAILED")) .stderr(predicates::str::contains("optional-dep@1.0.0")); // Non-fatal must also mean non-blocking: the failure raises no @@ -450,8 +457,15 @@ fn an_optional_dependency_that_is_also_required_still_fails_the_install() { }"#, ); + // Pinned on the lifecycle error, not a bare `.failure()`. `required-dep` + // reaches `optional-dep` through `file:../optional-dep`, which points + // outside its own directory — so if that ever stopped resolving, a bare + // exit-code assertion would keep passing while testing nothing, and the + // reachability regression this guards would go unnoticed. sbx.cmd() .args(["install", "--dangerously-allow-all-builds"]) .assert() - .failure(); + .failure() + .stderr(predicates::str::contains("lifecycle script postinstall failed")) + .stderr(predicates::str::contains("optional-dep@1.0.0")); } diff --git a/vendor/aube/docs/error-codes.data.json b/vendor/aube/docs/error-codes.data.json index 20a9e6d0c..ec5a7e187 100644 --- a/vendor/aube/docs/error-codes.data.json +++ b/vendor/aube/docs/error-codes.data.json @@ -632,6 +632,12 @@ "description": "The `defaultTrust` floor let listed packages run build scripts without an explicit `allowBuilds` entry. Disclosure, not an error — set `defaultTrust=false` or an explicit `allowBuilds: false` entry to opt out.", "exit_code": null }, + { + "name": "WARN_AUBE_OPTIONAL_BUILD_FAILED", + "category": "Install lifecycle", + "description": "A package reachable only through `optionalDependencies` failed to build. The install continues without it, matching npm and pnpm. Emitted on the install that runs the build; a later up-to-date install skips the package and does not repeat it.", + "exit_code": null + }, { "name": "WARN_AUBE_SUSPICIOUS_LIFECYCLE_SCRIPT", "category": "Install lifecycle", From 1e99bf679d9b3113a6bea0699639c30b127d509c Mon Sep 17 00:00:00 2001 From: Colin McDonnell <3084745+colinhacks@users.noreply.github.com> Date: Sat, 15 Aug 2026 10:48:19 -0700 Subject: [PATCH 3/7] wiki: record what the optional-build fix shipped, and what case 4 still owes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The script-failure-readout design is where this behavior is specified, so it moves with the behavior. Case 4 called the exit code "the highest-value item in this document"; that half is now fixed, and the document said the opposite. Its recommendation follows npm on the substance — unlink the failed optional package — and what shipped follows pnpm and leaves it linked, so the comparison table gains a "Nub today" column rather than reading as though the case were closed. The unlink is scoped, not silently dropped: it means removing the package's symlink from every consumer in the isolated store and withholding its bins, which is linker work rather than a change to the lifecycle runner. The measured re-run bullet no longer holds for an optional dependency. A successful install now writes install state, and `delta::fingerprint` records resolve identity only, so the delta filter skips the package on every later install and the warning is emitted once. The unlink would restore the retry by making the package absent rather than merely unbuilt. Refs #660 --- wiki/design/install-script-failure-readout.md | 23 ++++++++++++------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/wiki/design/install-script-failure-readout.md b/wiki/design/install-script-failure-readout.md index 40d2381a3..8c8e285cf 100644 --- a/wiki/design/install-script-failure-readout.md +++ b/wiki/design/install-script-failure-readout.md @@ -32,7 +32,7 @@ Exit code 1. Three packages failed; one is named. The stray `failing-a: build fa Three further facts, each measured: - Linking happens before scripts, so a package whose build failed is still present in `node_modules`. -- A re-run retries the failed build rather than fast-pathing over it. +- A re-run retries the failed build rather than fast-pathing over it. This holds for a *required* dependency, whose failure still aborts the install before install state is written. It no longer holds for an optional one: since [#737](https://github.com/nubjs/nub/pull/737) that install succeeds, state is written, and the delta filter skips the package on every later install, so the warning is emitted once and not repeated. Case 4's unlink is what would restore the retry, by making the package absent rather than merely unbuilt. - The error carries no `ERR_*` code, so it misses `EXIT_TABLE` and falls through to the generic exit 1. ## Prior art @@ -115,7 +115,11 @@ The package stays in `node_modules`, half-built, and the summary lists it as ins ### Where Nub is today -Nub makes no required/optional distinction: the same optional dependency fails the whole install with exit 1 — the one place Nub refuses an install both reference tools complete. +Nub made no required/optional distinction: an optional dependency whose build failed took the whole install down with exit 1 — the one place Nub refused an install both reference tools complete. + +[#737](https://github.com/nubjs/nub/pull/737) closed the exit-code half. A package reachable only through optional edges now warns under `WARN_NUB_OPTIONAL_BUILD_FAILED` and the install exits 0, with optionality derived from the graph exactly as this document defines it below — one fully-required path keeps a package required. + +The reporting half of case 4 is unbuilt, and so is its substance: the package is still left linked, which is pnpm's behavior rather than the npm behavior recommended below. ## The design @@ -266,15 +270,17 @@ nub 0.6.0 · ✓ installed 200 packages in 1.2s Exit code 0. -**This should not be an error, and today it is.** Both reference tools exit 0 here; Nub exits 1 and fails the install. That is a compatibility defect, not a stricter-by-design choice, and it is the highest-value item in this document. +**This should not be an error, and until [#737](https://github.com/nubjs/nub/pull/737) it was.** Both reference tools exit 0 here; Nub exited 1 and failed the install. That was a compatibility defect, not a stricter-by-design choice, and it was the highest-value item in this document. The exit code is fixed; the rest of this case is not. The recommendation follows npm on the substance and neither tool on the reporting: -| | npm | pnpm | Proposed | -| --- | --- | --- | --- | -| Exit code | 0 | 0 | 0 | -| Package left in `node_modules` | No | Yes | No | -| Named in the output | No | Only as installed | Yes, with the cause | +| | npm | pnpm | Proposed | Nub today | +| --- | --- | --- | --- | --- | +| Exit code | 0 | 0 | 0 | 0 | +| Package left in `node_modules` | No | Yes | No | Yes | +| Named in the output | No | Only as installed | Yes, with the cause | Yes, with the cause | + +Nub matches the proposal on the exit code and on naming the failure, and matches pnpm rather than the proposal on leaving the package linked. The unlink was deliberately not attempted in [#737](https://github.com/nubjs/nub/pull/737): it means removing the package's symlink from every consumer in the isolated store and withholding its bins, which is linker blast radius rather than a change to the lifecycle runner, and it pairs naturally with the log file and summary this case also proposes. Until it lands, the argument below still describes a real gap in Nub, not only in pnpm. Removing the package is what makes optionality work, and leaving it linked converts a handled condition into an unhandled one. A consumer guards an optional dependency with a `try`/`catch` around the require, so an absent package degrades gracefully while a present-but-broken one throws from inside the module — past the guard, at some later point, with an error that names neither the install nor the failed build. @@ -358,3 +364,4 @@ The summary is strictly last, after the drained output of every build that was s Every revision to this document, with the date and what changed. - 2026-08-01 — Initial write-up. +- 2026-08-15 — Case 4's exit code shipped in [#737](https://github.com/nubjs/nub/pull/737): an optional-only build failure warns and the install exits 0, with optionality derived from the graph as this document defines it. Recorded what that leaves outstanding — the package is still left linked, against this document's npm-following recommendation — and corrected the re-run bullet, which no longer holds for an optional dependency now that a successful install writes state the delta filter reads. From d0261ec012f47edb2f345a98fd707ef427f977ea Mon Sep 17 00:00:00 2001 From: Colin McDonnell <3084745+colinhacks@users.noreply.github.com> Date: Sat, 15 Aug 2026 10:50:41 -0700 Subject: [PATCH 4/7] docs(install): document what happens when a dependency build fails MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The lifecycle-scripts section covered which packages are allowed to build and what happens when one is denied, but not what happens when an allowed build fails — which is now two different outcomes depending on how the package is reached. States the optional carve-out with its warning code, that optionality is a property of the edge so a package anything requires still fails the install, and that the warning comes from the install that runs the build rather than repeating on later no-op installs, with `nub rebuild ` as the way back to it. The sample is captured output, not a mockup. Deliberately does not claim the exit code is the failing script's own: for a required dependency Nub exits 1 today where npm and pnpm exit 3 (#670). Refs #660 --- site/content/docs/install/index.mdx | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/site/content/docs/install/index.mdx b/site/content/docs/install/index.mdx index 39b6160a7..658d7644d 100644 --- a/site/content/docs/install/index.mdx +++ b/site/content/docs/install/index.mdx @@ -464,6 +464,18 @@ The neutral `allowBuilds` field grants permission, and it keys on the package na That field applies in any project, whoever owns it. A project owned by another package manager grants permission through its own field as well: pnpm projects use `pnpm.onlyBuiltDependencies` / `pnpm.allowBuilds`, and Bun projects use `trustedDependencies`. A package that wants to build but isn't allowed is skipped, with `WARN_NUB_IGNORED_BUILD_SCRIPTS` naming it and `nub approve-builds` as the remedy. +### When a build fails + +A failed build fails the install. The one exception is a package reachable only through `optionalDependencies`, which the project has declared it can work without: that failure is reported and the install continues, matching npm and pnpm. + +```bash +WARN optfail@1.0.0 is an optional dependency and failed to build; continuing without it: lifecycle script postinstall failed for optfail@1.0.0: script `postinstall` exited with code 3 code=WARN_NUB_OPTIONAL_BUILD_FAILED +``` + +Optionality is a property of the edge, not the package. A package that anything reaches through a normal dependency is required, and its build failure still fails the install, even when something else depends on it optionally. + +The warning is emitted by the install that runs the build. A later install with nothing to do skips the package along with everything else, so it does not repeat. Run `nub rebuild ` to attempt the build again and see the failure. + ### Cooling window A registry-resolved version must be older than `minimumReleaseAge` — 24 hours by default — before Nub will install it. The window is what keeps a compromised publish out of your tree during the hours between it going up and being caught. From 215c2a6589cf871334808644235c8d90c0ddf6ba Mon Sep 17 00:00:00 2001 From: Colin McDonnell <3084745+colinhacks@users.noreply.github.com> Date: Sat, 15 Aug 2026 10:58:19 -0700 Subject: [PATCH 5/7] fix(codes): author the optional-build description in the registry, not the generated JSON MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `docs/error-codes.data.json` is generated — `generate_error_codes_docs.rs` says hand-edits "will be clobbered on the next `mise run render`. Update the registry instead." The entry added by hand would have been overwritten, and its prose had already diverged from the `CodeMeta` that is the actual source. The one-shot note now lives in the `CodeMeta` description and the JSON is regenerated from it. That also clears pre-existing drift: the file carried 83 warnings against 84 in `warnings::ALL`, so `WARN_AUBE_NODE_GYP_BOOTSTRAP_FAILED` was missing and is now present. The e2e assertion references `aube_codes::warnings::WARN_AUBE_OPTIONAL_BUILD_FAILED` rather than a literal. Both halves of the justification for using a literal were wrong: `aube-codes` is a normal dependency of this package and Cargo makes those available to integration-test targets (`aube-lockfile`'s own tests use `aube_manifest` the same way), and `every_warning_const_value_matches_its_name` asserts only the `WARN_AUBE_` prefix and non-empty metadata — nothing ties a constant's value to its identifier, so a rename would have unhooked the literal silently while every self-test passed. aube e2e: 13 passed, 0 failed. Refs #660 --- vendor/aube/crates/aube-codes/src/warnings.rs | 2 +- vendor/aube/crates/aube/tests/e2e.rs | 12 ++++++----- vendor/aube/docs/error-codes.data.json | 20 ++++++++++++++++++- 3 files changed, 27 insertions(+), 7 deletions(-) diff --git a/vendor/aube/crates/aube-codes/src/warnings.rs b/vendor/aube/crates/aube-codes/src/warnings.rs index 1673fe270..95ec2590c 100644 --- a/vendor/aube/crates/aube-codes/src/warnings.rs +++ b/vendor/aube/crates/aube-codes/src/warnings.rs @@ -209,7 +209,7 @@ pub const ALL: &[CodeMeta] = &[ CodeMeta { name: WARN_AUBE_OPTIONAL_BUILD_FAILED, category: category::INSTALL_LIFECYCLE, - description: "A package reachable only through `optionalDependencies` failed to build. The install continues without it (npm and pnpm both treat an optional build failure as non-fatal). Anything importing it at runtime will fail — declare it a regular dependency if it is actually required.", + description: "A package reachable only through `optionalDependencies` failed to build. The install continues without it (npm and pnpm both treat an optional build failure as non-fatal). Anything importing it at runtime will fail — declare it a regular dependency if it is actually required. Emitted by the install that runs the build: a later up-to-date install skips the package and does not repeat this, so run `rebuild ` to attempt the build again.", exit_code: None, }, CodeMeta { diff --git a/vendor/aube/crates/aube/tests/e2e.rs b/vendor/aube/crates/aube/tests/e2e.rs index e9c6efc32..fa51c2bb5 100644 --- a/vendor/aube/crates/aube/tests/e2e.rs +++ b/vendor/aube/crates/aube/tests/e2e.rs @@ -376,15 +376,17 @@ fn a_failing_optional_dependency_build_does_not_fail_the_install() { // never built is miserable to trace back from the runtime import error. // Pinned on the stable warning CODE, not the spec string: the underlying // build error names the package too, so a spec-only assertion would pass - // even if the skip stopped being warned about at all. The literal is safe - // because `aube-codes`' `every_const_value_matches_its_name` self-test - // holds each constant's value equal to its name, and the crate is not a - // dev-dependency here. + // even if the skip stopped being warned about at all. Referenced through + // the constant rather than a literal — `aube-codes` is a normal dependency + // of this package, so an integration test can name it, and no self-test + // ties a constant's value to its identifier. sbx.cmd() .args(["install", "--dangerously-allow-all-builds"]) .assert() .success() - .stderr(predicates::str::contains("WARN_AUBE_OPTIONAL_BUILD_FAILED")) + .stderr(predicates::str::contains( + aube_codes::warnings::WARN_AUBE_OPTIONAL_BUILD_FAILED, + )) .stderr(predicates::str::contains("optional-dep@1.0.0")); // Non-fatal must also mean non-blocking: the failure raises no diff --git a/vendor/aube/docs/error-codes.data.json b/vendor/aube/docs/error-codes.data.json index ec5a7e187..46e85b5c6 100644 --- a/vendor/aube/docs/error-codes.data.json +++ b/vendor/aube/docs/error-codes.data.json @@ -180,6 +180,12 @@ "description": "Registry returned 401 — missing or invalid auth. Run `aube login`.", "exit_code": 42 }, + { + "name": "ERR_AUBE_METADATA_DECODE", + "category": "Registry / network", + "description": "A registry response body failed to decode. Carries the package it belongs to and an excerpt of the payload around the offending field.", + "exit_code": null + }, { "name": "ERR_AUBE_FORBIDDEN", "category": "Registry / network", @@ -342,6 +348,12 @@ "description": "An `aube-workspace.yaml` / `pnpm-workspace.yaml` was structurally invalid.", "exit_code": 71 }, + { + "name": "ERR_AUBE_WORKSPACE_PKG_NOT_FOUND", + "category": "Manifest / workspace", + "description": "A `workspace:` dependency named a package that is not a member of this workspace.", + "exit_code": null + }, { "name": "ERR_AUBE_MANIFEST_YAML_PARSE", "category": "Manifest / workspace", @@ -635,7 +647,13 @@ { "name": "WARN_AUBE_OPTIONAL_BUILD_FAILED", "category": "Install lifecycle", - "description": "A package reachable only through `optionalDependencies` failed to build. The install continues without it, matching npm and pnpm. Emitted on the install that runs the build; a later up-to-date install skips the package and does not repeat it.", + "description": "A package reachable only through `optionalDependencies` failed to build. The install continues without it (npm and pnpm both treat an optional build failure as non-fatal). Anything importing it at runtime will fail — declare it a regular dependency if it is actually required. Emitted by the install that runs the build: a later up-to-date install skips the package and does not repeat this, so run `rebuild ` to attempt the build again.", + "exit_code": null + }, + { + "name": "WARN_AUBE_NODE_GYP_BOOTSTRAP_FAILED", + "category": "Install lifecycle", + "description": "Jailed builds were requested but node-gyp could not be prepared for them (a jailed script cannot bootstrap it itself). The install continues: builds that don't use node-gyp are unaffected, and one that does will fail with node-gyp's own error.", "exit_code": null }, { From 8b94cb64cad3e22e52f215a145320e52d468dd9f Mon Sep 17 00:00:00 2001 From: Colin McDonnell <3084745+colinhacks@users.noreply.github.com> Date: Sat, 15 Aug 2026 10:58:20 -0700 Subject: [PATCH 6/7] docs(install): match the sibling captured-output pattern for the build-failure sample The other two warning samples on this page use a `console` fence carrying a `# captured:` provenance header and the command that produced the output, which is what lets a reader check the real-output-only rule instead of trusting it. This block was the page's only `bash`-fenced warning sample and carried no provenance. The header names the fixture rather than a version: the behavior is not in a published release yet, so citing one would send a reader to a build that does not have it. Refs #660 --- site/content/docs/install/index.mdx | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/site/content/docs/install/index.mdx b/site/content/docs/install/index.mdx index 658d7644d..189b9ee23 100644 --- a/site/content/docs/install/index.mdx +++ b/site/content/docs/install/index.mdx @@ -468,8 +468,14 @@ That field applies in any project, whoever owns it. A project owned by another p A failed build fails the install. The one exception is a package reachable only through `optionalDependencies`, which the project has declared it can work without: that failure is reported and the install continues, matching npm and pnpm. -```bash -WARN optfail@1.0.0 is an optional dependency and failed to build; continuing without it: lifecycle script postinstall failed for optfail@1.0.0: script `postinstall` exited with code 3 code=WARN_NUB_OPTIONAL_BUILD_FAILED +```console +# captured: optfail@1.0.0 whose postinstall exits 3, allowBuilds entry present +$ nub install +WARN optfail@1.0.0 is an optional dependency and failed to build; continuing + without it: lifecycle script postinstall failed for optfail@1.0.0: script + `postinstall` exited with code 3 code=WARN_NUB_OPTIONAL_BUILD_FAILED +optionalDependencies: ++ optfail@1.0.0 ``` Optionality is a property of the edge, not the package. A package that anything reaches through a normal dependency is required, and its build failure still fails the install, even when something else depends on it optionally. From 7b8ee5398b0ce048f80d92fd134e05c2122c3d2d Mon Sep 17 00:00:00 2001 From: Colin McDonnell <3084745+colinhacks@users.noreply.github.com> Date: Sat, 15 Aug 2026 11:08:51 -0700 Subject: [PATCH 7/7] docs: name the binary in the rebuild hint and the nub version in the captured sample Two consistency fixes from review. Every other command reference in the code registry names the binary (`aube add`, `Run `aube approve-builds``, `aube update`), so the bare `rebuild ` in the optional-build description read as an unqualified verb on the rendered /error-codes page. It names the binary now, and the JSON is regenerated from the registry rather than edited. The captured docs sample gets the nub version its sibling headers carry. It was left off on the grounds that the behavior is in no published release, which confused two different things: the header records the version the output was captured on, not the version required to reproduce it, and every sibling header is a historical capture on that same reading. Refs #660 --- site/content/docs/install/index.mdx | 2 +- vendor/aube/crates/aube-codes/src/warnings.rs | 2 +- vendor/aube/docs/error-codes.data.json | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/site/content/docs/install/index.mdx b/site/content/docs/install/index.mdx index 189b9ee23..02bb985f8 100644 --- a/site/content/docs/install/index.mdx +++ b/site/content/docs/install/index.mdx @@ -469,7 +469,7 @@ That field applies in any project, whoever owns it. A project owned by another p A failed build fails the install. The one exception is a package reachable only through `optionalDependencies`, which the project has declared it can work without: that failure is reported and the install continues, matching npm and pnpm. ```console -# captured: optfail@1.0.0 whose postinstall exits 3, allowBuilds entry present +# captured: nub 0.7.5, optfail@1.0.0 whose postinstall exits 3, allowBuilds entry present $ nub install WARN optfail@1.0.0 is an optional dependency and failed to build; continuing without it: lifecycle script postinstall failed for optfail@1.0.0: script diff --git a/vendor/aube/crates/aube-codes/src/warnings.rs b/vendor/aube/crates/aube-codes/src/warnings.rs index 95ec2590c..a2f47a9c3 100644 --- a/vendor/aube/crates/aube-codes/src/warnings.rs +++ b/vendor/aube/crates/aube-codes/src/warnings.rs @@ -209,7 +209,7 @@ pub const ALL: &[CodeMeta] = &[ CodeMeta { name: WARN_AUBE_OPTIONAL_BUILD_FAILED, category: category::INSTALL_LIFECYCLE, - description: "A package reachable only through `optionalDependencies` failed to build. The install continues without it (npm and pnpm both treat an optional build failure as non-fatal). Anything importing it at runtime will fail — declare it a regular dependency if it is actually required. Emitted by the install that runs the build: a later up-to-date install skips the package and does not repeat this, so run `rebuild ` to attempt the build again.", + description: "A package reachable only through `optionalDependencies` failed to build. The install continues without it (npm and pnpm both treat an optional build failure as non-fatal). Anything importing it at runtime will fail — declare it a regular dependency if it is actually required. Emitted by the install that runs the build: a later up-to-date install skips the package and does not repeat this, so run `aube rebuild ` to attempt the build again.", exit_code: None, }, CodeMeta { diff --git a/vendor/aube/docs/error-codes.data.json b/vendor/aube/docs/error-codes.data.json index 46e85b5c6..db8b2b3a6 100644 --- a/vendor/aube/docs/error-codes.data.json +++ b/vendor/aube/docs/error-codes.data.json @@ -647,7 +647,7 @@ { "name": "WARN_AUBE_OPTIONAL_BUILD_FAILED", "category": "Install lifecycle", - "description": "A package reachable only through `optionalDependencies` failed to build. The install continues without it (npm and pnpm both treat an optional build failure as non-fatal). Anything importing it at runtime will fail — declare it a regular dependency if it is actually required. Emitted by the install that runs the build: a later up-to-date install skips the package and does not repeat this, so run `rebuild ` to attempt the build again.", + "description": "A package reachable only through `optionalDependencies` failed to build. The install continues without it (npm and pnpm both treat an optional build failure as non-fatal). Anything importing it at runtime will fail — declare it a regular dependency if it is actually required. Emitted by the install that runs the build: a later up-to-date install skips the package and does not repeat this, so run `aube rebuild ` to attempt the build again.", "exit_code": null }, {