fix: global installs respect build policy from global config.yaml when GVS is enabled (#11363)

* fix(config.reader): move GVS allowBuilds default after globalDepsBuildConfig re-application

The GVS default allowBuilds = {} was applied too early — before
workspace manifest settings were read and before .npmrc values
(dangerouslyAllowAllBuilds) were re-applied via globalDepsBuildConfig.

This caused hasDependencyBuildOptions() to return true (because {}} is
not null), blocking restoration of .npmrc values. Global installs
with GVS enabled would silently skip all build scripts even when
the config explicitly allowed them.

This fix moves the GVS default to after both workspace manifest
reading and globalDepsBuildConfig re-application, so that:
1. Workspace manifest allowBuilds takes precedence (if present)
2. .npmrc dangerously-allow-all-builds is properly restored
3. Empty {}} is only applied as a last resort

Closes #9249

* fix(config.reader): apply Copilot suggestion for GVS allowBuilds guard

From PR review discussion_r3141002317:

- Replace hasDependencyBuildOptions() == null with hasDependencyBuildOptions()
  so the GVS default only applies when no build policy at all is
  configured (not even dangerouslyAllowAllBuilds). This is cleaner because
  the condition now matches the re-application guard on the line
  immediately before it.

- Add regression test verifying that dangerouslyAllowAllBuilds with GVS
  preserves allowBuilds when no global workspace manifest exists.

* docs: update changeset

* fix(config.reader): address PR review feedback

- Fix unreachable GVS allowBuilds default: hasDependencyBuildOptions()
  always returns true after globalDepsBuildConfig re-applies defaults
  (dangerouslyAllowAllBuilds: false is != null). Replace with explicit
  allowBuilds == null && dangerouslyAllowAllBuilds !== true check.
- Rename .npmrc references to global config.yaml in changeset, comments,
  and test names (zkochan: v11 reads from global rc file, not .npmrc).
- Add try/finally env cleanup for XDG_CONFIG_HOME and PNPM_HOME in tests.
- Add test for workspace manifest allowBuilds precedence over config.yaml.

* fix(config.reader): fix GVS workspace manifest test

- Use import.meta.dirname/global/v11 for globalPkgDir (matches env.PNPM_HOME)
- Fix assertion: dangerouslyAllowAllBuilds coexists with allowBuilds
- Clean up global/v11 directory in finally block to prevent test leakage

* fix(config.reader): use object form for workspace manifest allowBuilds; clean up parent global/ dir

Fixes two PR #11363 review threads:
1. allowBuilds in workspace manifest must be Record<string, boolean>,
   not array — createAllowBuildFunction uses Object.entries()
2. Remove empty config/reader/test/global/ directory after test

* fix(config.reader): address production review nits

- Update changeset: use camelCase dangerouslyAllowAllBuilds (YAML key, not .npmrc)
- Add enableGlobalVirtualStore assertion to first GVS test
- Add comment explaining dangerouslyAllowAllBuilds coexistence on config object

* fix(config.reader): address Copilot review — env safety, GLOBAL_LAYOUT_VERSION, try/finally

- Move XDG_CONFIG_HOME mutation and file setup inside try blocks
  so env is always restored even if setup throws
- Replace hard-coded v11 with GLOBAL_LAYOUT_VERSION import
- Fix corrupted try/finally in workspace manifest precedence test
  (missing finally block and mangled expect line from prior bad edit)
- Reword comment: enableGlobalVirtualStore defaults to true for
  global installs, not \"when not in CI\"

* fix(config.reader): address last 3 Copilot review threads — comment wording, cleanup placement, test rename

* fix(config.reader): fix block-scoped globalDir leak in GVS test

* fix: address Copilot review #4194783789 — restore auth test, fix naming, remove artifacts

* Remove local dev tooling — not part of this PR

* Remove PR.md — issue context is in the PR description

---------

Co-authored-by: Zoltan Kochan <z@kochan.io>
Co-authored-by: Tom Hale <tom@hale.net>

.changeset/fix-global-allow-builds.md

