fix: update GVS symlinks after approve-builds by running install by zkochan · Pull Request #11043 · pnpm/pnpm

zkochan · 2026-03-20T21:36:20Z

Summary

Fixes #11042

Root cause: When enableGlobalVirtualStore is true and allowBuilds is not configured, createAllowBuildFunction() returned undefined, causing all GVS hashes to include ENGINE_NAME. When approve-builds later configured allowBuilds, the hash didn't change because the engine was already included.
Fix: Default allowBuilds to {} in GVS mode so hashes are engine-agnostic by default, and have approve-builds call install.handler() in GVS mode instead of the low-level install() function, so it properly handles workspaces and updates symlinks.
Refactor: Broke circular dependencies between building/commands, installing/commands, and global/commands using dependency injection via a commands map passed as the third argument to command handlers. Added CommandHandler and CommandHandlerMap types to @pnpm/cli.command.

Changes

Architecture

Command handlers now receive a commands map as an optional third argument (opts, params, commands?)
The CLI dispatcher in main.ts passes the full commands map to every handler
Handlers that need other commands (e.g., globalAdd needs approve-builds, recursive needs rebuild) access them from this map
This replaces direct cross-package imports that would create circular dependencies

Packages changed

@pnpm/cli.command — new CommandHandler and CommandHandlerMap types
@pnpm/building.commands — approve-builds uses install.handler for GVS
@pnpm/global.commands — removed building/commands dependency; receives approve-builds via commands map
@pnpm/installing.commands — receives rebuild via commands map instead of direct import
@pnpm/installing.deps-installer / @pnpm/installing.deps-restorer — default allowBuilds to {} in GVS mode
pnpm CLI — dispatcher passes commands map to all handlers

Test plan

e2e test (pnpm/test/install/globalVirtualStore.ts): install with GVS → approve-builds --all → verify new hash directory + build artifacts accessible via node_modules
All existing approve-builds tests pass (14/14)
All existing GVS tests pass
Full compile + lint + meta-updater pass

🤖 Generated with Claude Code

Copilot

Pull request overview

Fixes GVS (global virtual store) symlink/hash inconsistencies when approve-builds changes allowBuilds, ensuring builds land in a new hash directory and node_modules points at the correct location.

Changes:

Default allowBuilds to {} in GVS mode so GVS hashing doesn’t depend on createAllowBuildFunction() being undefined.
In GVS mode, make approve-builds trigger a headless install() instead of rebuild to refresh GVS hash directories and symlinks.
Refactor installing/commands to call buildProjects() directly (removing dependency on @pnpm/building.commands).

Reviewed changes

Copilot reviewed 12 out of 13 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
pnpm/test/install/globalVirtualStore.ts	Adds e2e coverage for `approve-builds` updating GVS hash dir + symlinks and persisting `allowBuilds`.
pnpm-lock.yaml	Updates workspace lockfile entries due to package dependency graph changes.
installing/deps-restorer/src/index.ts	Defaults `allowBuilds` to `{}` in headless (restorer) path when GVS is enabled.
installing/deps-installer/test/install/globalVirtualStore.ts	Adds unit test for hash change + artifacts after `allowBuilds` change in GVS mode.
installing/deps-installer/src/install/extendInstallOptions.ts	Defaults `allowBuilds` to `{}` in GVS mode during option extension.
installing/commands/tsconfig.json	Removes project reference to `building/commands` to break a circular dependency.
installing/commands/src/recursive.ts	Replaces `rebuild.handler()` call with `buildProjects()` call and passes store controller/dir explicitly.
installing/commands/package.json	Removes dependency on `@pnpm/building.commands`.
building/commands/tsconfig.json	Adds project reference to `installing/deps-installer`.
building/commands/test/policy/approveBuilds.test.ts	Makes helper install command configurable for `enableGlobalVirtualStore`.
building/commands/src/policy/approveBuilds.ts	In GVS mode, runs a headless install after updating `allowBuilds`.
building/commands/package.json	Adds dependency on `@pnpm/installing.deps-installer`.
.changeset/fix-gvs-approve-builds.md	Adds changeset describing the CLI behavior change in GVS mode.

Files not reviewed (1)

pnpm-lock.yaml: Language not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

When `enableGlobalVirtualStore` is true and `allowBuilds` is not configured, GVS hashes incorrectly included ENGINE_NAME for all packages. This meant that approving builds later wouldn't change the hash, so symlinks were never updated and build artifacts ended up in the wrong directory. Root cause: `createAllowBuildFunction()` returned `undefined` when `allowBuilds` was not configured, causing `builtDepPaths` to be `undefined` and `includeEngine` to be `true` for all packages. Fix: - Default `allowBuilds` to `{}` in GVS mode (in extendInstallOptions and headlessInstall) so hashes are engine-agnostic by default - In approve-builds handler, call `install()` from deps-installer in GVS mode (with frozenLockfile) instead of rebuild, so the headless install detects the allowBuilds change and re-links - Break circular dependency: replace rebuild.handler() call in installing/commands with direct buildProjects() from building/after-install Closes #11042 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Workspace roots may not have a package.json. Use the manifest already available from config rather than reading from disk. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

