Conversation
… 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.
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.
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.
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.
- 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.
zkochan
added a commit
that referenced
this pull request
Apr 30, 2026
… 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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
pnpm add -gwould crash withENOENTon Windows when the install triggered the approve-builds prompt and the user approved a build.promptApproveGlobalBuildsforwarded an absolutemodulesDir(<installDir>/node_modules) intoapprove-builds, which then passed it through toinstall.handler. The install layer treatsmodulesDiras relative tolockfileDirand joined them again. On Windows,path.joindoes not collapse an embedded absolute path, so the result was<installDir>\<installDir>\node_modules\.pnpm\.... The hoist step then failed trying tomkdirthat doubled path.modulesDir: undefined. Bothapprove-builds(getModulesDirderivespath.join(lockfileDir, 'node_modules')) andinstall.handler(defaultsmodulesDirto'node_modules'and joins withlockfileDir) end up with the correct path on every OS.Closes #11403.
Test plan
global/commands/test/promptApproveGlobalBuilds.test.tsassertsmodulesDirisundefinedwhen forwarded toapprove-builds. Verified the test fails when the absolutemodulesDiris reintroduced.pnpm --filter @pnpm/global.commands run lintclean.pnpm --filter @pnpm/global.commands test(13/13 passing).