Skip to content

fix: inconsistent resolution of a peer shared through a diamond#12081

Merged
zkochan merged 3 commits into
mainfrom
fix/12079
May 31, 2026
Merged

fix: inconsistent resolution of a peer shared through a diamond#12081
zkochan merged 3 commits into
mainfrom
fix/12079

Conversation

@zkochan

@zkochan zkochan commented May 30, 2026

Copy link
Copy Markdown
Member

Summary

Fixes #12079 — a 9.9.0 regression where a peer dependency shared through a "diamond" resolved inconsistently, crashing xo at runtime (TypeError: tsutils.unionConstituents is not a function).

@typescript-eslint/eslint-plugin peer-depends on both @typescript-eslint/parser and typescript, and @typescript-eslint/parser itself peer-depends typescript. All three must agree on a single typescript. With typescript@6.0.3 at the root and typescript@5.9.3 nested under xo, the resolver gave eslint-plugin a typescript@5.9.3 but a parser bound to typescript@6.0.3:

'@typescript-eslint/eslint-plugin@8.60.0(@typescript-eslint/parser@8.60.0(...)(typescript@6.0.3))(...)(typescript@5.9.3)'

Root cause

#8457 (the OOM fix for #8370) made the peer-merge reuse a hoisted instance of a peer-dep package for nested subtrees whenever name+version match, instead of re-resolving the node's own nested child. That reuse is correct for the non-diamond cases it targeted, but when a sibling peer-depends both the package and one of that package's own peers, reusing the hoisted instance silently pulls in the wrong transitive peer version.

The fix

The merge keeps the #8457 reuse except when doing so would break a peer-diamond — i.e. when a sibling peer-depends both the package and one of its peers that resolves differently in the inherited vs. local context. Then it falls back to the node's own child (inheritedParentPkgBreaksPeerDiamond in resolvePeers.ts).

This is the discriminator that separates #12079 from the cases #8457 must keep collapsing (e.g. the repeat-peers "hoisted" test), where no node peer-depends both — so those are untouched.

Performance

No regression — and not slower:

  • The new check runs only in the already-matched collapse branch and short-circuits cheaply; the child-scan only runs when a real conflict exists (rare).
  • It does not re-introduce the Pnpm install hangs indefinitely #8370 blowup: duplication happens only for genuine diamond conflicts, not for every shadowed peer.
  • The xo repro lockfile went from 439 → 438 snapshots (slightly fewer, not more).

Validation

  • Issue's exact repro now yields "5.9.3" / "5.9.3" (was 5.9.3 / 6.0.3).
  • New unit test in test/resolvePeers.ts modeling the diamond — verified it fails without the fix.
  • All existing peer suites pass: resolvePeers unit (94 incl. the new one), peerDependencies (67), autoInstallPeers / defaultPeerDependencies / cyclicPeerDeterminism (26).

pacquet parity

pacquet already resolves this diamond correctly — its resolve_peers.rs uses bump_occurrence_on_shadow, which always prefers the node's own child over an inherited same-version instance (it never ported #8457's collapse), so it never reuses a root-level ts@2.0.0 parser for a nested plugin. No pacquet code change is needed.

To lock that in, this PR adds matching coverage on both stacks using shared @pnpm.e2e/peer-diamond-* fixtures:

  • pnpm: installing/deps-installer/test/install/peerDependencies.ts (plus the focused unit test in installing/deps-resolver/test/resolvePeers.ts).
  • pacquet: pacquet/crates/cli/tests/install.rs::peer_shared_through_a_diamond_is_resolved_consistently.

Both tests were verified to fail when their respective merge logic is broken, so they genuinely guard the behavior.


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

Summary by CodeRabbit

  • Bug Fixes

    • Fixed inconsistent peer dependency resolution in “diamond” dependency graphs so packages no longer get paired with incompatible hoisted peer instances; installations and lockfiles are now consistent.
  • Tests

    • Added unit and end-to-end tests covering diamond peer scenarios to prevent regressions and ensure correct peer pairing.

