fix(install): persist revoked builds to .modules.yaml (#12221) (#12224)

aqeelat · zkochan · web-flow · commit 564619f04d29 · 2026-06-16T10:51:29.000+02:00
* fix(install): persist revoked builds to .modules.yaml (#12221) After a build-script dependency whose approval was revoked (e.g. via git stash dropping allowBuilds from pnpm-workspace.yaml) is re-added, the revocation detection populated ignoredBuilds in memory but the install path's writeModulesManifest had already run, so .modules.yaml never recorded the revoked packages. pnpm approve-builds then read an empty ignoredBuilds and reported 'no packages awaiting approval'. Re-read the manifest from disk after the revocation scan and write back the updated ignoredBuilds, merging with any entries the install path captured. * refactor(install): address review comments - Inline dead ignoredBuildsFromInstall indirection - Drop unsafe allowBuilds cast in the regression test --------- Co-authored-by: Zoltan Kochan <z@kochan.io>
diff --git a/.changeset/fix-approve-builds-stash-revocation.md b/.changeset/fix-approve-builds-stash-revocation.md
@@ -0,0 +1,5 @@
+---
+"pnpm": patch
+---
+
+Fixed `pnpm approve-builds` reporting "no packages awaiting approval" when a build-script dependency whose approval was revoked (e.g. after `git stash` drops the `allowBuilds` from `pnpm-workspace.yaml`) is re-added. The revoked packages are now correctly recorded in `.modules.yaml` so `approve-builds` can find them. [#12221](https://github.com/pnpm/pnpm/issues/12221)
diff --git a/installing/deps-installer/src/install/index.ts b/installing/deps-installer/src/install/index.ts
@@ -38,7 +38,7 @@ import {
   type WantedDependency,
 } from '@pnpm/installing.deps-resolver'
 import { extendProjectsWithTargetDirs, headlessInstall, type InstallationResultStats } from '@pnpm/installing.deps-restorer'
-import { writeModulesManifest } from '@pnpm/installing.modules-yaml'
+import { readModulesManifest, type StrictModules, writeModulesManifest } from '@pnpm/installing.modules-yaml'
 import {
   type CatalogSnapshots,
   cleanGitBranchLockfiles,
@@ -497,6 +497,7 @@ export async function mutateModules (
   if (!opts.ignoreScripts && ignoredBuilds?.size) {
     ignoredBuilds = await runUnignoredDependencyBuilds(opts, ignoredBuilds, ctx.wantedLockfile, allowBuild)
   }
+  let revokedBuilds = false
   // 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
@@ -519,10 +520,28 @@ export async function mutateModules (
         if (allowBuild?.(depPath) === undefined) {
           ignoredBuilds ??= new Set()
           ignoredBuilds.add(depPath)
+          revokedBuilds = true
         }
       }
     }
   }
+  if (revokedBuilds && !opts.lockfileOnly && opts.enableModulesDir) {
+    // The install path already wrote .modules.yaml with the current
+    // install's state, but it captured ignoredBuilds before the revocation
+    // scan above added to it. Re-read the manifest from disk so we only
+    // update ignoredBuilds and don't clobber fields (hoistedDependencies,
+    // pendingBuilds, etc.) the install just wrote. The current computed
+    // set is authoritative — runUnignoredDependencyBuilds may have removed
+    // entries (for packages it successfully rebuilt) that the on-disk
+    // manifest still records, and those must not be re-introduced.
+    const writtenManifest = await readModulesManifest(ctx.rootModulesDir)
+    if (writtenManifest) {
+      // writeModulesManifest converts ignoredBuilds to an array before
+      // serializing, so a Set is fine here.
+      writtenManifest.ignoredBuilds = ignoredBuilds
+      await writeModulesManifest(ctx.rootModulesDir, writtenManifest as StrictModules)
+    }
+  }
   ignoredScriptsLogger.debug({
     packageNames: ignoredBuilds ? dedupePackageNamesFromIgnoredBuilds(ignoredBuilds) : [],
   })
diff --git a/pnpm/test/install/lifecycleScripts.ts b/pnpm/test/install/lifecycleScripts.ts
@@ -2,6 +2,7 @@ import fs from 'node:fs'
 import path from 'node:path'
 
 import { expect, test } from '@jest/globals'
+import { parse } from '@pnpm/deps.path'
 import { prepare } from '@pnpm/prepare'
 import type { PackageManifest, ProjectManifest } from '@pnpm/types'
 import { readWorkspaceManifest } from '@pnpm/workspace.workspace-manifest-reader'
@@ -407,3 +408,38 @@ test('--allow-build flag should error when conflicting with allowBuilds: false',
   expect(result.status).toBe(1)
   expect(result.stdout.toString()).toContain('The following dependencies are ignored by the root project, but are allowed to be built by the current command: @pnpm.e2e/install-script-example')
 })
+
+test('approve-builds works after stashing and re-adding a dependency (#12221)', async () => {
+  const project = prepare({})
+
+  const pkgName = '@pnpm.e2e/pre-and-postinstall-scripts-example'
+
+  const firstAdd = execPnpmSync(['add', `${pkgName}@1.0.0`])
+  expect(firstAdd.status).toBe(1)
+  expect(firstAdd.stdout.toString()).toContain('Ignored build scripts:')
+
+  const firstApprove = execPnpmSync(['approve-builds', '--all'])
+  expect(firstApprove.status).toBe(0)
+
+  const wsManifest = await readWorkspaceManifest(process.cwd())
+  expect(wsManifest!.allowBuilds?.[pkgName]).toBe(true)
+
+  fs.rmSync('package.json', { force: true })
+  fs.rmSync('pnpm-workspace.yaml', { force: true })
+  fs.rmSync('pnpm-lock.yaml', { force: true })
+
+  fs.writeFileSync('package.json', '{}')
+  writeYamlFileSync('pnpm-workspace.yaml', {})
+
+  const secondAdd = execPnpmSync(['add', `${pkgName}@1.0.0`])
+  expect(secondAdd.status).toBe(1)
+  expect(secondAdd.stdout.toString()).toContain('Ignored build scripts:')
+
+  const modulesManifest = project.readModulesManifest()
+  expect(modulesManifest?.ignoredBuilds).toBeDefined()
+  expect(Array.from(modulesManifest!.ignoredBuilds!).some((dp) => parse(dp).name === pkgName)).toBe(true)
+
+  const secondApprove = execPnpmSync(['approve-builds', '--all'])
+  expect(secondApprove.status).toBe(0)
+  expect(secondApprove.stdout.toString()).not.toContain('No packages awaiting approval')
+})

-Original file line number
+Diff line change
@@ @@ -0,0 +1,5 @@ @@
 +---
 +"pnpm": patch
 +---
++
 +Fixed `pnpm approve-builds` reporting "no packages awaiting approval" when a build-script dependency whose approval was revoked (e.g. after `git stash` drops the `allowBuilds` from `pnpm-workspace.yaml`) is re-added. The revoked packages are now correctly recorded in `.modules.yaml` so `approve-builds` can find them. [#12221](https://github.com/pnpm/pnpm/issues/12221)