- Pass dir, lockfileDir, modulesDir to install() for correct context - Throw descriptive error when package.json can't be read - Add deps-installer and deps-restorer to changeset Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

The previous change only passed the root project manifest, which meant workspace project build scripts didn't run. Now passes all workspace projects, matching the original recursiveRebuild behavior. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Revert to single-project buildProjects call matching original rebuild.handler() behavior. Use rootProjectManifest from config (falls back to {} for workspace roots without package.json). Passing all workspace projects caused wrong build order and overwrote per-project module settings like hoistPattern. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

…attern Use allProjects for correct workspace builds (topological ordering, all importers walked). Pass hoistPattern/publicHoistPattern as undefined so buildProjects doesn't overwrite per-project module settings that were configured during the install phase. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Sort projects via selectedProjectsGraph to assign correct buildIndex per topological chunk. Destructure out hoistPattern/publicHoistPattern from opts to prevent buildProjects from overwriting per-project module settings. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Move rebuild handler and recursive rebuild to a new package to break the circular dependency between installing/commands and building/commands. Now: - installing/commands depends on building.rebuild-command (not building.commands) - building/commands re-exports from building.rebuild-command - building/commands can depend on installing.deps-installer for GVS Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 19 out of 20 changed files in this pull request and generated 3 comments.

Files not reviewed (1)

pnpm-lock.yaml: Language not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Only default allowBuilds to {} in extendInstallOptions (the install() entry point), not in headlessInstall itself. Direct callers like installPnpm pass enableGlobalVirtualStore: true without allowBuilds and expect engine-aware hashes. Defaulting in headlessInstall changed those hashes and broke pnpm version switching. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

- Fix grammar in --pending help text ("were not built") - Use path.join for package.json path construction - Add @pnpm/building.rebuild-command to changeset - Add README.md for rebuild-command package Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Add the default in both the config reader (for CLI installs) and extendInstallOptions (for programmatic install() calls). This ensures GVS hashes are engine-agnostic by default regardless of entry point. headlessInstall is intentionally not changed since direct callers like installPnpm need engine-aware hashes. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

- Default allowBuilds to {} in headlessInstall, extendInstallOptions, and the config reader when GVS is enabled - Fix installPnpm to pass allowBuilds: { '@pnpm/exe': true } to both findPnpmGvsPath and installFromLockfile so hash computation is consistent (exe has platform-specific binaries needing engine hashes) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 24 out of 25 changed files in this pull request and generated 3 comments.

Files not reviewed (1)

pnpm-lock.yaml: Language not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

…bal/commands Pass approveBuilds handler via options instead of importing it directly in global/commands, and use install.handler in approve-builds for proper workspace support instead of the low-level install function. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Use dependency injection for rebuild.handler (same pattern as approveBuilds) to avoid the circular dependency between building/commands and installing/commands. The withBuildingHandlers wrapper in the CLI injects both handlers into opts. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

…a opts Introduce CommandHandler and CommandHandlerMap types in @pnpm/cli.command. The CLI dispatcher passes the commands map as a third argument to handlers. Handlers that need other commands (globalAdd, globalUpdate, recursive) access them from this map instead of receiving callbacks via opts. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 29 out of 30 changed files in this pull request and generated 2 comments.

Files not reviewed (1)

pnpm-lock.yaml: Language not supported

Comments suppressed due to low confidence (2)

installing/commands/src/recursive.ts:471

recursive() always runs project installs with ignoreScripts: true (see line 410), so the final rebuild step is required to actually execute lifecycle scripts. With the new optional rebuildHandler, this can silently skip rebuilding when the caller doesn't provide it, leaving installs/adds/updates in workspaces without built artifacts. Either make rebuildHandler required when !opts.ignoreScripts for add/install/update, or throw a clear error if it's missing (and ensure all call sites pass it).

    await opts.rebuildHandler?.({
      ...opts,
      pending: opts.pending === true,
      skipIfHasSideEffectsCache: true,
    }, [])

installing/commands/src/update/index.ts:186

For non-global updates, commands is currently ignored and installDeps() is called without rebuildHandler. In workspace/recursive mode, recursive() installs with ignoreScripts: true and depends on opts.rebuildHandler to actually run builds. Please thread rebuildHandler: commands?.rebuild through the non-global update flow (either by passing it in the installDeps() call or by extending opts before calling update()).

  if (opts.interactive) {
    return interactiveUpdate(params, opts)
  }
  return update(params, opts) as Promise<undefined>
}

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Without this, the GVS install triggered by approve-builds wouldn't get the rebuild handler, causing builds to not run after approval. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Ensures add, update, and dedupe handlers forward the rebuild command so recursive installs trigger the rebuild step. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