@@ -0,0 +1,13 @@
+---
+"@pnpm/config.reader": patch
+"pnpm": patch
+---
+
+**fix**: global installs respect global config build policy (e.g., `dangerouslyAllowAllBuilds` from config.yaml) when GVS is enabled [#9249](https://github.com/pnpm/pnpm/issues/9249).
+
+The global virtual-store (GVS) default `allowBuilds = {}` was applied before workspace manifest settings were read and before global config values (stripped by `extractAndRemoveDependencyBuildOptions`) were re-applied via `globalDepsBuildConfig`. This caused `hasDependencyBuildOptions` to return `true` (because `{}` is not null), blocking restoration of global config values like `dangerouslyAllowAllBuilds`. As a result, global installs skipped all build scripts even when the config explicitly allowed them.
+
+This fix moves the GVS default to **after** workspace manifest reading and `globalDepsBuildConfig` re-application, so that:
+1. Workspace manifest `allowBuilds` takes precedence (if present)
+2. Global config `dangerouslyAllowAllBuilds` is properly restored (if set and no workspace policy exists)
+3. Empty `{}` is only applied as a last resort when no policy is configured anywhere

config/reader/src/index.ts

@@ -395,13 +395,6 @@ export async function getConfig (opts: {
   } else if (!pnpmConfig.bin) {
     pnpmConfig.bin = path.join(pnpmConfig.dir, 'node_modules', '.bin')
   }
-  // Default allowBuilds to {} when GVS is enabled so that GVS hashes
-  // are engine-agnostic when no build policy is configured. Without
-  // this, allowBuilds is undefined which makes createAllowBuildFunction
-  // return undefined, causing all hashes to include ENGINE_NAME.
-  if (pnpmConfig.enableGlobalVirtualStore && pnpmConfig.allowBuilds == null) {
-    pnpmConfig.allowBuilds = {}
-  }
   pnpmConfig.packageManager = packageManager
 
   pnpmConfig.rootProjectManifestDir = pnpmConfig.lockfileDir ?? pnpmConfig.workspaceDir ?? pnpmConfig.dir
@@ -531,6 +524,19 @@ export async function getConfig (opts: {
   if (!hasDependencyBuildOptions(pnpmConfig)) {
     Object.assign(pnpmConfig, globalDepsBuildConfig)
   }
+  // Default allowBuilds to {} when GVS is enabled and no build policy is
+  // configured. This makes GVS hashes engine-agnostic for pure-JS packages.
+  // When a build policy (dangerouslyAllowAllBuilds from global config.yaml,
+  // or allowBuilds from the workspace manifest) exists, GVS hashes must
+  // include ENGINE_NAME so that built packages and their dependents are
+  // correctly invalidated across Node upgrades and architecture changes.
+  if (
+    pnpmConfig.enableGlobalVirtualStore &&
+    pnpmConfig.allowBuilds == null &&
+    pnpmConfig.dangerouslyAllowAllBuilds !== true
+  ) {
+    pnpmConfig.allowBuilds = {}
+  }
   if (opts.cliOptions['save-peer']) {
     if (opts.cliOptions['save-prod']) {
       throw new PnpmError('CONFIG_CONFLICT_PEER_CANNOT_BE_PROD_DEP', 'A package cannot be a peer dependency and a prod dependency at the same time')

config/reader/test/index.ts

@@ -4,6 +4,7 @@ import os from 'node:os'
 import path from 'node:path'
 
 import { afterEach, beforeEach, describe, expect, jest, test } from '@jest/globals'
+import { GLOBAL_LAYOUT_VERSION } from '@pnpm/constants'
 import { prepare, prepareEmpty } from '@pnpm/prepare'
 import { fixtures } from '@pnpm/test-fixtures'
 import PATH from 'path-name'
@@ -2223,3 +2224,100 @@ test('pnpm_config_git_branch_lockfile env var overrides git-branch-lockfile from
 
   expect(config.useGitBranchLockfile).toBe(true)
 })
+
+test('GVS: workspace manifest allowBuilds takes precedence over global config.yaml dangerouslyAllowAllBuilds', async () => {
+  prepareEmpty()
+
+  const prevXdgConfigHome = process.env.XDG_CONFIG_HOME
+
+  const globalDir = path.join(import.meta.dirname, 'global', GLOBAL_LAYOUT_VERSION)
+
+  try {
+    process.env.XDG_CONFIG_HOME = path.resolve('.config')
+
+    fs.mkdirSync(path.join(process.env.XDG_CONFIG_HOME, 'pnpm'), { recursive: true })
+    writeYamlFileSync(path.join(process.env.XDG_CONFIG_HOME, 'pnpm', 'config.yaml'), {
+      dangerouslyAllowAllBuilds: true,
+    })
+
+    fs.mkdirSync(globalDir, { recursive: true })
+    writeYamlFileSync(path.join(globalDir, 'pnpm-workspace.yaml'), {
+      allowBuilds: { '@some/pkg': true, esbuild: true },
+    })
+
+    const { config } = await getConfig({
+      cliOptions: {
+        global: true,
+      },
+      env,
+      packageManager: {
+        name: 'pnpm',
+        version: '1.0.0',
+      },
+    })
+
+    expect(config.enableGlobalVirtualStore).toBe(true)
+    expect(config.allowBuilds).toStrictEqual({ '@some/pkg': true, esbuild: true })
+    // The dangerouslyAllowAllBuilds value from the already-loaded global config.yaml
+    // is preserved when workspace manifest settings are applied after
+    // extractAndRemoveDependencyBuildOptions strips the workspace build options.
+    expect(config.dangerouslyAllowAllBuilds).toBe(true)
+  } finally {
+    if (prevXdgConfigHome === undefined) {
+      delete process.env.XDG_CONFIG_HOME
+    } else {
+      process.env.XDG_CONFIG_HOME = prevXdgConfigHome
+    }
+    fs.rmSync(globalDir, { recursive: true, force: true })
+    const parentGlobalDir = path.join(import.meta.dirname, 'global')
+    if (fs.existsSync(parentGlobalDir)) {
+      fs.rmSync(parentGlobalDir, { recursive: true, force: true })
+    }
+  }
+})
+
+test('GVS: global config.yaml dangerouslyAllowAllBuilds is preserved when no workspace manifest exists', async () => {
+  prepareEmpty()
+
+  const prevXdgConfigHome = process.env.XDG_CONFIG_HOME
+
+  try {
+    // Set up global config.yaml with a build policy
+    fs.mkdirSync('.config/pnpm', { recursive: true })
+    writeYamlFileSync('.config/pnpm/config.yaml', {
+      dangerouslyAllowAllBuilds: true,
+    })
+    process.env.XDG_CONFIG_HOME = path.resolve('.config')
+
+    // No global pnpm-workspace.yaml
+    // intentionally do not write a workspace manifest
+
+    const { config } = await getConfig({
+      cliOptions: {
+        global: true,
+      },
+      env,
+      packageManager: {
+        name: 'pnpm',
+        version: '1.0.0',
+      },
+    })
+
+    // For global installs, enableGlobalVirtualStore defaults to true.
+    expect(config.enableGlobalVirtualStore).toBe(true)
+    // The key assertion: global config.yaml policy should NOT be wiped by the GVS
+    // allowBuilds = {} default. Previously this block set allowBuilds
+    // before globalDepsBuildConfig was re-applied, so hasDependencyBuildOptions
+    // saw allowBuilds = {} and skipped re-application, silently losing
+    // dangerouslyAllowAllBuilds.
+    expect(config.dangerouslyAllowAllBuilds).toBe(true)
+    // allowBuilds should remain null — dangerouslyAllowAllBuilds IS the policy
+    expect(config.allowBuilds).toBeUndefined()
+  } finally {
+    if (prevXdgConfigHome === undefined) {
+      delete process.env.XDG_CONFIG_HOME
+    } else {
+      process.env.XDG_CONFIG_HOME = prevXdgConfigHome
+    }
+  }
+})