fix(setup): skip `@pnpm/exe` build scripts during global self-install by zkochan · Pull Request #12402 · pnpm/pnpm

zkochan · 2026-06-14T09:29:39Z

Problem

When pnpm setup runs from the standalone executable, it installs pnpm into the global directory via pnpm add -g file:<execDir>. The @pnpm/exe package.json shipped alongside the binary carries preinstall/prepare scripts (setup.js/prepare.js), so the install prints a build-approval prompt for @pnpm/exe — pnpm asking to allow building itself, which understandably reads as "someone pushed postinstall malware to pnpm." (pnpm self-update doesn't show this because it auto-trusts @pnpm/exe and links the platform binary itself.)

Fix

Pass --ignore-scripts to the pnpm add -g invocation in installCliGlobally. Those scripts have no job on this path anyway:

the binary is already present (it is the standalone executable);
the optional platform packages (@pnpm/macos-arm64, …) aren't installed next to the file: dependency, so setup.js would only ERR_MODULE_NOT_FOUND;
a Node-less SEA host has no node to run the scripts at all;
pnpm already performs the platform-binary hardlinking itself via linkExePlatformBinary.

With --ignore-scripts, @pnpm/exe is never added to the ignoredBuilds set (building/during-install/src/index.ts), so no prompt or warning is emitted — and no script is run, so nothing can fail.

Test

Added a regression test that forces the SEA install branch and asserts the spawned arguments are exactly ['add', '-g', '--ignore-scripts', 'file:<dir>'].

Notes

setup is a TypeScript-only command (not in pacquet's install/add/update/remove surface), so no pacquet-side port is needed.

Written by an agent (Claude Code, claude-opus-4-8).

Summary by CodeRabbit

Improvements
- pnpm setup no longer prompts to approve build scripts when installing the standalone executable, streamlining the setup process.

When running from the standalone executable, `pnpm setup` installs pnpm via `pnpm add -g file:<dir>`. The shipped `@pnpm/exe` package.json carries preinstall/prepare scripts, which triggered a build-approval prompt for pnpm's own install. pnpm links the platform-specific binary itself, so these scripts are unnecessary (and unrunnable on a Node-less host); pass --ignore-scripts to skip them. Closes #12377

qodo-free-for-open-source-projects · 2026-06-14T09:29:46Z

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0)

Context used

✅ Tickets: 🎫 pnpm asks for "allowing build" for @pnpm/exe

1. execPath descriptor not restored 🐞 Bug ☼ Reliability

Description

The new test overrides process.execPath using Object.defineProperty without saving/restoring the
original property descriptor, which can silently change writable/enumerable flags and leak
mutated process state into later tests. This can cause cross-test interference/flakiness in the Jest
worker process after this test runs.

Code

engine/pm/commands/test/setup/setup.test.ts[R112-118]

+  const originalExecPath = process.execPath
+  Object.defineProperty(process, 'execPath', { value: execPath, configurable: true })
+  try {
+    await setup.handler({ pnpmHomeDir: path.join(tmpDir, 'home') })
+  } finally {
+    Object.defineProperty(process, 'execPath', { value: originalExecPath, configurable: true })
+    actualFs.rmSync(tmpDir, { recursive: true, force: true })

Evidence

The test currently redefines process.execPath with a partial descriptor and restores only the
value, not the full descriptor. The repo’s forceInteractiveTty() helper demonstrates the
established safe approach: store the original descriptor and restore it in cleanup, avoiding
descriptor/attribute leaks across tests.

engine/pm/commands/test/setup/setup.test.ts[104-124]
releasing/commands/test/stage.test.ts[438-455]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The test mutates `process.execPath` via `Object.defineProperty(process, 'execPath', { value, configurable: true })` and later “restores” it the same way. Because the original descriptor is not preserved, the mutation can permanently change descriptor attributes (e.g. `writable`/`enumerable`) for the remainder of the Jest process, potentially impacting other tests.

### Issue Context
A safe pattern already exists in the repo: capture the original descriptor with `Object.getOwnPropertyDescriptor` and restore it in cleanup.

### Fix Focus Areas
- engine/pm/commands/test/setup/setup.test.ts[112-118]

### Suggested fix
- Capture the original descriptor:
 - `const originalExecPathDesc = Object.getOwnPropertyDescriptor(process, 'execPath')`
- Override `execPath` while preserving other attributes (prefer spreading the original descriptor when present):
 - `Object.defineProperty(process, 'execPath', { ...originalExecPathDesc, value: execPath, configurable: true })`
- In `finally`, restore the original descriptor if it existed, otherwise delete the property (matching the repo pattern used elsewhere).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

coderabbitai · 2026-06-14T09:29:54Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: a544b59c-bcb2-4596-a648-d04b708b988a

📥 Commits

Reviewing files that changed from the base of the PR and between baf1502 and 1041f6a.

📒 Files selected for processing (3)

.changeset/setup-skip-exe-build-scripts.md
engine/pm/commands/src/setup/setup.ts
engine/pm/commands/test/setup/setup.test.ts

📜 Recent review details

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

GitHub Check: Compile & Lint

🧰 Additional context used

📓 Path-based instructions (2)

**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ts,tsx}: Use Standard Style with trailing commas, prefer functions over classes, declare functions after they are used relying on hoisting, limit function arguments to two or three with options objects for additional parameters
Follow import order: standard libraries, external dependencies (sorted alphabetically), then relative imports
Use JSDoc for function contracts (preconditions, postconditions, edge cases, why it exists) not for re-narrating the body; do not record past implementation shape or refactor history in comments

Files:

engine/pm/commands/src/setup/setup.ts
engine/pm/commands/test/setup/setup.test.ts

**/*.test.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

When checking if a caught error is an Error object in Jest, use util.types.isNativeError() instead of instanceof Error because instanceof checks can fail across VM realms

Files:

engine/pm/commands/test/setup/setup.test.ts

🧠 Learnings (4)

📚 Learning: 2026-05-26T21:01:06.666Z

Learnt from: zkochan
Repo: pnpm/pnpm PR: 11966
File: .changeset/require-tarball-integrity.md:6-6
Timestamp: 2026-05-26T21:01:06.666Z
Learning: In pnpm lockfile-related release notes/docs (especially changeset markdown), preserve URL hostnames exactly as they appear in pnpm-lock.yaml tarball resolution entries—keep hosts like `codeload.github.com`, `bitbucket.org`, and `gitlab.com` in lowercase. Do not “correct” them to title-case/preserve brand capitalization (e.g., LanguageTool rules like `GITHUB` capitalization) because these are literal URL fragments, not platform brand names.

Applied to files:

.changeset/setup-skip-exe-build-scripts.md

📚 Learning: 2026-05-14T09:04:00.133Z

Learnt from: zkochan
Repo: pnpm/pnpm PR: 11622
File: resolving/npm-resolver/test/publishedBy.test.ts:350-354
Timestamp: 2026-05-14T09:04:00.133Z
Learning: In the pnpm/pnpm repository, ESLint is the authoritative style linter. Do not raise review findings for missing trailing commas in multiline function calls (e.g., `fs.writeFileSync(...)`) when this repo’s ESLint configuration does not report them and lint passes. Prefer deferring to the ESLint results for this specific trailing-comma rule rather than enforcing it manually in code review.

Applied to files:

engine/pm/commands/src/setup/setup.ts
engine/pm/commands/test/setup/setup.test.ts

📚 Learning: 2026-06-05T13:47:26.046Z

Learnt from: vsumner
Repo: pnpm/pnpm PR: 12190
File: installing/deps-installer/src/install/index.ts:2337-2343
Timestamp: 2026-06-05T13:47:26.046Z
Learning: In the pnpm/pnpm codebase, `PnpmError` automatically prefixes `err.code` with `ERR_PNPM_` when you pass a code that does not already start with `ERR_PNPM_` (it normalizes `this.code` via `code.startsWith('ERR_PNPM_') ? code : `ERR_PNPM_${code}``). Therefore, during code review you should NOT flag `new PnpmError(...)` call sites for passing a bare error code (e.g., `new PnpmError('FROZEN_STORE_INCOMPATIBLE_WITH_PNPR', ...)`); the resulting `err.code` will still be `ERR_PNPM_FROZEN_STORE_INCOMPATIBLE_WITH_PNPR`.

Applied to files:

engine/pm/commands/src/setup/setup.ts
engine/pm/commands/test/setup/setup.test.ts

📚 Learning: 2026-06-05T13:47:05.929Z

Learnt from: vsumner
Repo: pnpm/pnpm PR: 12190
File: installing/deps-installer/test/install/frozenStore.ts:2-17
Timestamp: 2026-06-05T13:47:05.929Z
Learning: In the pnpm/pnpm repository, the shared Jest preset keeps `injectGlobals` at its default (`true`), so `test` and `expect` are available as Jest globals. Therefore, reviewers should not flag (or treat as TypeScript/compilation errors) missing `import { test, expect } from 'jest/globals'` when a test file uses `test`/`expect` without importing them. Importing from `jest/globals` may still be used for consistency with sibling files, but it is not required for execution in this repo unless a Jest preset is explicitly configured with `injectGlobals: false`.

Applied to files:

engine/pm/commands/test/setup/setup.test.ts

🔇 Additional comments (6)

engine/pm/commands/src/setup/setup.ts (1)

87-95: LGTM!

engine/pm/commands/test/setup/setup.test.ts (4)

1-2: LGTM!

12-22: LGTM!

37-38: LGTM!

104-124: LGTM!

.changeset/setup-skip-exe-build-scripts.md (1)

1-6: LGTM!

📝 Walkthrough

Walkthrough

The pnpm setup global install step now passes --ignore-scripts when running pnpm add -g from file:${execDir} in the standalone executable flow. A new test mocks detectIfCurrentPkgIsExecutable and spawnSync to verify the flag is present. A changeset entry documents the patch.

Changes

Skip @pnpm/exe build scripts in pnpm setup

Layer / File(s)	Summary
`--ignore-scripts` flag in setup handler and verification test `engine/pm/commands/src/setup/setup.ts`, `engine/pm/commands/test/setup/setup.test.ts`, `.changeset/setup-skip-exe-build-scripts.md`	`setup.ts` adds `--ignore-scripts` to the `spawnSync` invocation for the `file:`-based global install and adds a comment explaining that `preinstall`/`prepare` scripts from `@pnpm/exe` are intentionally skipped. The test file introduces `os`/`path` imports, `jest.unstable_mockModule` stubs for `detectIfCurrentPkgIsExecutable` and `spawnSync`, and a new test case that temporarily overrides `process.execPath` and asserts `spawnSync` is called once with `['add', '-g', '--ignore-scripts', 'file:<tmpDir>']`. The changeset records the patch for `pnpm` and `@pnpm/engine.pm.commands`.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐇 No more scary build prompts, hooray!
The rabbit said, "Skip those scripts today."
--ignore-scripts tucked right in line,
The exe installs perfectly fine.
A test confirms the flag is there —
Hop along, nothing suspicious, I swear! 🌿

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: adding `--ignore-scripts` flag to skip build scripts for `@pnpm/exe` during the global setup installation process.
Linked Issues check	✅ Passed	The PR successfully addresses issue `#12377` by preventing the build approval prompt for `@pnpm/exe` during `pnpm setup` through the `--ignore-scripts` flag.
Out of Scope Changes check	✅ Passed	All changes are directly related to the issue: updated setup command implementation, added regression test, and created changeset entry documenting the fix.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch fix-setup

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint install timed out. The project may have too many dependencies for the sandbox.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

qodo-free-for-open-source-projects · 2026-06-14T09:30:30Z

PR Summary by Qodo

fix(setup): ignore @pnpm/exe scripts during standalone global self-install
🐞 Bug fix 🧪 Tests 📝 Documentation 🕐 10-20 Minutes

Walkthroughs

Description

• Add --ignore-scripts when SEA pnpm setup installs itself globally.
• Prevent build-approval prompt for @pnpm/exe during standalone setup.
• Add regression test asserting exact global-install spawn arguments.

Diagram

graph TD
U(["Run pnpm setup"]) --> H["setup.handler"] --> D{"Standalone exe?"}
D -->|yes| I["installCliGlobally"] --> S["spawnSync: add -g --ignore-scripts file:<dir>"] --> A["addDirToEnvPath"]
D -->|no| A
subgraph Legend
direction LR
_start([Start]) ~~~ _dec{Decision} ~~~ _step[Process]
end

High-Level Assessment

The following are alternative approaches to this PR:

1. Auto-trust @pnpm/exe on this install path

➕ Preserves script execution if it ever becomes required for this code path
➕ Keeps behavior closer to pnpm self-update’s trust model
➖ Still runs scripts on hosts that may not have Node (SEA scenario)
➖ More policy/complexity: needs trust rules and maintenance

2. Remove/avoid shipping scripts in the packaged @pnpm/exe manifest for SEA

➕ Eliminates prompts without changing install command flags
➕ Avoids relying on install-time flags correctness
➖ Packaging/build pipeline change; higher surface area and regression risk
➖ Harder to guarantee across all release artifacts and distribution methods

Recommendation: Keep the current approach: passing --ignore-scripts to the SEA self-install (pnpm add -g file:) is the lowest-risk, most targeted fix. It directly prevents the build-approval prompt and avoids executing scripts that are irrelevant/unrunnable in this scenario, and the added regression test locks in the intended spawn arguments.

File Changes

Bug fix (1)

setup.ts Skip scripts when globally self-installing from the standalone executable +9/-1
Skip scripts when globally self-installing from the standalone executable
• Updates the SEA global install path to run 'pnpm add -g --ignore-scripts file:<execDir>'. This avoids triggering build-approval prompts for @pnpm/exe’s preinstall/prepare scripts and prevents failures on Node-less SEA hosts.
engine/pm/commands/src/setup/setup.ts

Tests (1)

setup.test.ts Add regression test for SEA global install args +39/-0
Add regression test for SEA global install args
• Adds a test that forces the standalone-executable branch and asserts spawnSync is called with '['add','-g','--ignore-scripts','file:<dir>']'. Introduces mocks for @pnpm/cli.meta and node:child_process and temporarily overrides process.execPath.
engine/pm/commands/test/setup/setup.test.ts

Documentation (1)

setup-skip-exe-build-scripts.md Add changeset note for setup self-install script skipping +6/-0
Add changeset note for setup self-install script skipping
• Adds a patch changeset for @pnpm/engine.pm.commands and pnpm documenting that standalone 'pnpm setup' no longer prompts to approve @pnpm/exe build scripts during global self-install.
.changeset/setup-skip-exe-build-scripts.md

coderabbitai Bot approved these changes Jun 14, 2026

View reviewed changes

zkochan merged commit 0fe6ea9 into main Jun 14, 2026
14 of 15 checks passed

zkochan deleted the fix-setup branch June 14, 2026 09:59

zkochan mentioned this pull request Jun 14, 2026

fix(setup): allow @pnpm/exe build during pnpm setup #12381

Closed

coderabbitai Bot mentioned this pull request Jun 16, 2026

feat(pacquet): add --ignore-scripts to install #12454

Merged

aqeelat mentioned this pull request Jun 18, 2026

pnpm self-update keeps warning about v10 layout even after pnpm setup — legacy shim at PNPM_HOME is never cleaned up and can become dangling #12496

Open

4 tasks

coderabbitai Bot mentioned this pull request Jun 27, 2026

feat(pacquet-cli): port the setup command #12697

Merged

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(setup): skip `@pnpm/exe` build scripts during global self-install#12402

fix(setup): skip `@pnpm/exe` build scripts during global self-install#12402
zkochan merged 1 commit into
mainfrom
fix-setup

zkochan commented Jun 14, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

qodo-free-for-open-source-projects Bot commented Jun 14, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Jun 14, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Poem

Uh oh!

qodo-free-for-open-source-projects Bot commented Jun 14, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Uh oh!

Conversation

zkochan commented Jun 14, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Fix

Test

Notes

Summary by CodeRabbit

Uh oh!

qodo-free-for-open-source-projects Bot commented Jun 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review by Qodo

Uh oh!

coderabbitai Bot commented Jun 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Poem

Uh oh!

qodo-free-for-open-source-projects Bot commented Jun 14, 2026

PR Summary by Qodo

Walkthroughs

File Changes

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

zkochan commented Jun 14, 2026 •

edited by coderabbitai Bot

Loading

qodo-free-for-open-source-projects Bot commented Jun 14, 2026 •

edited

Loading

coderabbitai Bot commented Jun 14, 2026 •

edited

Loading