Review Change Stack

When a package peer-depends both another package and one of that
package's own peer dependencies (e.g. @typescript-eslint/eslint-plugin
peer-depends both @typescript-eslint/parser and typescript, and
@typescript-eslint/parser peer-depends typescript), pnpm reused a
hoisted instance of the shared peer that was resolved against a
different version, producing an inconsistent resolution.

Close #12079
@coderabbitai

coderabbitai Bot commented May 30, 2026

Copy link
Copy Markdown

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: 8cd09501-b564-4105-a0b0-3caed107f359

📥 Commits

Reviewing files that changed from the base of the PR and between d6c32de and 070e2b8.

📒 Files selected for processing (1)
  • .changeset/twelve-peers-diamond.md
✅ Files skipped from review due to trivial changes (1)
  • .changeset/twelve-peers-diamond.md
📜 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). (9)
  • GitHub Check: Run benchmark on ubuntu-latest
  • GitHub Check: Lint and Test (windows-latest)
  • GitHub Check: Lint and Test (ubuntu-latest)
  • GitHub Check: Lint and Test (macos-latest)
  • GitHub Check: Dylint
  • GitHub Check: Run benchmark on ubuntu-latest
  • GitHub Check: Analyze (javascript)
  • GitHub Check: Code Coverage
  • GitHub Check: Compile & Lint

📝 Walkthrough

Walkthrough

This PR fixes a regression in pnpm 9.9.0 where peer dependencies were resolved inconsistently across "diamond" dependency graphs (multiple packages sharing the same peer). The fix adds conflict detection to prevent reusing inherited parent packages when doing so would cause sibling peer dependencies to resolve against different versions, plus comprehensive test coverage.

Changes

Peer Diamond Conflict Detection and Testing

