Skip to content

feat: pnpm login (continuation)#11093

Closed
KSXGitHub wants to merge 114 commits into
claude/implement-pnpm-login-4VvtQfrom
claude/unarchive-session-feature-Sf7Xr
Closed

feat: pnpm login (continuation)#11093
KSXGitHub wants to merge 114 commits into
claude/implement-pnpm-login-4VvtQfrom
claude/unarchive-session-feature-Sf7Xr

Conversation

@KSXGitHub

@KSXGitHub KSXGitHub commented Mar 26, 2026

Copy link
Copy Markdown
Contributor

zkochan and others added 25 commits March 22, 2026 10:56
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
@KSXGitHub KSXGitHub changed the title feat: pnpm login (continuation) feat: pnpm login (new) Mar 26, 2026
@KSXGitHub KSXGitHub changed the title feat: pnpm login (new) feat: pnpm login (continuation) Mar 26, 2026
@KSXGitHub KSXGitHub mentioned this pull request Mar 26, 2026
KSXGitHub and others added 28 commits March 27, 2026 06:59
…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
@zkochan zkochan deleted the claude/unarchive-session-feature-Sf7Xr branch March 27, 2026 11:00
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.