fix: check allowBuild for packages with cached side-effects (#11039)

Closes #11035

## Summary

### Root cause fix: don't apply cached side-effects for unapproved packages
When importing packages from the store, side-effects cache was applied for any package not explicitly denied (`allowBuild !== false`). This meant unapproved packages (`allowBuild === undefined`) got cached build artifacts, setting `isBuilt: true` and bypassing the `allowBuild` check in `buildModules`.

**Fix:** Only apply side-effects cache when `allowBuild` returns `true` (explicitly approved). Changed in three locations:
- `installing/deps-restorer/src/index.ts` (isolated linker)
- `installing/deps-restorer/src/linkHoistedModules.ts` (hoisted linker)
- `installing/deps-installer/src/install/link.ts` (non-headless install)

### Revocation detection
When a package's build approval is revoked between installs (was `true` in `.modules.yaml`, now undefined), detect it in `mutateModules` and add to `ignoredBuilds` so `strictDepBuilds` fails.

### Status messages in `_rebuild`
Users now see what happened to each package during rebuild:
- `pkg@version: built successfully`
- `pkg@version: skipped (no build scripts)`
- `pkg@version: skipped (not allowed)`
- `pkg@version: reused from store cache`

And during install:
- `pkg@version: reused from store (side effects cache)`

### `buildSelectedPkgs` fixes
- Preserve `storeDir`, `virtualStoreDir`, `virtualStoreDirMaxLength` from existing `.modules.yaml` instead of overwriting with config-derived values (which caused "reinstall from scratch" prompt)
- Write `allowBuilds` to `.modules.yaml` so GVS doesn't detect a mismatch on next install
- Merge `ignoredBuilds` with existing entries for packages not being rebuilt

.changeset/fix-cached-builds-bypass-allowbuilds.md

@@ -0,0 +1,7 @@
+---
+"@pnpm/building.during-install": patch
+"@pnpm/installing.deps-installer": patch
+"pnpm": patch
+---
+
+Fixed `strictDepBuilds` and `allowBuilds` checks being bypassed when a package's build side-effects are cached in the store. Packages with cached builds were skipped by `buildModules` (`isBuilt: true`) and never reached the `allowBuild` check. Now checks `allowBuild` for all packages with `requiresBuild` regardless of `isBuilt` state. Also detects packages whose build approval was revoked between installs.

AGENTS.md

@@ -49,6 +49,20 @@ pnpm install
 pnpm run compile
 ```
 
+To compile a specific package:
+
+```bash
+pnpm --filter <package_name> run compile
+```
+
+**Important:** The pnpm CLI e2e tests (in `pnpm/test/`) use the **bundled** `pnpm/dist/pnpm.mjs`, not the individual package `lib/` outputs. After changing any package, you must rebuild the bundle before running e2e tests:
+
+```bash
+pnpm --filter pnpm run compile
+```
+
+This runs `tsgo --build`, linting, and `pnpm run bundle` (which bundles all packages into `pnpm/dist/pnpm.mjs`). Without this step, e2e tests will use a stale bundle and your changes won't be tested.
+
 ## Testing
 
 Never run all tests in the repository as it takes a lot of time.

building/after-install/src/index.ts

@@ -28,7 +28,7 @@ import {
   type PackageSnapshots,
 } from '@pnpm/lockfile.utils'
 import { lockfileWalker, type LockfileWalkerStep } from '@pnpm/lockfile.walker'
-import { logger, streamParser } from '@pnpm/logger'
+import { globalInfo, logger, streamParser } from '@pnpm/logger'
 import npa from '@pnpm/npm-package-arg'
 import { safeReadPackageJsonFromDir } from '@pnpm/pkg-manifest.reader'
 import type { PackageFilesIndex } from '@pnpm/store.cafs'
@@ -153,9 +153,10 @@ export async function buildSelectedPkgs (
     publicHoistPattern: ctx.publicHoistPattern,
     registries: ctx.registries,
     skipped: Array.from(ctx.skipped),
-    storeDir: ctx.storeDir,
-    virtualStoreDir: ctx.virtualStoreDir,
-    virtualStoreDirMaxLength: ctx.virtualStoreDirMaxLength,
+    storeDir: ctx.modulesFile?.storeDir ?? ctx.storeDir,
+    virtualStoreDir: ctx.modulesFile?.virtualStoreDir ?? ctx.virtualStoreDir,
+    virtualStoreDirMaxLength: ctx.modulesFile?.virtualStoreDirMaxLength ?? ctx.virtualStoreDirMaxLength,
+    allowBuilds: opts.allowBuilds,
   })
   return {
     ignoredBuilds: ignoredPkgs,
@@ -366,6 +367,7 @@ async function _rebuild (
               includeDepGraphHash: true,
             })
             if (pkgFilesIndex.sideEffects?.has(sideEffectsCacheKey)) {
+              globalInfo(`${pkgId}: reused from store cache`)
               pkgsThatWereRebuilt.add(depPath)
               return
             }
@@ -379,7 +381,17 @@ async function _rebuild (
           requiresBuild = pkgRequiresBuild(pgkManifest, new Map())
         }
 
-        const hasSideEffects = requiresBuild && allowBuild(pkgInfo.name, pkgInfo.version, depPath) && await runPostinstallHooks({
+        if (!requiresBuild) {
+          globalInfo(`${pkgId}: skipped (no build scripts)`)
+          pkgsThatWereRebuilt.add(depPath)
+          return
+        }
+        if (!allowBuild(pkgInfo.name, pkgInfo.version, depPath)) {
+          globalInfo(`${pkgId}: skipped (not allowed)`)
+          pkgsThatWereRebuilt.add(depPath)
+          return
+        }
+        const hasSideEffects = await runPostinstallHooks({
           depPath,
           extraBinPaths,
           extraEnv: opts.extraEnv,
@@ -391,6 +403,7 @@ async function _rebuild (
           shellEmulator: opts.shellEmulator,
           unsafePerm: opts.unsafePerm || false,
         })
+        globalInfo(`${pkgId}: built successfully`)
         if (hasSideEffects && (opts.sideEffectsCacheWrite ?? true) && resolution.integrity) {
           builtDepPaths.add(depPath)
           const filesIndexFile = storeIndexKey(resolution.integrity!.toString(), pkgId)

building/during-install/src/index.ts

@@ -9,7 +9,7 @@ import { skippedOptionalDependencyLogger } from '@pnpm/core-loggers'
 import { calcDepState, type DepsStateCache } from '@pnpm/deps.graph-hasher'
 import { PnpmError } from '@pnpm/error'
 import { runPostinstallHooks } from '@pnpm/exec.lifecycle'
-import { logger } from '@pnpm/logger'
+import { globalInfo, logger } from '@pnpm/logger'
 import { applyPatchToDir } from '@pnpm/patching.apply-patch'
 import { safeReadPackageJsonFromDir } from '@pnpm/pkg-manifest.reader'
 import type { StoreController } from '@pnpm/store.controller-types'
@@ -74,6 +74,9 @@ export async function buildModules<T extends string> (
   const groups = chunks.map((chunk) => {
     chunk = chunk.filter((depPath) => {
       const node = depGraph[depPath]
+      if ((node.requiresBuild || node.patch != null) && node.isBuilt) {
+        globalInfo(`${node.name}@${node.version}: reused from store (side effects cache)`)
+      }
       return (node.requiresBuild || node.patch != null) && !node.isBuilt
     })
     if (opts.depsToBuild != null) {

installing/deps-installer/src/install/index.ts

@@ -371,6 +371,29 @@ export async function mutateModules (
   if (!opts.ignoreScripts && ignoredBuilds?.size) {
     ignoredBuilds = await runUnignoredDependencyBuilds(opts, ignoredBuilds, allowBuild)
   }
+  // Detect packages whose build approval was revoked between the previous
+  // and current install. A package is considered revoked when it was
+  // previously allowed (true) but is now undecided (undefined). Packages
+  // explicitly denied (false) are not added to ignoredBuilds, consistent
+  // with how buildModules treats them.
+  if (
+    ctx.modulesFile?.allowBuilds &&
+    ctx.wantedLockfile.packages &&
+    Object.values(ctx.modulesFile.allowBuilds).some((v) => v === true)
+  ) {
+    const oldAllowBuild = createAllowBuildFunction({ allowBuilds: ctx.modulesFile.allowBuilds })
+    if (oldAllowBuild) {
+      for (const depPath of Object.keys(ctx.wantedLockfile.packages) as DepPath[]) {
+        if (ignoredBuilds?.has(depPath)) continue
+        const { name, version } = dp.parse(depPath)
+        if (!name || !version) continue
+        if (oldAllowBuild(name, version) === true && allowBuild?.(name, version) === undefined) {
+          ignoredBuilds ??= new Set()
+          ignoredBuilds.add(depPath)
+        }
+      }
+    }
+  }
   ignoredScriptsLogger.debug({
     packageNames: ignoredBuilds ? dedupePackageNamesFromIgnoredBuilds(ignoredBuilds) : [],
   })

installing/deps-installer/src/install/link.ts

@@ -472,7 +472,7 @@ async function linkAllPkgs (
       depNode.requiresBuild = files.requiresBuild
       let sideEffectsCacheKey: string | undefined
       if (opts.sideEffectsCacheRead && files.sideEffectsMaps && !isEmpty(files.sideEffectsMaps)) {
-        if (opts?.allowBuild?.(depNode.name, depNode.version) !== false) {
+        if (opts.allowBuild?.(depNode.name, depNode.version) === true) {
           sideEffectsCacheKey = calcDepState(opts.depGraph, opts.depsStateCache, depNode.depPath, {
             includeDepGraphHash: !opts.ignoreScripts && depNode.requiresBuild, // true when is built
             patchFileHash: depNode.patch?.hash,

installing/deps-restorer/src/index.ts

@@ -908,7 +908,7 @@ async function linkAllPkgs (
       depNode.requiresBuild = filesResponse.requiresBuild
       let sideEffectsCacheKey: string | undefined
       if (opts.sideEffectsCacheRead && filesResponse.sideEffectsMaps && !isEmpty(filesResponse.sideEffectsMaps)) {
-        if (opts?.allowBuild?.(depNode.name, depNode.version) !== false) {
+        if (opts.allowBuild?.(depNode.name, depNode.version) === true) {
           sideEffectsCacheKey = calcDepState(opts.depGraph, opts.depsStateCache, depNode.dir, {
             includeDepGraphHash: !opts.ignoreScripts && depNode.requiresBuild, // true when is built
             patchFileHash: depNode.patch?.hash,

installing/deps-restorer/src/linkHoistedModules.ts

@@ -116,7 +116,7 @@ async function linkAllPkgsInOrder (
         depNode.requiresBuild = filesResponse.requiresBuild
         let sideEffectsCacheKey: string | undefined
         if (opts.sideEffectsCacheRead && filesResponse.sideEffectsMaps && !isEmpty(filesResponse.sideEffectsMaps)) {
-          if (opts?.allowBuild?.(depNode.name, depNode.version) !== false) {
+          if (opts.allowBuild?.(depNode.name, depNode.version) === true) {
             sideEffectsCacheKey = _calcDepState(dir, {
               includeDepGraphHash: !opts.ignoreScripts && depNode.requiresBuild, // true when is built
               patchFileHash: depNode.patch?.hash,

pnpm/test/install/lifecycleScripts.ts

@@ -299,6 +299,39 @@ test('selective rebuild preserves ignoredBuilds for packages not being rebuilt',
   expect(afterRebuild!.ignoredBuilds).toBeDefined()
 })
 
+test('strictDepBuilds fails for packages with cached side-effects (#11035)', async () => {
+  prepare({
+    dependencies: {
+      '@pnpm.e2e/pre-and-postinstall-scripts-example': '1.0.0',
+    },
+  })
+  const storeDir = path.resolve('isolated-store')
+
+  // First install: allow the build so side-effects get cached in the store
+  writeYamlFileSync('pnpm-workspace.yaml', {
+    storeDir,
+    enableGlobalVirtualStore: false,
+    allowBuilds: {
+      '@pnpm.e2e/pre-and-postinstall-scripts-example': true,
+    },
+  })
+  const firstResult = execPnpmSync(['install'])
+  expect(firstResult.status).toBe(0)
+  expect(fs.existsSync('node_modules/@pnpm.e2e/pre-and-postinstall-scripts-example/generated-by-postinstall.js')).toBeTruthy()
+
+  // Second install: remove the approval. Side-effects are cached in the store
+  // but strictDepBuilds should still fail.
+  writeYamlFileSync('pnpm-workspace.yaml', {
+    storeDir,
+    enableGlobalVirtualStore: false,
+    strictDepBuilds: true,
+    optimisticRepeatInstall: false,
+  })
+  const secondResult = execPnpmSync(['install'])
+  expect(secondResult.status).toBe(1)
+  expect(secondResult.stdout.toString()).toContain('Ignored build scripts:')
+})
+
 test('git dependencies with preparation scripts should be installed when dangerouslyAllowAllBuilds is true', async () => {
   prepare({})
   writeYamlFileSync('pnpm-workspace.yaml', { dangerouslyAllowAllBuilds: true })