Layer / File(s) Summary
Peer diamond conflict detection logic
installing/deps-resolver/src/resolvePeers.ts
inheritedParentPkgBreaksPeerDiamond and parentPeerDiffers detect when reusing an inherited parent package causes inconsistent peer resolution; the check is integrated into _parentPkgsMatch to prevent reuse when a conflict is detected.
Diamond peer dependency test packages
pnpr/.fixtures/packages/@pnpm.e2e/peer-diamond-{ts,parser,plugin,bundle,app}/*
Test fixture packages defining an interconnected peer dependency graph: peer-diamond-ts (root peer at v1.0.0 and v2.0.0), peer-diamond-parser (peer-depends on ts), peer-diamond-plugin (peer-depends on parser and ts), peer-diamond-bundle (depends on plugin and parser, peer-depends on ts), and peer-diamond-app (depends on ts and bundle).
Unit, integration, and e2e test coverage
installing/deps-resolver/test/resolvePeers.ts, installing/deps-installer/test/install/peerDependencies.ts, pacquet/crates/cli/tests/install.rs
Unit test in deps-resolver verifies correct typescript@1.0.0 resolution while rejecting incorrect typescript@2.0.0 reuse; integration test checks lockfile snapshot consistency; e2e test validates full install flow produces correct peer pairing in pnpm-lock.yaml.
Release notes
.changeset/twelve-peers-diamond.md
Documents patch version bumps for @pnpm/installing.deps-resolver and pnpm explaining the fix for inconsistent peer dependency resolution in diamond graphs.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • pnpm/pnpm#11906: Also modifies peer-resolution behavior and peers cache validation to avoid incorrect reuse across differing parent peer contexts.

Poem

🐰 A diamond peer's no longer bent,
When siblings split what should be spent,
With conflict checks both old and new,
Each resolver finds the matching view,
No more hoisting breaks the chain!

🚥 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 core fix: resolving inconsistent peer-dependency versions in diamond dependency graphs.
Linked Issues check ✅ Passed All objectives from #12079 are addressed: peer-diamond regression fixed, test coverage added (unit tests in resolvePeers.ts, e2e tests in install.rs), performance optimizations maintained.
Out of Scope Changes check ✅ Passed All changes are directly related to fixing the peer-diamond resolution issue. Test fixtures, test cases, and resolution logic changes are all in scope.
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 unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/12079

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.

Add @pnpm.e2e/peer-diamond-* fixtures modeling #12079 (plugin
peer-depends both parser and ts; parser peer-depends ts) and
integration tests on both stacks. The pnpm test guards the fix; the
pacquet test confirms pacquet already resolves the diamond consistently
(its merge always prefers the node's own child).
@github-actions

github-actions Bot commented May 30, 2026

Copy link
Copy Markdown
Contributor

Micro-Benchmark Results

Linux

group                          main                                   pr
-----                          ----                                   --
tarball/download_dependency    1.04      8.0±0.31ms   541.2 KB/sec    1.00      7.7±0.28ms   561.0 KB/sec

@codecov-commenter

codecov-commenter commented May 30, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.17%. Comparing base (c5d9d3a) to head (070e2b8).
⚠️ Report is 6 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #12081      +/-   ##
==========================================
- Coverage   89.38%   87.17%   -2.21%     
==========================================
  Files         243      243              
  Lines       32536    26798    -5738     
==========================================
- Hits        29082    23362    -5720     
+ Misses       3454     3436      -18     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@zkochan zkochan marked this pull request as ready for review May 30, 2026 21:15
@qodo-free-for-open-source-projects

Copy link
Copy Markdown

Review Summary by Qodo

Fix inconsistent resolution of peer dependencies shared through a diamond

🐞 Bug fix

Grey Divider

Walkthroughs

Description
• Fixes inconsistent peer resolution in diamond dependency patterns
• Prevents reusing hoisted peer instances when they break peer-diamond consistency
• Adds inheritedParentPkgBreaksPeerDiamond check to detect and handle conflicts
• Includes comprehensive test coverage for both pnpm and pacquet implementations
Diagram
flowchart LR
  A["Plugin peer-depends<br/>Parser + TypeScript"] -->|Diamond conflict| B["Inherited Parser<br/>resolved vs ts@2.0.0"]
  A -->|Must use| C["Own child Parser<br/>resolved vs ts@1.0.0"]
  B -->|Breaks consistency| D["Inconsistent versions"]
  C -->|Maintains consistency| E["All peers agree<br/>on ts@1.0.0"]
  F["New check:<br/>inheritedParentPkgBreaksPeerDiamond"] -->|Detects| B
  F -->|Falls back to| C

Loading

Grey Divider

File Changes

1. installing/deps-resolver/src/resolvePeers.ts 🐞 Bug fix +66/-1

Add peer-diamond conflict detection logic

installing/deps-resolver/src/resolvePeers.ts


2. installing/deps-resolver/test/resolvePeers.ts 🧪 Tests +126/-0

Add unit test for peer-diamond resolution

installing/deps-resolver/test/resolvePeers.ts


3. installing/deps-installer/test/install/peerDependencies.ts 🧪 Tests +20/-0

Add integration test for peer-diamond consistency

installing/deps-installer/test/install/peerDependencies.ts


View more (8)
4. pacquet/crates/cli/tests/install.rs 🧪 Tests +47/-0

Add pacquet test for peer-diamond resolution

pacquet/crates/cli/tests/install.rs


5. .changeset/twelve-peers-diamond.md 📝 Documentation +6/-0

Document peer-diamond resolution fix

.changeset/twelve-peers-diamond.md


6. pnpr/.fixtures/packages/@pnpm.e2e/peer-diamond-app/1.0.0/package.json 🧪 Tests +8/-0

Add peer-diamond-app test fixture

pnpr/.fixtures/packages/@pnpm.e2e/peer-diamond-app/1.0.0/package.json


7. pnpr/.fixtures/packages/@pnpm.e2e/peer-diamond-bundle/1.0.0/package.json 🧪 Tests +11/-0

Add peer-diamond-bundle test fixture

pnpr/.fixtures/packages/@pnpm.e2e/peer-diamond-bundle/1.0.0/package.json


8. pnpr/.fixtures/packages/@pnpm.e2e/peer-diamond-parser/1.0.0/package.json 🧪 Tests +7/-0

Add peer-diamond-parser test fixture

pnpr/.fixtures/packages/@pnpm.e2e/peer-diamond-parser/1.0.0/package.json


9. pnpr/.fixtures/packages/@pnpm.e2e/peer-diamond-plugin/1.0.0/package.json 🧪 Tests +8/-0

Add peer-diamond-plugin test fixture

pnpr/.fixtures/packages/@pnpm.e2e/peer-diamond-plugin/1.0.0/package.json


10. pnpr/.fixtures/packages/@pnpm.e2e/peer-diamond-ts/1.0.0/package.json 🧪 Tests +4/-0

Add peer-diamond-ts v1.0.0 test fixture

pnpr/.fixtures/packages/@pnpm.e2e/peer-diamond-ts/1.0.0/package.json


11. pnpr/.fixtures/packages/@pnpm.e2e/peer-diamond-ts/2.0.0/package.json 🧪 Tests +4/-0

Add peer-diamond-ts v2.0.0 test fixture

pnpr/.fixtures/packages/@pnpm.e2e/peer-diamond-ts/2.0.0/package.json


Grey Divider

Qodo Logo

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.changeset/twelve-peers-diamond.md:
- Line 6: The changelog sentence is missing the preposition "on"; update the
sentence that reads "peer-depends both another package" to "peer-depends on both
another package" so the example line referencing
`@typescript-eslint/eslint-plugin`, `@typescript-eslint/parser` and typescript reads
correctly; locate the sentence in the change description about inconsistent
resolution of a peer dependency shared through a diamond and insert "on" after
"peer-depends".
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 0122a4f4-1048-4ece-9d18-656aec5bb3a1

📥 Commits

Reviewing files that changed from the base of the PR and between 394ee27 and d6c32de.

📒 Files selected for processing (11)
  • .changeset/twelve-peers-diamond.md
  • installing/deps-installer/test/install/peerDependencies.ts
  • installing/deps-resolver/src/resolvePeers.ts
  • installing/deps-resolver/test/resolvePeers.ts
  • pacquet/crates/cli/tests/install.rs
  • pnpr/.fixtures/packages/@pnpm.e2e/peer-diamond-app/1.0.0/package.json
  • pnpr/.fixtures/packages/@pnpm.e2e/peer-diamond-bundle/1.0.0/package.json
  • pnpr/.fixtures/packages/@pnpm.e2e/peer-diamond-parser/1.0.0/package.json
  • pnpr/.fixtures/packages/@pnpm.e2e/peer-diamond-plugin/1.0.0/package.json
  • pnpr/.fixtures/packages/@pnpm.e2e/peer-diamond-ts/1.0.0/package.json
  • pnpr/.fixtures/packages/@pnpm.e2e/peer-diamond-ts/2.0.0/package.json
📜 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). (2)
  • GitHub Check: ubuntu-latest / Node.js 24 / Test
  • GitHub Check: Run benchmark on ubuntu-latest
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ts,tsx}: Follow Standard Style with trailing commas, preferring functions over classes, and declaring functions after they are used (relying on hoisting)
Use a single options object instead of multiple parameters when a function needs more than two or three arguments
Follow Import Order: Standard libraries first, then external dependencies (alphabetically), then relative imports
Write self-documenting code where function names, parameters, and types explain what a function does without requiring prose comments
Do not write comments that restate what the code already says; refactor via renaming, splitting helpers, or restructuring instead
Do not repeat documentation at call sites that already exists in JSDoc on the callee; update JSDoc once for all call sites to benefit
Use JSDoc only for a function's contract (preconditions, postconditions, edge cases, why the function exists), not for re-narrating the body
Do not record past implementation shape, refactor history, or 'the previous code did X' framing in code; use git log and git blame instead
Write comments only when: the reason for code is non-obvious (hidden invariant, workaround for known bug, deliberate exception), or the right name doesn't fit (temporary technical constraint)

Files:

  • installing/deps-resolver/test/resolvePeers.ts
  • installing/deps-installer/test/install/peerDependencies.ts
  • installing/deps-resolver/src/resolvePeers.ts
pacquet/**/*.rs