…mistic install The optimistic repeat install check in installDeps would bail out with "Already up to date" since the lockfile hasn't changed, causing the GVS reinstall to be skipped entirely. Setting optimisticRepeatInstall: false ensures the install actually runs. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Required params are not assignable to the Command type which has commands as optional. Keep it optional in the signature — the CLI always provides it. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Copilot AI review requested due to automatic review settings March 20, 2026 21:36

Copilot started reviewing on behalf of zkochan March 20, 2026 21:36 View session

Copilot AI reviewed Mar 20, 2026

View reviewed changes

Comment thread building/commands/src/policy/approveBuilds.ts Outdated

Comment thread building/commands/src/policy/approveBuilds.ts Outdated

Comment thread .changeset/fix-gvs-approve-builds.md

zkochan force-pushed the fix/11042 branch from be89be4 to cf48e21 Compare March 20, 2026 21:47

zkochan and others added 9 commits March 20, 2026 22:50

fix: use rootProjectManifest from config instead of reading from disk

dc2a542

Workspace roots may not have a package.json. Use the manifest already available from config rather than reading from disk. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

fix: address review comments

aafbd3f

- Pass dir, lockfileDir, modulesDir to install() for correct context - Throw descriptive error when package.json can't be read - Add deps-installer and deps-restorer to changeset Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

fix: remove publicHoistPattern destructure (not on RecursiveOptions)

e9a5234

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

fix: correct homepage URL in building.rebuild-command package.json

64bce58

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

zkochan requested a review from Copilot March 21, 2026 00:21

Copilot started reviewing on behalf of zkochan March 21, 2026 00:21 View session

Copilot AI reviewed Mar 21, 2026

View reviewed changes

Comment thread building/rebuild-command/src/rebuild.ts Outdated

Comment thread building/commands/src/policy/approveBuilds.ts Outdated

Comment thread .changeset/fix-gvs-approve-builds.md

zkochan and others added 6 commits March 21, 2026 01:34

chore: add building/policy tsconfig reference for engine/pm/commands

a14060d

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

fix: use PNPM_ALLOW_BUILDS constant in installPnpmToGlobalDir too

7ca8c2f

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

zkochan requested a review from Copilot March 21, 2026 01:17

Copilot started reviewing on behalf of zkochan March 21, 2026 01:17 View session

zkochan added the area: global virtual store label Mar 21, 2026

Copilot AI reviewed Mar 21, 2026

View reviewed changes

Comment thread building/commands/src/policy/approveBuilds.ts Outdated

Comment thread building/commands/src/policy/approveBuilds.ts Outdated

Comment thread building/rebuild-command/package.json Outdated

zkochan and others added 3 commits March 21, 2026 02:34

chore: add @pnpm/logger to rebuild-command devDependencies

14b0e32

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

chore: fix tsconfig reference for rebuild-command logger

69de231

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

zkochan and others added 5 commits March 21, 2026 02:47

fix: fix compile errors in approveBuilds test types

728cd75

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

chore: remove accidentally committed generated files

c3bfbc7

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

chore: add cli/command tsconfig reference for global/commands

887c7c8

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

zkochan requested a review from Copilot March 21, 2026 02:27

Copilot started reviewing on behalf of zkochan March 21, 2026 02:28 View session

zkochan and others added 2 commits March 21, 2026 03:28

chore: remove accidentally tracked __typecheck__/tsconfig.json

21f8cda

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

chore: remove unnecessary truthiness checks on commands map

013df9f

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Copilot AI reviewed Mar 21, 2026

View reviewed changes

Comment thread installing/commands/src/add.ts

Comment thread building/commands/src/policy/approveBuilds.ts

zkochan and others added 3 commits March 21, 2026 11:29

fix: forward commands to install.handler in approveBuilds

f6d9427

Without this, the GVS install triggered by approve-builds wouldn't get the rebuild handler, causing builds to not run after approval. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

chore: add cli/command tsconfig reference for building/commands

e72babf

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

fix: pass rebuildHandler from commands to installDeps in all handlers

7db016f

Ensures add, update, and dedupe handlers forward the rebuild command so recursive installs trigger the rebuild step. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

zkochan changed the title ~~fix: update GVS symlinks after approve-builds~~ fix: update GVS symlinks after approve-builds by running install Mar 21, 2026

zkochan and others added 2 commits March 21, 2026 12:14

zkochan merged commit 9fc552d into main Mar 21, 2026
12 checks passed

zkochan deleted the fix/11042 branch March 21, 2026 11:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: update GVS symlinks after approve-builds by running install#11043

fix: update GVS symlinks after approve-builds by running install#11043
zkochan merged 31 commits intomainfrom
fix/11042

zkochan commented Mar 20, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

zkochan commented Mar 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Architecture

Packages changed

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

zkochan commented Mar 20, 2026 •

edited

Loading