fix(global): avoid doubled modulesDir in approve-builds during global add (#11404)

* fix(global): avoid doubled modulesDir when approving builds in global add

The global add → approve-builds flow used to forward an absolute
`modulesDir` (`<installDir>/node_modules`) into the install run by
`approve-builds`. The install layer treats `modulesDir` as a path
relative to `lockfileDir` and joins it again — producing a doubled
path on Windows because `path.join` does not collapse an embedded
absolute path. The hoist step then failed with `ENOENT` while trying
to symlink under `<installDir>\<installDir>\node_modules\.pnpm\...`.

Closes #11403.

* test: type test fixtures correctly

* fix(install): tolerate absolute modulesDir in headless install context

Replace the prior unit test (which only checked the call shape) with an
integration test that exercises `install()` with an absolute `modulesDir`
through both the regular and frozen-lockfile paths — the failure mode the
global add → approve-builds chain originally hit on Windows.

`headlessInstall` and `readProjectsContext` now resolve `modulesDir` via
`pathAbsolute` instead of `path.join(lockfileDir, modulesDir)`, so an
absolute value no longer produces a doubled prefix. The
`promptApproveGlobalBuilds` change from the previous commit is retained
as the contract-level fix.

* test: add e2e test driving the pnpm CLI with --modules-dir=<abs>

Replace the programmatic install() regression test with an e2e test in
pnpm/test/install/absoluteModulesDir.ts that runs the bundled pnpm
binary with `pnpm install --modules-dir=<abs>` (regular and frozen).
This is the closest CLI-level reproduction of the doubled-prefix path
bug from #11403 — the bug fired specifically in the headless install
path that --frozen-lockfile triggers.

* test(global): drive add -g + approve-builds chain end-to-end

Add an e2e test that runs the bundled pnpm CLI through the full
`pnpm add -g <pkg-with-build>` → approve-builds → install chain that
produced the doubled-prefix `ENOENT` in #11403.

The chain only fires when `process.stdin.isTTY` is true, which CI
subprocesses don't satisfy. Add a test-only env var
`PNPM_AUTO_APPROVE_BUILDS_FOR_TESTS` that bypasses the TTY guard in
`promptApproveGlobalBuilds` and forwards `all: true` so `approve-builds`
skips its multiselect and confirm prompts. The post-approval install
then runs the same code path a real user hit, and the test asserts the
build artifacts ended up in the global install dir.

Replaces the narrower `--modules-dir=<abs>` regression test, which
only exercised the install layer and not the global-add flow that
originally surfaced the bug.

* test: enable global add -g + approve-builds e2e test on Windows

- Switch to @pnpm.e2e/install-script-example which is cross-platform.
- Use pathAbsolute for modulesDir to prevent doubled path bugs on Windows.
- Add path-absolute dependency to affected packages.

.changeset/fix-global-add-approve-builds-doubled-modules-dir.md

@@ -0,0 +1,10 @@
+---
+"@pnpm/installing.deps-restorer": patch
+"@pnpm/installing.read-projects-context": patch
+"@pnpm/global.commands": patch
+"pnpm": patch
+---
+
+Fix `ENOENT` symlink failure when `pnpm add -g` triggers the approve-builds prompt. The global add flow used to forward an absolute `modulesDir` (`<installDir>/node_modules`) into the install run by `approve-builds`. The install layer treated `modulesDir` as a path relative to `lockfileDir` and joined it again, producing a doubled path on Windows because `path.join` does not collapse an embedded absolute path. The hoist step then tried to `mkdir` and symlink under `<installDir>\<installDir>\node_modules\.pnpm\node_modules\...` and failed with `ENOENT` [#11403](https://github.com/pnpm/pnpm/issues/11403).
+
+`promptApproveGlobalBuilds` now forwards `modulesDir: undefined` so downstream code derives it from `lockfileDir`. The install layer (`headlessInstall` and `readProjectsContext`) also resolves `modulesDir` via `pathAbsolute` so that absolute values from any caller no longer produce a doubled prefix.

deps/inspection/tree-builder/package.json

@@ -50,6 +50,7 @@
     "@pnpm/util.lex-comparator": "catalog:",
     "load-json-file": "catalog:",
     "normalize-path": "catalog:",
+    "path-absolute": "catalog:",
     "realpath-missing": "catalog:",
     "resolve-link-target": "catalog:",
     "semver": "catalog:"

deps/inspection/tree-builder/src/buildDependenciesTree.ts

@@ -16,6 +16,7 @@ import { safeReadPackageJsonFromDir } from '@pnpm/pkg-manifest.reader'
 import { StoreIndex } from '@pnpm/store.index'
 import { DEPENDENCIES_FIELDS, type DependenciesField, type Finder, type Registries } from '@pnpm/types'
 import normalizePath from 'normalize-path'
+import { pathAbsolute } from 'path-absolute'
 import { realpathMissing } from 'realpath-missing'
 import { resolveLinkTarget } from 'resolve-link-target'
 
@@ -50,7 +51,7 @@ export async function buildDependenciesTree (
   if (!maybeOpts?.lockfileDir) {
     throw new TypeError('opts.lockfileDir is required')
   }
-  const modulesDir = await realpathMissing(path.join(maybeOpts.lockfileDir, maybeOpts.modulesDir ?? 'node_modules'))
+  const modulesDir = await realpathMissing(pathAbsolute(maybeOpts.modulesDir ?? 'node_modules', maybeOpts.lockfileDir))
   const modules = await readModulesManifest(modulesDir)
   const registries = normalizeRegistries({
     ...maybeOpts?.registries,

global/commands/src/promptApproveGlobalBuilds.ts

@@ -1,5 +1,3 @@
-import path from 'node:path'
-
 import type { CommandHandlerMap } from '@pnpm/cli.command'
 import type { IgnoredBuilds } from '@pnpm/types'
 
@@ -12,6 +10,18 @@ export interface PromptApproveGlobalBuildsOptions {
   inheritedOpts: object
 }
 
+/**
+ * Test-only env var. When set, `promptApproveGlobalBuilds` bypasses the
+ * TTY check and forwards `all: true` so the `approve-builds` command
+ * skips both its multiselect and confirm prompts. The flow then runs
+ * the same install machinery as a real interactive approval, which is
+ * what e2e tests need to reproduce regressions in this code path.
+ *
+ * Not for production use — pnpm has no UI to opt into "approve every
+ * pending build" silently, by design.
+ */
+const AUTO_APPROVE_FOR_TESTS_ENV = 'PNPM_AUTO_APPROVE_BUILDS_FOR_TESTS'
+
 /**
  * If the previous global install left builds awaiting approval, run the
  * interactive `approve-builds` flow against the install directory.
@@ -24,19 +34,28 @@ export interface PromptApproveGlobalBuildsOptions {
  * only on the install directory — otherwise it would treat the global
  * packages dir as a workspace and discover sibling install directories as
  * workspace projects.
+ *
+ * `modulesDir` is left undefined so that downstream consumers compute it
+ * relative to `lockfileDir`. Passing an absolute value here would be
+ * forwarded as-is to `install.handler`, which treats `modulesDir` as a
+ * path relative to `lockfileDir` and joins it again — producing a
+ * doubled path on Windows (path.join does not collapse an embedded
+ * absolute path).
  */
 export async function promptApproveGlobalBuilds (
   opts: PromptApproveGlobalBuildsOptions,
   commands: CommandHandlerMap
 ): Promise<void> {
-  if (!opts.ignoredBuilds?.size || !process.stdin.isTTY) return
+  if (!opts.ignoredBuilds?.size) return
+  const autoApproveForTests = process.env[AUTO_APPROVE_FOR_TESTS_ENV] === '1'
+  if (!autoApproveForTests && !process.stdin.isTTY) return
   await commands['approve-builds']({
     ...opts.inheritedOpts,
     workspaceDir: undefined,
     allProjects: undefined,
     selectedProjectsGraph: undefined,
     workspacePackagePatterns: undefined,
-    modulesDir: path.join(opts.installDir, 'node_modules'),
+    modulesDir: undefined,
     dir: opts.installDir,
     lockfileDir: opts.installDir,
     rootProjectManifest: undefined,
@@ -45,5 +64,8 @@ export async function promptApproveGlobalBuilds (
     global: false,
     pending: false,
     allowBuilds: opts.allowBuilds,
+    // When set, makes `approve-builds` skip both its multiselect and
+    // confirm prompts and approve every pending build.
+    all: autoApproveForTests ? true : undefined,
   }, [], commands)
 }

installing/deps-installer/package.json

@@ -116,6 +116,7 @@
     "normalize-path": "catalog:",
     "p-filter": "catalog:",
     "p-limit": "catalog:",
+    "path-absolute": "catalog:",
     "path-exists": "catalog:",
     "ramda": "catalog:",
     "run-groups": "catalog:",

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

@@ -14,6 +14,7 @@ import {
 } from '@pnpm/types'
 import { rimraf } from '@zkochan/rimraf'
 import enquirer from 'enquirer'
+import { pathAbsolute } from 'path-absolute'
 import { equals } from 'ramda'
 
 import { checkCompatibility } from './checkCompatibility/index.js'
@@ -115,7 +116,7 @@ export async function validateModules (
   }
   if (importersToPurge.length > 0 && (rootProject == null)) {
     importersToPurge.push({
-      modulesDir: path.join(opts.lockfileDir, opts.modulesDir),
+      modulesDir: pathAbsolute(opts.modulesDir, opts.lockfileDir),
       rootDir: opts.lockfileDir as ProjectRootDir,
     })
   }

installing/deps-restorer/src/index.ts

@@ -216,11 +216,15 @@ export async function headlessInstall (opts: HeadlessOptions): Promise<Installat
   }
 
   const depsStateCache: DepsStateCache = {}
-  const relativeModulesDir = opts.modulesDir ?? 'node_modules'
-  const rootModulesDir = await realpathMissing(path.join(lockfileDir, relativeModulesDir))
+  // `modulesDir` is conventionally a path relative to `lockfileDir`, but
+  // some callers pass it as an absolute path. Resolve via `pathAbsolute`
+  // so both forms work — `path.join` on Windows would otherwise produce a
+  // doubled prefix when the second argument is also absolute.
+  const modulesDir = opts.modulesDir ?? 'node_modules'
+  const rootModulesDir = await realpathMissing(pathAbsolute(modulesDir, lockfileDir))
   const internalPnpmDir = path.join(rootModulesDir, '.pnpm')
   const currentLockfile = opts.currentLockfile ?? await readCurrentLockfile(internalPnpmDir, { ignoreIncompatible: false })
-  const virtualStoreDir = pathAbsolute(opts.virtualStoreDir ?? path.join(relativeModulesDir, '.pnpm'), lockfileDir)
+  const virtualStoreDir = pathAbsolute(opts.virtualStoreDir ?? path.join(modulesDir, '.pnpm'), lockfileDir)
   const hoistedModulesDir = path.join(
     opts.enableGlobalVirtualStore ? internalPnpmDir : virtualStoreDir,
     'node_modules'

installing/deps-restorer/src/lockfileToHoistedDepGraph.ts

@@ -28,6 +28,7 @@ import type {
   StoreController,
 } from '@pnpm/store.controller-types'
 import type { AllowBuild, DepPath, ProjectId, Registries, SupportedArchitectures } from '@pnpm/types'
+import { pathAbsolute } from 'path-absolute'
 import { pathExists } from 'path-exists'
 
 export interface LockfileToHoistedDepGraphOptions {
@@ -92,7 +93,7 @@ async function _lockfileToHoistedDepGraph (
     autoInstallPeers: opts.autoInstallPeers,
   })
   const graph: DependenciesGraph = {}
-  const modulesDir = path.join(opts.lockfileDir, opts.modulesDir ?? 'node_modules')
+  const modulesDir = pathAbsolute(opts.modulesDir ?? 'node_modules', opts.lockfileDir)
   const fetchDepsOpts = {
     ...opts,
     lockfile,

installing/read-projects-context/package.json

@@ -35,6 +35,7 @@
     "@pnpm/installing.modules-yaml": "workspace:*",
     "@pnpm/lockfile.fs": "workspace:*",
     "@pnpm/types": "workspace:*",
+    "path-absolute": "catalog:",
     "realpath-missing": "catalog:"
   },
   "peerDependencies": {

installing/read-projects-context/src/index.ts

@@ -14,6 +14,7 @@ import type {
   ProjectRootDirRealPath,
   Registries,
 } from '@pnpm/types'
+import { pathAbsolute } from 'path-absolute'
 import { realpathMissing } from 'realpath-missing'
 
 export interface ProjectOptions {
@@ -45,8 +46,12 @@ export async function readProjectsContext<T> (
   skipped: Set<DepPath>
   virtualStoreDirMaxLength?: number
 }> {
-  const relativeModulesDir = opts.modulesDir ?? 'node_modules'
-  const rootModulesDir = await realpathMissing(path.join(opts.lockfileDir, relativeModulesDir))
+  // `modulesDir` is conventionally a path relative to `lockfileDir`, but
+  // some callers pass it as an absolute path. Resolve via `pathAbsolute`
+  // so both forms work — `path.join` on Windows would otherwise produce a
+  // doubled prefix when the second argument is also absolute.
+  const modulesDirOpt = opts.modulesDir ?? 'node_modules'
+  const rootModulesDir = await realpathMissing(pathAbsolute(modulesDirOpt, opts.lockfileDir))
   const modules = await readModulesManifest(rootModulesDir)
   return {
     currentHoistPattern: modules?.hoistPattern,
@@ -58,12 +63,12 @@ export async function readProjectsContext<T> (
     pendingBuilds: modules?.pendingBuilds ?? [],
     projects: await Promise.all(
       projects.map(async (project) => {
-        const modulesDir = await realpathMissing(path.join(project.rootDir, project.modulesDir ?? relativeModulesDir))
+        const modulesDir = await realpathMissing(pathAbsolute(project.modulesDir ?? modulesDirOpt, project.rootDir))
         const importerId = getLockfileImporterId(opts.lockfileDir, project.rootDir)
 
         return {
           ...project,
-          binsDir: project.binsDir ?? path.join(project.rootDir, relativeModulesDir, '.bin'),
+          binsDir: project.binsDir ?? path.join(modulesDir, '.bin'),
           id: importerId,
           modulesDir,
           rootDirRealPath: project.rootDirRealPath ?? await realpath(project.rootDir),

pnpm-lock.yaml

@@ -3670,6 +3670,9 @@ importers:
       normalize-path:
         specifier: 'catalog:'
         version: 3.0.0
+      path-absolute:
+        specifier: 'catalog:'
+        version: 2.0.0
       realpath-missing:
         specifier: 'catalog:'
         version: 2.0.0
@@ -5642,6 +5645,9 @@ importers:
       p-limit:
         specifier: 'catalog:'
         version: 7.3.0
+      path-absolute:
+        specifier: 'catalog:'
+        version: 2.0.0
       path-exists:
         specifier: 'catalog:'
         version: 5.0.0
@@ -6489,6 +6495,9 @@ importers:
       '@pnpm/types':
         specifier: workspace:*
         version: link:../../core/types
+      path-absolute:
+        specifier: 'catalog:'
+        version: 2.0.0
       realpath-missing:
         specifier: 'catalog:'
         version: 2.0.0
@@ -7747,6 +7756,9 @@ importers:
       p-defer:
         specifier: 'catalog:'
         version: 4.0.1
+      path-absolute:
+        specifier: 'catalog:'
+        version: 2.0.0
       path-exists:
         specifier: 'catalog:'
         version: 5.0.0

pnpm/package.json

@@ -165,6 +165,7 @@
     "normalize-newline": "catalog:",
     "p-defer": "catalog:",
     "path-exists": "catalog:",
+    "path-absolute": "catalog:",
     "path-name": "catalog:",
     "pidtree": "catalog:",
     "ps-list": "catalog:",

pnpm/src/cmd/clean.ts

@@ -5,6 +5,7 @@ import { docsUrl } from '@pnpm/cli.utils'
 import { findWorkspaceProjectsNoCheck } from '@pnpm/workspace.projects-reader'
 import { rimraf } from '@zkochan/rimraf'
 import { isSubdir } from 'is-subdir'
+import { pathAbsolute } from 'path-absolute'
 import { pathExists } from 'path-exists'
 import { renderHelp } from 'render-help'
 
@@ -72,7 +73,7 @@ export async function handler (
     const resolvedVirtualStoreDir = path.isAbsolute(opts.virtualStoreDir)
       ? opts.virtualStoreDir
       : path.resolve(rootDir, opts.virtualStoreDir)
-    const rootModulesDir = path.join(rootDir, modulesDir)
+    const rootModulesDir = pathAbsolute(modulesDir, rootDir)
     if (
       !isSubdir(rootModulesDir, resolvedVirtualStoreDir) &&
       isSubdir(rootDir, resolvedVirtualStoreDir) &&
@@ -89,7 +90,7 @@ function printRemoving (p: string): void {
 }
 
 async function cleanProjectDir (opts: { modulesDir: string, removeLockfile?: boolean }, dir: string): Promise<void> {
-  const fullModulesDir = path.join(dir, opts.modulesDir)
+  const fullModulesDir = pathAbsolute(opts.modulesDir, dir)
   if (await hasContentsToRemove(fullModulesDir)) {
     printRemoving(fullModulesDir)
     await removeModulesDirContents(fullModulesDir)

pnpm/test/install/global.ts

@@ -155,6 +155,49 @@ test('run lifecycle events of global packages in correct working directory', asy
   expect(fs.existsSync(path.join(pkgPath!, 'created-by-postinstall'))).toBeTruthy()
 })
 
+// Regression test for https://github.com/pnpm/pnpm/issues/11403.
+//
+// When `pnpm add -g` installs a package whose build is not pre-allowed,
+// the install succeeds with the build ignored and `globalAdd` then runs
+// `promptApproveGlobalBuilds`. If the user approves, `approve-builds`
+// runs `install.handler` against the install directory — and that is the
+// install run that crashed with `ENOENT` because `modulesDir` was being
+// forwarded as an absolute path and re-joined with `lockfileDir`.
+//
+// `PNPM_AUTO_APPROVE_BUILDS_FOR_TESTS=1` lets the test drive this flow
+// non-interactively: `promptApproveGlobalBuilds` skips the TTY check and
+// passes `all: true` so `approve-builds` approves every pending build
+// without prompting. The post-approval install must complete and the
+// build artifact must end up in the global install dir.
+test('approve-builds during global add does not produce a doubled modules path', async () => {
+  prepare()
+  const global = path.resolve('..', 'global')
+  const pnpmHome = path.join(global, 'pnpm')
+  fs.mkdirSync(pnpmHome, { recursive: true })
+
+  const env = {
+    [PATH_NAME]: `${path.join(pnpmHome, 'bin')}${path.delimiter}${process.env[PATH_NAME]!}`,
+    PNPM_HOME: pnpmHome,
+    XDG_DATA_HOME: global,
+    PNPM_AUTO_APPROVE_BUILDS_FOR_TESTS: '1',
+  }
+
+  // Note: no --allow-build here. The first install records the build in
+  // `ignoredBuilds`; `promptApproveGlobalBuilds` then drives the
+  // approve → install chain that produced the doubled-path crash.
+  await execPnpm([
+    'add',
+    '-g',
+    '@pnpm.e2e/install-script-example@1.0.0',
+  ], { env })
+
+  const pkgPath = findGlobalPkg(globalPkgDir(pnpmHome), '@pnpm.e2e/install-script-example')
+  expect(pkgPath).toBeTruthy()
+  // The build artifacts are only present if the post-approval install
+  // ran the package's install scripts.
+  expect(fs.existsSync(path.join(pkgPath!, 'generated-by-install.js'))).toBe(true)
+})
+
 // CONTEXT: dangerously-allow-all-builds has been removed from rc files, as a result, this test no longer applies
 // TODO: Maybe we should create a yaml config file specifically for `--global`? After all, this test is to serve such use-cases
 test.skip('dangerously-allow-all-builds=true in global config', async () => {