📄 CodeRabbit inference engine (pacquet/AGENTS.md)

pacquet/**/*.rs: Log emissions are part of matching pnpm — when porting a function that fires pnpm:<channel> events through globalLogger, logger.debug(...), or streamParser.write(...), mirror the call site, payload, and ordering so @pnpm/cli.default-reporter parses pacquet's NDJSON the same way
Declare a newtype wrapper for branded string types instead of collapsing the brand into a plain String or &str in Rust
If upstream TypeScript always validates before construction of a branded string, validate in the Rust wrapper too via TryFrom<String> and/or FromStr and do not provide an infallible public constructor
If upstream TypeScript never validates a branded string, just brand for type-safety in Rust by exposing an infallible From<String> constructor
If upstream TypeScript occasionally constructs a branded string without validation, expose from_str_unchecked in Rust as an escape hatch alongside the validating constructor
Match upstream serde behavior for branded strings crossing JSON, YAML, or INI boundaries by using #[serde(try_from = "String")] for deserialization and #[serde(into = "String")] for serialization
Derive simple conversions for branded strings using #[derive(derive_more::From)] and #[derive(derive_more::Into)] instead of handwriting impl blocks; use manual impl only when conversion needs custom logic
Model TypeScript string literal unions (like 'auto' | 'always' | 'never') as Rust enums instead of newtype wrappers, since the set of valid values is closed
Treat TypeScript string template literal types (like `${string}@${string}`) the same as branded string types in Rust, using a newtype wrapper with validation
Follow the code style guide in CODE_STYLE_GUIDE.md — imports, modules, naming, ownership and borrowing, parameter type selection, trait bounds, pattern matching, pipe-trait, error handling, test layout, and cloning of Arc and Rc
Choose owned vs. borrowed parameters to minimize copies; widen to t...

