feat: pnpm login (continuation)#11093
Closed
KSXGitHub wants to merge 114 commits into
Closed
Conversation
Packages whose tests spawn the local pnpm CLI (pnpm/bin/pnpm.mjs) need the bundle (pnpm/dist/pnpm.mjs) to exist. Add `pnpm --filter pnpm run compile` to their test scripts so the bundle is built before tests run.
…10975) BIN_OWNER_OVERRIDES was only used in checkGlobalBinConflicts for global installs. This change applies the same ownership rules in compareCommandsInConflict so that conflict resolution is consistent between global conflict checking and actual bin linking. This ensures packages like npm get priority for bins like npx even in non-global installs. Closes #10850 * test(link-bins): add missing fixture for bin-owner-override test * refactor: extract BIN_OWNER_OVERRIDES to @pnpm/package-bins Move shared logic to avoid code duplication between link-bins and checkGlobalBinConflicts. * fix(link-bins): use regex for Windows path compatibility in test * refactor(link-bins): remove redundant ownName field pkgOwnsBin already handles the binName === pkgName case, making the ownName field and its associated checks redundant. * Change versioning to patch for bins resolver and linker Added BIN_OWNER_OVERRIDES and pkgOwnsBin to @pnpm/bins.resolver for improved conflict resolution in bin linking. * test: remove node_modules from bin-owner-override fixture Move fixture packages to the directory root instead of nesting them inside node_modules, avoiding committing node_modules to the repo. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Zoltan Kochan <z@kochan.io> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* feat: implement non-interactive version command * fix: address review issues in version command - Fix changeset package name to @pnpm/releasing.commands - Use writeProjectManifest instead of writeJsonFile to preserve formatting - Remove dead updateWorkspaceDependencies placeholder function - Remove unused imports (path, ProjectManifest, writeJsonFile) - Add expect.assertions(1) to prevent silent test pass on no-throw Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Zoltan Kochan <z@kochan.io> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add a "Resolving Conflicts in GitHub PRs" section to AGENTS.md with step-by-step instructions for force-fetching refs, rebasing, resolving lockfile conflicts, and verifying mergeability. Add shell/resolve-pr-conflicts.sh that automates the workflow: fetches metadata, force-updates the base ref, rebases, auto-resolves lockfile conflicts via pnpm install, force-pushes, and verifies the result.
…Windows (#11004) * fix: use ENOENT check instead of which.sync for command-not-found on Windows On Windows, `which.sync()` only checks if a command exists in PATH, not whether it actually executed successfully. This caused false "Command not found" errors when a command exists but exits with a non-zero code. Use the same `spawn ENOENT` check across all platforms, which is reliable thanks to cross-spawn used by execa. Closes #11000 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: resolve prependPaths against exec prefix for correct Windows command lookup The previous ENOENT-only approach doesn't work on Windows because execa 9.x uses cross-spawn only for command parsing, not spawning. This means cross-spawn's ENOENT hook (hookChildProcess) never fires, and non-existent commands wrapped as `cmd.exe /c <command>` exit with code 1 instead of emitting ENOENT. Restore the which.sync fallback for Windows, but fix the original #11000 bug by resolving relative prependPaths (like ./node_modules/.bin) against the exec prefix instead of relying on process.cwd(). This ensures correct path resolution in --filter contexts where the command runs in a different package directory. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: zubeyralmaho <zubeyralmaho@users.noreply.github.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Replace the hardcoded command name list in main.ts with a declarative recursiveByDefault property on CommandDefinition. Each command that should run workspace-wide by default now exports this property. Also adds recursiveByDefault to list, ll, and why commands.
Adds a `--check-peers` flag to `pnpm list` that detects unmet and missing peer dependency issues by reading the lockfile. This allows users to check for peer dependency problems without triggering a full resolution, which is especially useful in CI or after pulling a lockfile from another developer. Closes #7087
* perf: skip redundant GVS internal linking on warm reinstall When GVS is enabled and the store is warm (added === 0), skip re-creating internal symlinks, re-linking bins inside the GVS store, and re-importing packages since they already persist outside node_modules/. Also filter directPkgDirs by hasBin to avoid unnecessary package.json reads when linking direct dep bins. * fix: preserve link: deps in hasBin filter for bin linking The hasBin filter was dropping directories not present in the dep graph (e.g. link: dependencies), which would silently break bin linking for linked local packages that expose binaries. --------- Co-authored-by: Zoltan Kochan <z@kochan.io>
Co-authored-by: rohan436 <rohan.santhoshkumar@googlemail.com>
…11071) * feat: add `dedupePeers` option to reduce peer dependency duplication When enabled, this option applies two optimizations to peer dependency resolution: 1. Version-only peer suffixes: Uses name@version instead of full dep paths (including nested peer suffixes) when building peer identity hashes. This eliminates deeply nested suffixes like (foo@1.0.0(bar@2.0.0)). 2. Transitive peer pruning: Only directly declared peer dependencies are included in a package's suffix. Transitive peers from children are not propagated upward, preventing combinatorial explosion while maintaining correct node_modules layout. The option is scoped per-project: each workspace project defines a peer resolution environment, and all packages within that project's tree share that environment. Projects with different peer versions correctly produce different instances. Closes #11070 * fix: pass dedupePeers to getOutdatedLockfileSetting and use spread for lockfile write The frozen install path (used by approve-builds) calls getOutdatedLockfileSetting but was missing the dedupePeers parameter. This caused a false LOCKFILE_CONFIG_MISMATCH error because the lockfile had the key written (as undefined/null via YAML serialization) while the check function received undefined for the config value. Fix: pass dedupePeers to the settings check call, and use spread syntax to only write the dedupePeers key to lockfile settings when it's truthy (avoiding undefined keys). * fix: write dedupePeers to lockfile like other settings Write the value directly instead of spread syntax, and use the same != null guard pattern as autoInstallPeers in the settings checker. * test: add integration test for dedupePeers in peerDependencies.ts * fix: only write dedupePeers to lockfile when enabled When dedupePeers is false (default), don't write it to lockfile settings. This avoids adding a new key to every lockfile. * test: simplify dedupePeers test assertions * test: check exact snapshot keys in dedupePeers integration test * test: add workspace test for dedupePeers with different peer versions * fix: keep transitive peers in suffix with version-only IDs Instead of pruning transitive peers entirely (which prevented per-project differentiation), keep them but use version-only identifiers. This way: - Packages like abc-grand-parent still get a peer suffix when different projects provide different peer versions (correct per-project isolation) - But the suffixes use name@version instead of full dep paths, eliminating the nested parentheses that cause combinatorial explosion * refactor: extract peerNodeIdToPeerId helper in resolvePeers * refactor: simplify peerNodeIdToPeerId return * fix: pin peer-a dist tag in dedupePeers tests for CI stability * fix: address review comments - Register dedupe-peers in config schema, types, and defaults so .npmrc/pnpm-workspace.yaml settings are parsed correctly - Use Boolean() comparison in settings checker so enabling dedupePeers on a pre-existing lockfile triggers re-resolution - Fix changeset text and test names: transitive peers are still propagated, just with version-only IDs (no nested dep paths)
* fix: ensure patches are applied during pnpm fetch * test: add coverage for patch file resolution during pnpm fetch fallback * fix(test): remove invalid pnpm property in fetch tests * fix: resolve lint errors in fetch test
…ing cleanupUnusedCatalogs (#11075) * fix(workspace): treat catalog refs in workspace overrides as used during cleanupUnusedCatalogs * fix: update * fix: update
* fix: handle non-native Error throws in requirePnpmfile When a pnpmfile throws a non-native Error value (e.g. a string), `assert(util.types.isNativeError(err))` crashes pnpm with an unhelpful assertion failure. Replace the assertion with a guard that wraps non-native errors into a proper Error and reports them via PnpmFileFailError. * fix: improve non-native error wrapping with toError helper
* perf(cafs): optimize hot path string operations
Replace path.join with string concatenation in contentPathFromHex and
getFilePathByModeInCafs. These functions are called ~30k times per
install and the simpler string operations avoid path.join's argument
validation overhead.
Increase gunzipSync chunk size from default 16KB to 128KB for faster
tarball decompression with fewer zlib iterations.
* refactor: remove dead Buffer.isBuffer check in tarball path
tarballBuffer is typed as Buffer, so the isBuffer/Buffer.from
fallback was unreachable dead code.
* docs: add comments explaining path.join bypass and chunkSize choice
Address review feedback:
- Explain why string concat is used instead of path.join in CAS hot path
- Document why 128KB chunkSize was chosen (microbenchmarks, diminishing
returns at larger sizes, bounded memory cost)
* fix: cspell — use 'Benchmarks' instead of 'Microbenchmarks'
* fix(cafs): restore Buffer.isBuffer check for worker thread compatibility
The structured clone algorithm converts Buffer to Uint8Array when sent
via postMessage to worker threads. parseTarball relies on
Buffer.prototype.toString('utf8', ...) which doesn't exist on
Uint8Array — Uint8Array.toString() returns comma-separated decimal
values, causing parseOctal to misparse tar headers.
#11085) * fix(cafs): update locker cache when file exists with correct integrity The CAS locker cache was not updated when a file already existed on disk with correct integrity. This caused repeated verifyFileIntegrity calls on subsequent lookups within the same process, adding unnecessary I/O. * fix(test): assert locker cache value not just key existence Strengthen the test to verify locker.get() returns the correct checkedAt timestamp, not just that the key exists.
…overy (#11087) ## Problem Every file extracted to the CAS goes through a temp-file-plus-rename cycle: `writeFile(temp, buffer)` then `renameOverwriteSync(temp, fileDest)`. For a typical cold install with ~30k files, this adds ~30k extra rename syscalls. ## Solution Use `writeFileExclusive()` with `{ flag: 'wx' }` (O_CREAT|O_EXCL) to write directly to the final CAS path when the file doesn't exist — skipping the temp+rename overhead. For recovery paths (corrupt/partial files, EEXIST races), fall back to the existing atomic temp+rename via `optimisticRenameOverwrite`. ### Write paths - **File doesn't exist (common cold-install path)** → `writeFileExclusive` writes directly, no rename - **File exists with correct integrity** → return immediately, no write - **File exists with wrong integrity (corruption/crash)** → atomic temp+rename recovery - **EEXIST (concurrent write)** → verify integrity; if OK return, otherwise atomic temp+rename recovery ### Concurrent safety - `writeFileExclusive` (`O_CREAT|O_EXCL`) ensures only one process creates a given CAS file - Recovery overwrites use the battle-tested `optimisticRenameOverwrite` + `pathTemp` for atomic replacement - `verifyFileIntegrity` is non-destructive (no `unlinkSync` on mismatch), safe when another process may be mid-write - A crash mid-`writeFileExclusive` can leave a partial file, recovered on next access via atomic temp+rename --------- Co-authored-by: Zoltan Kochan <z@kochan.io>
* feat: load default trusted deps list from @pnpm/plugin-trusted-deps Add a new `use-default-trusted-deps` setting (default: true) that automatically loads a curated list of known-good packages into `allowBuilds` from @pnpm/plugin-trusted-deps. User-configured allowBuilds entries take precedence over the defaults. Set `use-default-trusted-deps=false` to disable. * fix: use catalog reference for @pnpm/plugin-trusted-deps * fix: use default import for @pnpm/plugin-trusted-deps CJS compat The package uses Object.defineProperty for DEFAULT_ALLOW_BUILDS, which Node.js/Jest ESM interop can't detect as a named export. Switch to a default import to fix test failures. * fix: use named ESM import from @pnpm/plugin-trusted-deps@0.3.0-1 The package now ships an ESM entry point with proper named exports, so we can use a clean named import instead of the default import workaround. * fix: update @pnpm/plugin-trusted-deps to 0.3.0-2 Uses static JSON import attributes in ESM entry, fixing the bundle issue where createRequire resolved paths relative to the bundle output instead of the original package. * refactor: rename setting to allow-builds-for-trusted-deps * test: disable default trusted deps in approveBuilds tests The tests assert exact allowBuilds contents, so the default trusted list must be disabled to avoid polluting the expected values. * fix: don't persist default trusted deps list to pnpm-workspace.yaml Track the user's original allowBuilds separately as userAllowBuilds before merging the default trusted list. Use userAllowBuilds when writing back to pnpm-workspace.yaml to avoid persisting the ~370 default entries from @pnpm/plugin-trusted-deps. * refactor: rename setting to allow-builds-of-trusted-deps * docs: use camelCase for setting name in changeset * fix: include userAllowBuilds in install command opts types Without this, userAllowBuilds wasn't passed through to handleIgnoredBuilds, causing the default trusted list to be written to pnpm-workspace.yaml during e2e tests. * fix: set userAllowBuilds to empty object when user has no config When the user has no allowBuilds configured, userAllowBuilds was undefined, causing handleIgnoredBuilds to fall back to the merged allowBuilds (with defaults). Use empty object instead so the fallback doesn't trigger. * fix: read allowBuilds from workspace manifest when writing back Instead of tracking userAllowBuilds separately (which gets stale when other code writes to pnpm-workspace.yaml mid-install), read the current allowBuilds directly from pnpm-workspace.yaml before writing. This avoids persisting the default trusted list and preserves entries written by --allow-build earlier in the flow. Also update e2e test expectation: esbuild is now in the default trusted list, so it builds instead of being ignored. * chore: update tsconfig references for new dependencies * test: disable default trusted deps in approveBuilds e2e install The execPnpmInstall helper runs the bundled CLI which picks up the default allowBuildsOfTrustedDeps=true. This causes extra placeholder entries in pnpm-workspace.yaml that break assertions. * fix: revert approveBuilds to use config-based allowBuilds approveBuilds.handler should use opts.allowBuilds from getConfig() (which excludes trusted deps defaults when disabled) rather than reading the workspace manifest. The handler's job is to write approve/deny decisions, not merge with auto-populated placeholders. * test: add config reader tests for allowBuildsOfTrustedDeps Cover: (1) default enabled with trusted defaults merged, (2) user allowBuilds overrides defaults, (3) setting allow-builds-of-trusted-deps=false disables the merge.
…letion marker (#11088) ## Problem The indexed package importer always creates a staging temp directory, imports files there, then renames to the final location. For cold installs where the target doesn't exist (the common case), the staging + rename is unnecessary overhead. ## Solution - **Fast path**: callers already verify the target package is missing before calling `importIndexedDir`, so we can write directly into the final directory and skip the temp dir + rename. Falls back to the atomic staging path on EEXIST (concurrent import race) or when `keepModulesDir` is set (hoisted linker needs to merge existing `node_modules`). - **Completion marker**: `package.json` is written last by `tryImportIndexedDir`, so `pkgExistsAtTargetDir()` (which checks for `package.json`) won't consider a partially-imported directory as complete after a crash. - **Atomic copy**: the copy import path (non-COW filesystems) uses a temp file + `renameOverwriteSync` for the `package.json` write, since `copyFileSync` is not atomic. Hard links and reflinks are inherently atomic. This is expressed via the `Importer` interface (`importFile` + `importFileAtomic`), passed as the first argument to `importIndexedDir`. - **Synthetic package.json**: packages that lack a `package.json` (e.g. injected Bit workspace packages) now get a synthetic empty `{}` added to the store, so the completion marker works universally. - **DRY**: extracted `retryWithSanitizedFilenames()` to deduplicate the ENOENT handler used by both the fast path and staging path.
On Windows, npm's .cmd/.ps1 bin shims reference the extensionless
`pnpm` file from the published package.json bin entry. Previously,
setup.js and linkExePlatformBinary wrote a dummy text file ("This file
intentionally left blank") at that path, causing the shim to silently
fail — PowerShell's $LASTEXITCODE stays $null, so `exit $LASTEXITCODE`
exits with code 0, making all pnpm commands appear to succeed while
doing nothing.
Fix by hardlinking the real platform binary as both `pnpm.exe` and
`pnpm` (no extension), so the shim executes the actual binary.
…uth (#11089) * fix(auth-header): decode _password from base64 for default registry auth * refactor: extract basicAuth helper to deduplicate password decoding --------- Co-authored-by: Zoltan Kochan <z@kochan.io>
Extract the LOGIN_FAILED error into a dedicated ClassicLoginError class with httpStatus and responseText properties, matching the WebLoginError pattern. https://claude.ai/code/session_0191GhgPWiD5TroLMoXAmkaZ
Merged
…ected calls Replace no-op defaults with throwing mocks in createOtpMockContext and createMockContext. Tests that expect these to be called now explicitly override them. https://claude.ai/code/session_0191GhgPWiD5TroLMoXAmkaZ
Replace toHaveLength + indexed toContain pairs with single toEqual([expect.stringContaining(...)]) assertions. https://claude.ai/code/session_0191GhgPWiD5TroLMoXAmkaZ
For error tests: remove globalInfo override entirely, letting the default throwing mock catch unexpected calls. For success tests: use jest.fn() and assert globalInfo was called with the expected arguments. https://claude.ai/code/session_0191GhgPWiD5TroLMoXAmkaZ
Replace infoMessages/warnings arrays and push callbacks with jest.fn() and assertions on .mock.calls. This is more idiomatic and eliminates the boilerplate array + push pattern. https://claude.ai/code/session_0191GhgPWiD5TroLMoXAmkaZ
jest is not a global in ESM mode (--experimental-vm-modules).
Add import { jest } from '@jest/globals' to all test files using
jest.fn(), and add @jest/globals devDependency to network/web-auth.
https://claude.ai/code/session_0191GhgPWiD5TroLMoXAmkaZ
The test triggers web login (which calls globalInfo with the QR code) before reaching readIniFile. Without a globalInfo override, the default throwing mock causes the test to fail at the wrong point. https://claude.ai/code/session_0191GhgPWiD5TroLMoXAmkaZ
Extract inline jest.fn() to const and assert it was called with the web login QR code URL. https://claude.ai/code/session_0191GhgPWiD5TroLMoXAmkaZ
Per the style guide: "Functions should have no more than two or three
arguments. If a function needs more parameters, use a single options
object instead."
- withOtpHandling(operation, context, fetchOptions) → withOtpHandling({ operation, context, fetchOptions })
- pollForWebAuthToken(doneUrl, context, fetchOptions, timeoutMs) → pollForWebAuthToken({ doneUrl, context, fetchOptions, timeoutMs })
- webLogin(registry, fetchOptions, context) → webLogin({ registry, fetchOptions, context })
- classicLogin(registry, context, fetchOptions) → classicLogin({ registry, context, fetchOptions })
https://claude.ai/code/session_0191GhgPWiD5TroLMoXAmkaZ
Sort interface properties, function signature destructuring, and call site arguments in alphabetical order to match the convention used by publishWithOtpHandling. https://claude.ai/code/session_0191GhgPWiD5TroLMoXAmkaZ
- Build context and opts as separate variables, then call login/ withOtpHandling/pollForWebAuthToken on a clean line - Add createMockContext to login.test.ts - Convert createMockContext to arrow functions (single return expression), keep createMockResponse as function declaration (has local state) https://claude.ai/code/session_0191GhgPWiD5TroLMoXAmkaZ
When otp is undefined (first attempt before OTP challenge), the header 'npm-otp': undefined could be coerced to the string "undefined" by some HTTP implementations. Use conditional spread instead. https://claude.ai/code/session_0191GhgPWiD5TroLMoXAmkaZ
path.join produces backslashes on Windows, so hardcoded forward-slash paths in assertions fail on Windows CI. https://claude.ai/code/session_0191GhgPWiD5TroLMoXAmkaZ
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.
https://claude.ai/code/session_0191GhgPWiD5TroLMoXAmkaZ