Skip to content

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

Merged
zkochan merged 6 commits into
mainfrom
fix/11403
Apr 30, 2026
Merged

fix(global): avoid doubled modulesDir in approve-builds during global add#11404
zkochan merged 6 commits into
mainfrom
fix/11403

Conversation

@zkochan

@zkochan zkochan commented Apr 30, 2026

Copy link
Copy Markdown
Member

Summary

  • pnpm add -g would crash with ENOENT on Windows when the install triggered the approve-builds prompt and the user approved a build.
  • Root cause: promptApproveGlobalBuilds forwarded an absolute modulesDir (<installDir>/node_modules) into approve-builds, which then passed it through to install.handler. The install layer treats modulesDir as relative to lockfileDir and joined them again. On Windows, path.join does not collapse an embedded absolute path, so the result was <installDir>\<installDir>\node_modules\.pnpm\.... The hoist step then failed trying to mkdir that doubled path.
  • Fix: pass modulesDir: undefined. Both approve-builds (getModulesDir derives path.join(lockfileDir, 'node_modules')) and install.handler (defaults modulesDir to 'node_modules' and joins with lockfileDir) end up with the correct path on every OS.

Closes #11403.

Test plan

  • New unit test in global/commands/test/promptApproveGlobalBuilds.test.ts asserts modulesDir is undefined when forwarded to approve-builds. Verified the test fails when the absolute modulesDir is reintroduced.
  • pnpm --filter @pnpm/global.commands run lint clean.
  • pnpm --filter @pnpm/global.commands test (13/13 passing).

zkochan added 6 commits April 30, 2026 13:50
… 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 zkochan merged commit dfa8258 into main Apr 30, 2026
12 checks passed
@zkochan zkochan deleted the fix/11403 branch April 30, 2026 15:05
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Error while trying to symlink

1 participant