Files:

  • pacquet/crates/cli/tests/install.rs
pacquet/**/tests/**/*.rs

📄 CodeRabbit inference engine (pacquet/AGENTS.md)

pacquet/**/tests/**/*.rs: Prefer real fixtures over dependency-injection seams — use tempfile::TempDir, the mocked registry, or integration tests spawning the actual binary for happy paths and most error paths; use the DI seam only for filesystem error kinds, deterministic time, shared process-global state, or external-service happy paths
Follow test-logging guidance — log before non-assert_eq! assertions, dbg! complex structures, skip logging for simple scalar assert_eq!
Tests must not be tolerant of missing build/runtime environment by silently returning early when a tool isn't found — let the test panic when a required tool is absent
Prefer #[cfg_attr(target_os = "windows", ignore = "...")] (or matching #[cfg(unix)] gates) over runtime probe-and-skip helpers for platform-locked tools
Port relevant pnpm tests to Rust tests whenever they translate when porting behavior from pnpm
Use snapshot tests with insta and carefully review diffs when intentional changes alter snapshots; accept with cargo insta review only after careful review
Tests that need the mocked registry should start pnpr through pacquet-testing-utils; cargo test / cargo nextest run should not require a separate just registry-mock launch step

Files:

  • pacquet/crates/cli/tests/install.rs
🧠 Learnings (5)
📚 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/twelve-peers-diamond.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:

  • installing/deps-resolver/test/resolvePeers.ts
  • installing/deps-installer/test/install/peerDependencies.ts
  • installing/deps-resolver/src/resolvePeers.ts
📚 Learning: 2026-05-20T19:40:55.051Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11774
File: pacquet/crates/resolving-deps-resolver/src/resolve_peers.rs:0-0
Timestamp: 2026-05-20T19:40:55.051Z
Learning: In the pacquet Rust code, ensure the semver implementation uses the `node-semver` crate (not `nodejs-semver`). `node-semver`’s public API does not include a `satisfies_with_prerelease`-style method; prerelease-tolerant matching should be implemented inline by first calling `Range::satisfies`, and when it rejects a prerelease version, retry matching against a stripped `MAJOR.MINOR.PATCH` base of the prerelease version.

