Description

The frozen up-to-date fast path returns Ok(()) without checking for previously-recorded ignored
builds under strict_dep_builds, so rerunning install after an ERR_PNPM_IGNORED_BUILDS failure
can exit 0 and undermine enforcement. Ignored builds are persisted into .modules.yaml and the
strict failure is only raised at the very end of the non-no-op install flow.

Code

pacquet/crates/package-manager/src/install.rs[R970-979]

&& let Some(current) = current_lockfile.as_ref()
&& wanted_lockfile == current
&& is_modules_yaml_consistent(&config.modules_dir, config, node_linker, included)
+            // An `allowBuilds` change that now permits a previously-ignored
+            // build must rebuild it, even though the lockfile and layout are
+            // unchanged. Mirrors pnpm's `runUnignoredDependencyBuilds`.
+            && !has_newly_allowed_ignored_builds(&config.modules_dir, config)
{
// Nothing to materialize means no fetch to overlap; verify
// eagerly before the up-to-date early return.

Evidence

The no-op short-circuit returns success before the strict ignored-builds gate, even though the
install persists ignored builds into .modules.yaml and later throws ERR_PNPM_IGNORED_BUILDS only
at the tail of the full install flow.

pacquet/crates/package-manager/src/install.rs[958-1019]
pacquet/crates/package-manager/src/install.rs[1254-1266]
pacquet/crates/package-manager/src/install.rs[1378-1386]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`Install::run` has a frozen no-op fast path that returns early when the lockfile and `.modules.yaml` match. Under `strictDepBuilds`, this path should still fail if there are any previously ignored dependency builds recorded in `.modules.yaml` (until the user approves builds), otherwise users can simply rerun install to avoid the non-zero exit.
### Issue Context
- Ignored builds are written into `.modules.yaml` via `build_modules_manifest(..., &ignored_builds)`.
- Strict mode currently raises `InstallError::IgnoredBuilds` only at the tail of the full install path.
### Fix Focus Areas
- pacquet/crates/package-manager/src/install.rs[958-1019]
- pacquet/crates/package-manager/src/install.rs[1254-1266]
- pacquet/crates/package-manager/src/install.rs[1378-1386]
### Implementation notes
- In the no-op short-circuit block, when `config.strict_dep_builds` is true:
- read `.modules.yaml` (you can reuse `read_modules_manifest::<Host>(&config.modules_dir)`),
- if `modules.ignored_builds` is `Some` and non-empty, convert to a sorted `Vec<String>` (matching the rest of the strict error behavior),
- return `Err(InstallError::IgnoredBuilds { package_names })`.
- Keep the existing `has_newly_allowed_ignored_builds` behavior intact (it handles the rebuild-needed case).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

pacquet_default_reporter::set_strict_dep_builds is called before run_update_config_hooks, but
those hooks can mutate cfg.strict_dep_builds; because the reporter caches the value in a
OnceLock, it cannot be updated later. This can suppress the “Ignored build scripts” warning when
the final config is non-strict (install exits 0) or show the warning even though the final config is
strict (install fails with ERR_PNPM_IGNORED_BUILDS).

Code

pacquet/crates/cli/src/cli_args.rs[325]

+                pacquet_default_reporter::set_strict_dep_builds(cfg.strict_dep_builds);

Evidence

The CLI seeds the reporter from the pre-hook config, but updateConfig hooks can change
strict_dep_builds afterward; the reporter cannot be updated because it stores the value in a
OnceLock. The install’s strictness gate uses the (possibly updated) config.strict_dep_builds, so
reporter output can diverge from actual install behavior (error vs warning).

pacquet/crates/cli/src/cli_args.rs[270-370]
pacquet/crates/cli/src/cli_args.rs[447-488]
pacquet/crates/cli/src/config_deps.rs[206-306]
pacquet/crates/config/src/workspace_yaml.rs[820-846]
pacquet/crates/default-reporter/src/lib.rs[62-73]
pacquet/crates/package-manager/src/install.rs[1373-1381]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The default reporter’s `strictDepBuilds` mode is currently seeded from `cfg.strict_dep_builds` before config-dependency `updateConfig` hooks run. Since hooks can modify `strict_dep_builds`, the reporter can become permanently out of sync (it uses `OnceLock`), causing incorrect/missing ignored-build messaging.
## Issue Context
- `run_update_config_hooks` applies a `WorkspaceSettings` delta back onto `Config`, including `strict_dep_builds`.
- The install behavior (error vs warning) is gated on `config.strict_dep_builds` at the end of `Install::run`.
- The default reporter suppresses the ignored-build warning box when it believes strict mode is enabled.
## Fix Focus Areas
- pacquet/crates/cli/src/cli_args.rs[314-488]
- pacquet/crates/cli/src/config_deps.rs[206-306]
- pacquet/crates/default-reporter/src/lib.rs[62-73]
## Recommended fix
1. Remove the early `pacquet_default_reporter::set_strict_dep_builds(cfg.strict_dep_builds);` call in the `CliCommand::Install` branch.
2. Add a call to `pacquet_default_reporter::set_strict_dep_builds(cfg.strict_dep_builds);` inside `InstallPipeline::run` **after** `config_deps::run_update_config_hooks::<Reporter>(...)` and **before** `State::init(...)` / the main install emits the `IgnoredScripts` event.
3. If keeping the “must be called before the first event” contract is important, change the reporter’s storage from `OnceLock<bool>` to an updatable primitive (e.g., `AtomicBool`) so you can seed early and then re-seed after hooks; otherwise update the comment to reflect the real requirement (“before IgnoredScripts render”).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

unapproved_recorded_ignored_builds uses AllowBuildPolicy::from_config(config).ok()?, so invalid
allowBuilds specs are treated as “no unapproved ignored builds” on strict up-to-date fast paths.
This can hide ERR_PNPM_INVALID_VERSION_UNION/ERR_PNPM_NAME_PATTERN_IN_VERSION_UNION and may let
strict reruns short-circuit to success instead of enforcing ERR_PNPM_IGNORED_BUILDS.

Code

pacquet/crates/package-manager/src/install.rs[R1742-1748]

+fn unapproved_recorded_ignored_builds(modules: &Modules, config: &Config) -> Option<Vec<String>> {
+    let ignored = modules.ignored_builds.as_ref().filter(|set| !set.is_empty())?;
+    let policy = crate::AllowBuildPolicy::from_config(config).ok()?;
+    let mut names: Vec<String> = ignored
+        .iter()
+        .filter(|dep_path| policy.check(dep_path.as_str()).is_none())
+        .map(|dep_path| dep_path.as_str().to_string())

Evidence

The strict fast-path helper explicitly drops AllowBuildPolicy::from_config errors (.ok()?), even
though from_config is documented and tested to surface invalid allowBuilds keys as errors.

pacquet/crates/package-manager/src/install.rs[1742-1751]
pacquet/crates/package-manager/src/build_modules.rs[159-203]
pacquet/crates/package-manager/src/build_modules/tests.rs[207-228]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`unapproved_recorded_ignored_builds()` converts `AllowBuildPolicy::from_config()` errors into `None` (`.ok()?`). Callers interpret `None` as “no unapproved ignored builds”, which can let strict up-to-date fast paths return success even when config is invalid and/or ignored builds are recorded.
## Issue Context
`AllowBuildPolicy::from_config` is designed to *surface* invalid `allowBuilds` keys as typed `VersionPolicyError` (e.g. `ERR_PNPM_INVALID_VERSION_UNION`). The helper should not silently drop these errors in strict enforcement logic.
## Fix Focus Areas
- pacquet/crates/package-manager/src/install.rs[1742-1752]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

run_build_phase suppresses emitting the pnpm:ignored-scripts event when strict_dep_builds is
true and the ignored list is non-empty, so structured/NDJSON consumers lose the ignored-build list
on the strict failure path. This contradicts the reporter contract that the event is emitted with
the names of skipped packages, and it makes strict failures less observable except via the rendered
error text.

Code

pacquet/crates/package-manager/src/install_frozen_lockfile.rs[R477-494]

+    // Emit the `pnpm:ignored-scripts` event so the reporter can render
+    // the "Ignored build scripts" warning box, mirroring pnpm's
+    // `reportIgnoredBuilds`. The empty-list event is always emitted
+    // (pnpm fires `ignoredScriptsLogger.debug({ packageNames: [] })`
+    // unconditionally at
+    // <https://github.com/pnpm/pnpm/blob/80037699fb/installing/deps-installer/src/install/index.ts#L526>),
+    // but a non-empty list is suppressed under `strictDepBuilds`: there
+    // the install fails with `ERR_PNPM_IGNORED_BUILDS` (raised by the
+    // caller from the returned list), so the box would only duplicate
+    // the error. Gating here — where `config` is the final, post-`updateConfig`
+    // value the strict-failure check also reads — keeps the warning and
+    // the error decision consistent without a reporter-side flag.
+    if ignored_builds.is_empty() || !config.strict_dep_builds {
+        Reporter::emit(&LogEvent::IgnoredScripts(IgnoredScriptsLog {
+            level: LogLevel::Debug,
+            package_names: ignored_builds.clone(),
+        }));
+    }

Evidence

The build-phase emitter explicitly suppresses pnpm:ignored-scripts under strict mode when there
are ignored builds, while the reporter schema documents the event as being emitted with the skipped
package list.

pacquet/crates/package-manager/src/install_frozen_lockfile.rs[477-494]
pacquet/crates/reporter/src/lib.rs[591-601]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`run_build_phase` currently gates `LogEvent::IgnoredScripts` behind `if ignored_builds.is_empty() || !config.strict_dep_builds`, which means the event is not emitted exactly when strict mode blocks builds (non-empty ignored list). This breaks the documented contract of `pnpm:ignored-scripts` being emitted with the skipped package list, and removes structured data from NDJSON output on the strict-failure path.
### Issue Context
- The log schema doc says `pnpm:ignored-scripts` is emitted with the names of every skipped package.
- The default reporter UI may want to suppress the warning box in strict mode, but that policy should not require dropping the structured event entirely.
### Fix Focus Areas
- pacquet/crates/package-manager/src/install_frozen_lockfile.rs[477-494]
- pacquet/crates/reporter/src/lib.rs[591-601]
- pacquet/crates/default-reporter/src/state.rs[846-859]
### Suggested fix approach
1. Emit `LogEvent::IgnoredScripts` unconditionally with the full `ignored_builds` list (including under strict mode).
2. Move the “suppress the warning box under strict” behavior into the default reporter (or into a separate dedicated event/field that indicates strict mode), so NDJSON/structured consumers still receive the ignored-build list.
3. If suppression-at-emitter is intentional, update the reporter schema documentation (and any tests/contracts) to explicitly state the event may be omitted under strict mode, and provide an alternate structured channel for the list on strict failures.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

In Install::run and install_already_up_to_date, strictDepBuilds enforcement uses
read_modules_manifest(...).ok().flatten(), so a corrupted/unreadable node_modules/.modules.yaml is
treated as “missing” and the up-to-date fast paths can exit successfully even if ignored builds were
previously recorded. This bypasses ERR_PNPM_IGNORED_BUILDS on reruns specifically when .modules.yaml
cannot be parsed/read.

Code

pacquet/crates/package-manager/src/install.rs[R617-622]

+            if config.strict_dep_builds
+                && let Some(modules) =
+                    read_modules_manifest::<Host>(&config.modules_dir).ok().flatten()
+                && let Some(package_names) = unapproved_recorded_ignored_builds(&modules, config)
+            {
+                return Err(InstallError::IgnoredBuilds { package_names });

Evidence

The fast paths explicitly discard read/parse errors from .modules.yaml via .ok().flatten(), but
read_modules_manifest returns Err on YAML parse errors and non-NotFound I/O failures; discarding
these errors means strict-mode enforcement can be skipped when the manifest is corrupted/unreadable.

pacquet/crates/package-manager/src/install.rs[609-623]
pacquet/crates/package-manager/src/install.rs[1967-1979]
pacquet/crates/modules-yaml/src/lib.rs[371-386]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`strictDepBuilds` enforcement on the up-to-date fast paths currently does `read_modules_manifest(..).ok().flatten()`, which drops read/parse errors. If `.modules.yaml` is corrupted/unreadable, the install may incorrectly short-circuit as up-to-date under strict mode, bypassing the intended `ERR_PNPM_IGNORED_BUILDS` rerun enforcement.
### Issue Context
`read_modules_manifest` returns `Err(ReadModulesError::ReadFile|ParseYaml|...)` for non-NotFound failures. Those errors are currently swallowed by `.ok()`.
### Fix Focus Areas
- pacquet/crates/package-manager/src/install.rs[610-623]
- pacquet/crates/package-manager/src/install.rs[1967-1979]
### Suggested change
1) In `Install::run` optimistic fast-path block:
- Replace `.ok().flatten()` with explicit handling:
- `Ok(Some(modules))`: keep current behavior.
- `Ok(None)`: no recorded ignored builds.
- `Err(_)`: **do not** short-circuit as up-to-date; fall through to full install (e.g., set a `must_run_full_install = true` and skip the `Already up to date` early return).
2) In `install_already_up_to_date`:
- If `config.strict_dep_builds` and `read_modules_manifest` returns `Err(_)`, return `None` (force the full async install path), rather than allowing the pre-runtime up-to-date short-circuit.
This keeps strict enforcement conservative when the state file is unreadable, aligning with the intent of strictDepBuilds as an enforcement gate.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Walkthrough

Implements pnpm's strictDepBuilds setting: adds strict_dep_builds: bool to Config, readable from pnpm-workspace.yaml and PNPM_CONFIG_STRICT_DEP_BUILDS. Extracts a shared run_build_phase helper from the frozen-lockfile path and wires both frozen and fresh install pipelines to propagate ignored_builds. The install dispatcher persists ignored builds to .modules.yaml, bypasses the fast-path when builds become newly allowed, and fails with ERR_PNPM_IGNORED_BUILDS under strict mode. Tests are migrated to workspace-YAML configuration with new regression coverage.

Changes

Strict Dependency Builds Feature

Sequence Diagram(s)

sequenceDiagram
  actor User
  participant Install as Install::run
  participant HNAI as has_newly_allowed_ignored_builds
  participant BuildPhase as run_build_phase
  participant Modules as .modules.yaml

  User->>Install: pacquet install / pacquet add
  Install->>HNAI: read Modules, check allowBuilds policy
  HNAI-->>Install: fast-path bypassed if newly allowed
  Install->>BuildPhase: run lifecycle scripts (frozen or fresh path)
  BuildPhase-->>Install: ignored_builds Vec~String~
  Install->>Modules: build_modules_manifest(ignored_builds)
  alt strict_dep_builds=true and ignored_builds non-empty
    Install-->>User: ERR_PNPM_IGNORED_BUILDS
  else
    Install-->>User: success (optional warning)
  end

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

• pnpm/pnpm#11904: Modifies the same "lockfile up-to-date" fast-path logic in install.rs and extends .modules.yaml state consultation for deciding whether Install::run can skip materialization.
• pnpm/pnpm#11867: Modifies the fresh-lockfile phased pipeline after CreateVirtualStore to adjust symlinking and linking sequencing before lifecycle build execution, overlapping with this PR's build-phase integration refactoring.
• pnpm/pnpm#11943: Modifies the installer's optimistic/"already up to date" short-circuit logic in the same install.rs file, adjusting the fast-path conditions that this PR extends with strict_dep_builds and ignored-build gating.

Poem

🐰 A rabbit once hopped through the scripts,
And found some builds blocked at the gate.
With strictDepBuilds set to true,
ERR_PNPM_IGNORED_BUILDS rang out on cue—
Now the warren knows which builds must wait! 🌿

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

• X
• Mastodon
• Reddit
• LinkedIn

• Run lifecycle-script build phase for fresh-lockfile installs and pacquet add, matching pnpm.
• Share build-phase tail across install modes, preserving ERR_PNPM_* error codes.
• Enable lifecycle-script integration tests and add regression for ignored build warnings.

The following are alternative approaches to this PR:

Recommendation: The chosen approach (extracting a shared run_build_phase with a transparent BuildPhaseError) is the best trade-off: it removes duplication, guarantees pnpm-parity behaviors (running approved lifecycle scripts and reporting ignored builds) across both install modes, and keeps ERR_PNPM_* codes stable. Further unification via a larger shared orchestrator would be over-scoped for this bug fix.

install_with_fresh_lockfile.rs Run shared build phase on fresh-lockfile path (and 'add') +136/-45

Run shared build phase on fresh-lockfile path (and 'add')

• Wires the fresh-lockfile install path to capture 'side_effects_maps_by_snapshot'/'requires_build_by_snapshot', surface hoisted roots and public-hoist aliases, and invoke the shared 'run_build_phase'. Reorders work so 'importing_done', store-index writer draining, and lockfile save happen after build success, and defers the node engine probe to overlap virtual-store I/O when possible.

pacquet/crates/package-manager/src/install_with_fresh_lockfile.rs

---

install_frozen_lockfile.rs Extract shared build-phase tail into 'run_build_phase' +262/-196

Extract shared build-phase tail into 'run_build_phase'

• Introduces 'BuildPhaseInputs', 'BuildPhaseError', and 'run_build_phase' to encapsulate patched-dependency resolution, 'BuildModules', ignored-scripts emission, and post-build top-level bin linking. Refactors the frozen-lockfile install path to call the shared helper while preserving transparent diagnostics and existing error-code behavior.

pacquet/crates/package-manager/src/install_frozen_lockfile.rs

---

lifecycle_scripts.rs Enable dependency build-script tests and add 'pacquet add' regression +111/-87

Enable dependency build-script tests and add 'pacquet add' regression

• Renames the previous 'known_failures' module to actively run dependency lifecycle-script tests, moving 'allowBuilds' setup to 'pnpm-workspace.yaml' via a helper. Fixes timestamp parsing in dependency-order assertions and adds a regression test ensuring 'pacquet add' reports ignored build scripts and does not execute blocked scripts.

pacquet/crates/cli/tests/lifecycle_scripts.rs

---

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.