Applied to files:

  • pacquet/crates/cli/tests/install.rs
📚 Learning: 2026-05-22T00:08:44.646Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11837
File: pacquet/crates/resolving-npm-resolver/src/pick_package.rs:33-51
Timestamp: 2026-05-22T00:08:44.646Z
Learning: In the pnpm/pnpm repo’s pacquet Rust crates, do not flag Unicode ellipsis characters (U+2026, `…`) in Rust doc comments (`///` / `/** */`) as a lint violation. The pacquet crate’s `dylint.toml` only enables `perfectionist::derive_ordering`, and the Dylint `unicode-ellipsis` rule is not enabled for this project—so `…` in doc comments is an intentional, repo-consistent style.

Applied to files:

  • pacquet/crates/cli/tests/install.rs
📚 Learning: 2026-05-20T23:07:58.444Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11784
File: pacquet/crates/resolving-deps-resolver/src/hoist_peers.rs:120-133
Timestamp: 2026-05-20T23:07:58.444Z
Learning: When reviewing code in this pacquet Rust port, follow the upstream pnpm compatibility rule: only match pnpm’s behavior exactly. Do not propose review changes that intentionally deviate from pnpm’s documented/observed behavior, even if pnpm appears buggy. If you identify a real bug in pnpm behavior, the review should prioritize fixing it upstream in pnpm first, and avoid implementing a pnpm-behavior workaround here unless the same fix has already landed upstream.

Applied to files:

  • pacquet/crates/cli/tests/install.rs
🔇 Additional comments (10)
installing/deps-resolver/src/resolvePeers.ts (1)

432-435: LGTM!

Also applies to: 670-730

pnpr/.fixtures/packages/@pnpm.e2e/peer-diamond-ts/2.0.0/package.json (1)

1-4: LGTM!

installing/deps-resolver/test/resolvePeers.ts (1)

928-1053: LGTM!

installing/deps-installer/test/install/peerDependencies.ts (1)

2047-2065: LGTM!

pacquet/crates/cli/tests/install.rs (1)

368-413: LGTM!

pnpr/.fixtures/packages/@pnpm.e2e/peer-diamond-ts/1.0.0/package.json (1)

1-4: LGTM!

pnpr/.fixtures/packages/@pnpm.e2e/peer-diamond-parser/1.0.0/package.json (1)

1-7: LGTM!

pnpr/.fixtures/packages/@pnpm.e2e/peer-diamond-plugin/1.0.0/package.json (1)

1-8: LGTM!

pnpr/.fixtures/packages/@pnpm.e2e/peer-diamond-bundle/1.0.0/package.json (1)

1-11: LGTM!

pnpr/.fixtures/packages/@pnpm.e2e/peer-diamond-app/1.0.0/package.json (1)

1-8: LGTM!

Comment thread .changeset/twelve-peers-diamond.md Outdated
@github-actions

github-actions Bot commented May 30, 2026

Copy link
Copy Markdown
Contributor

Integrated-Benchmark Report (Linux)

Scenario: Isolated linker: fresh restore, cold cache + cold store

Command Mean [s] Min [s] Max [s] Relative
pacquet@HEAD 2.065 ± 0.110 1.946 2.288 1.02 ± 0.06
pacquet@main 2.017 ± 0.068 1.947 2.189 1.00
BENCHMARK_REPORT.json
{
  "results": [
    {
      "command": "pacquet@HEAD",
      "mean": 2.0648381463800005,
      "stddev": 0.11038590910228807,
      "median": 2.0494464322800003,
      "user": 2.6838009,
      "system": 3.2008603399999997,
      "min": 1.94576954878,
      "max": 2.2879749047800004,
      "times": [
        2.2879749047800004,
        1.9786638357800002,
        1.94576954878,
        2.0945266677800003,
        1.97243364978,
        1.96811799678,
        2.1008408337800004,
        2.12829483978,
        2.0043661967800004,
        2.16739298978
      ]
    },
    {
      "command": "pacquet@main",
      "mean": 2.01691803478,
      "stddev": 0.06777935514243033,
      "median": 2.00378060178,
      "user": 2.6897699,
      "system": 3.2384550400000003,
      "min": 1.94710736578,
      "max": 2.18900379478,
      "times": [
        1.96157096778,
        2.18900379478,
        1.94710736578,
        2.04608123678,
        2.0336259617800003,
        1.97330928178,
        2.01401708278,
        2.0060695867800002,
        1.99690345278,
        2.00149161678
      ]
    }
  ]
}

Scenario: Isolated linker: fresh restore, hot cache + hot store

Command Mean [ms] Min [ms] Max [ms] Relative
pacquet@HEAD 667.8 ± 62.7 636.3 841.0 1.01 ± 0.10
pacquet@main 661.3 ± 28.0 636.5 725.6 1.00
BENCHMARK_REPORT.json
{
  "results": [
    {
      "command": "pacquet@HEAD",
      "mean": 0.6677722531600001,
      "stddev": 0.06268988416773781,
      "median": 0.64426065206,
      "user": 0.34319562,
      "system": 1.3306007599999998,
      "min": 0.6362986945600001,
      "max": 0.8409558275600001,
      "times": [
        0.8409558275600001,
        0.6516426515600001,
        0.6888305875600002,
        0.6396897865600001,
        0.6510192615600001,
        0.6404474575600001,
        0.6422450395600001,
        0.6462762645600001,
        0.6403169605600001,
        0.6362986945600001
      ]
    },
    {
      "command": "pacquet@main",
      "mean": 0.6612753597600001,
      "stddev": 0.027964435873740687,
      "median": 0.65222701506,
      "user": 0.35235832,
      "system": 1.33102346,
      "min": 0.63648028456,
      "max": 0.72560473156,
      "times": [
        0.72560473156,
        0.6570223755600001,
        0.63648028456,
        0.65132939856,
        0.6531246315600001,
        0.64319687556,
        0.6609602375600001,
        0.6446149535600001,
        0.6443506145600001,
        0.6960694945600001
      ]
    }
  ]
}

Scenario: Isolated linker: fresh install, cold cache + cold store

Command Mean [s] Min [s] Max [s] Relative
pacquet@HEAD 2.302 ± 0.029 2.254 2.351 1.00
pacquet@main 2.310 ± 0.024 2.282 2.351 1.00 ± 0.02
BENCHMARK_REPORT.json
{
  "results": [
    {
      "command": "pacquet@HEAD",
      "mean": 2.30169645442,
      "stddev": 0.028855606261801763,
      "median": 2.2950089904200004,
      "user": 3.8276410199999993,
      "system": 3.0455469,
      "min": 2.25387737492,
      "max": 2.35117324392,
      "times": [
        2.28248104592,
        2.25387737492,
        2.2959538789200002,
        2.35117324392,
        2.28617203692,
        2.29376824492,
        2.34549050992,
        2.31022199692,
        2.29406410192,
        2.30376210992
      ]
    },
    {
      "command": "pacquet@main",
      "mean": 2.3103861282200002,
      "stddev": 0.02396499718740754,
      "median": 2.3041284934200004,
      "user": 3.834107019999999,
      "system": 3.0340455000000004,
      "min": 2.28235370592,
      "max": 2.35085001892,
      "times": [
        2.29362859192,
        2.33788122892,
        2.30223851592,
        2.30677293292,
        2.28235370592,
        2.30001790292,
        2.28468758192,
        2.33941233192,
        2.3060184709200002,
        2.35085001892
      ]
    }
  ]
}

Scenario: Isolated linker: fresh install, hot cache + hot store

Command Mean [s] Min [s] Max [s] Relative
pacquet@HEAD 1.496 ± 0.017 1.468 1.533 1.00
pacquet@main 1.539 ± 0.063 1.492 1.707 1.03 ± 0.04
BENCHMARK_REPORT.json
{
  "results": [
    {
      "command": "pacquet@HEAD",
      "mean": 1.4961440914000002,
      "stddev": 0.01718734726490081,
      "median": 1.4959880056000001,
      "user": 1.6675393800000002,
      "system": 1.8621595,
      "min": 1.4680171431000002,
      "max": 1.5329997951,
      "times": [
        1.4944029031000001,
        1.5329997951,
        1.5039386101,
        1.4793379091,
        1.4975731081,
        1.5059945151,
        1.4903190761,
        1.4680171431000002,
        1.4909608801,
        1.4978969741000001
      ]
    },
    {
      "command": "pacquet@main",
      "mean": 1.5385936371000002,
      "stddev": 0.06326749190731726,
      "median": 1.5240105951,
      "user": 1.68648558,
      "system": 1.8749169999999995,
      "min": 1.4922707001,
      "max": 1.7066958201,
      "times": [
        1.5034300081,
        1.5228569031,
        1.5251642871000002,
        1.7066958201,
        1.5261775681,
        1.5036075671000002,
        1.5743413051000001,
        1.5044783781000002,
        1.4922707001,
        1.5269138341000001
      ]
    }
  ]
}

@github-actions

github-actions Bot commented May 30, 2026

Copy link
Copy Markdown
Contributor

🐰 Bencher Report

Branchpr/12081
Testbedpacquet
Click to view all benchmark results
BenchmarkLatencyBenchmark Result
milliseconds (ms)
(Result Δ%)
Upper Boundary
milliseconds (ms)
(Limit %)
isolated-linker.fresh-install.cold-cache.cold-store📈 view plot
🚷 view threshold
2,301.70 ms
(-3.56%)Baseline: 2,386.59 ms
2,863.91 ms
(80.37%)
isolated-linker.fresh-install.hot-cache.hot-store📈 view plot
🚷 view threshold
1,496.14 ms
(-1.41%)Baseline: 1,517.54 ms
1,821.05 ms
(82.16%)
isolated-linker.fresh-restore.cold-cache.cold-store📈 view plot
🚷 view threshold
2,064.84 ms
(-2.42%)Baseline: 2,116.01 ms
2,539.21 ms
(81.32%)
isolated-linker.fresh-restore.hot-cache.hot-store📈 view plot
🚷 view threshold
667.77 ms
(-1.32%)Baseline: 676.72 ms
812.07 ms
(82.23%)
🐰 View full continuous benchmarking report in Bencher

@github-actions

Copy link
Copy Markdown
Contributor

Benchmark Results

# Scenario main HEAD
1 Isolated linker: fresh restore, hot cache + hot store 2.352s ± 0.101s 2.309s ± 0.062s
2 Isolated linker: fresh add new dep, hot cache + hot store 5.744s ± 0.084s 5.682s ± 0.042s
3 Isolated linker: fresh install, hot cache + hot store 5.836s ± 0.101s 5.795s ± 0.038s
4 Isolated linker: fresh restore, cold cache + cold store 4.785s ± 0.046s 4.735s ± 0.037s
5 Isolated linker: fresh install, cold cache + cold store 7.823s ± 0.067s 7.881s ± 0.095s
6 GVS linker: fresh restore, hot cache + hot store 1.111s ± 0.044s 1.105s ± 0.015s

Run 26697942305 · 10 runs per scenario · triggered by @zkochan

@zkochan zkochan merged commit 1db05c6 into main May 31, 2026
28 checks passed
@zkochan zkochan deleted the fix/12079 branch May 31, 2026 14:03
@andersk

andersk commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

This seems to narrowly match the specific problematic scenario of #12079 without addressing the broader algorithmic problem, and is easily fooled:

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.

[9.9.0 regression] Inconsistent resolution of typescript peer-dependency in typescript-eslint packages leads to crash

3 participants