fix(resolver): make shared-children missing-peer reuse independent of resolution order by davidbarratt · Pull Request #12514 · pnpm/pnpm

davidbarratt · 2026-06-19T02:54:58Z

Summary

The problem

Peer resolution is non-deterministic in one case: an optional transitive peer can be pulled into — or left out of — a package's resolved peer-dependency suffix depending on resolution-completion order, not on the dependency graph. The result is lockfile churn and intermittent pnpm dedupe --check failures in CI, even when running pnpm dedupe locally reports nothing to change.

A concrete, public instance of the shape:

next depends on styled-jsx, and styled-jsx declares @babel/core as an optional peer (@babel/core under peerDependenciesMeta as optional — verifiable via npm view styled-jsx peerDependenciesMeta).
When @babel/core is present elsewhere in a workspace (e.g. pulled by a Babel/Storybook/React-Native toolchain in another project), whether a given next occurrence resolves it as a transitive peer — i.e. whether its dep path becomes next@x(@babel/core@y)… or just next@x(…) — varies between otherwise identical installs.

Root cause

The non-determinism is in claimChildrenResolution. When a shared package's children are resolved by one occurrence (the "owner") and a deeper occurrence reuses them, the reuse condition was:

existing.owner.depth >= opts.currentDepth ||
existing.missingPeersOfChildren.resolved

The second clause keys the decision on whether the owner's missingPeersOfChildren promise has settled yet at the moment this node is claimed. Under concurrent resolution that settling time varies run to run, so a deeper consumer inherits the owner's missing peers on some runs and not others. That is exactly what flips an optional transitive peer like @babel/core in and out of the suffix.

The deterministic owner selection added in #12362 (via compareChildrenResolutionOwners) made which occurrence owns a shared package order-independent — but it did not neutralize this .resolved timing branch, which predates it.

Why the clause existed

The .resolved flag was introduced in #5467 (closing #5454, "don't crash when auto-install-peers is true in a workspace"). With auto-install-peers in a workspace, a shared package reached through multiple importers could end up awaiting its own not-yet-settled missingPeersOfChildren promise — a deadlock. The flag let the resolver reuse that promise only once it had settled, and otherwise re-resolve children instead of awaiting. #12362 later moved this guard verbatim into claimChildrenResolution without neutralizing its timing-dependence.

The fix

Drop the .resolved clause: reuse the owner's missing peers only when the owner is at the same or a deeper depth — a function of the dependency-graph structure rather than completion order. This is strictly more conservative than the original guard: a shallower owner's promise is now never reused (settled or not), so no unsettled promise is ever awaited and the #5454 deadlock cannot return. The #5454 regression test (installation on a workspace with many complex circular dependencies does not fail when auto install peers is on) still passes.

How to verify the analysis

The race is visible by reading claimChildrenResolution: the only run-to-run-variable input to the branch is existing.missingPeersOfChildren.resolved; everything else (owner.depth, currentDepth, parentIds) is structural.
.resolved is set when the owner's children finish resolving (see where it is assigned), so its value at claim time depends on concurrent ordering.
The existing peer/dedupe/cyclic suites still pass with the clause removed (resolvePeers, cyclicPeerDeterminism, peerDependencies, and the peers/hoist/dedupe/transitive/autoInstallPeers install tests), including the regressions from #12286 and #11999.

Testing note

I was not able to add a failing-then-passing unit test. The race only manifests under real concurrent network I/O; the mock-registry test harness serializes resolution through the depth barrier (waitForPackageResolutionTurn) and produces deterministic output regardless of response timing, so it does not reproduce the flake. The fix was validated against a large real workspace: before, dedupe --check flipped between clean and "changes" across cold runs; after, 30/30 cold runs produced a byte-identical result. Guidance on an acceptable deterministic test (e.g. a scheduler-injection seam) would be welcome.

Update: test coverage added in b19c3f5. The mock-registry harness still can't reproduce the flake, but the changed line is testable directly. A unit test on claimChildrenResolution asserts a deeper consumer never reuses a strictly shallower owner's missingPeersOfChildren even when that promise has already settled — the run-to-run-variable input the dropped clause keyed on. It fails with the old clause restored and passes after. pacquet gets the parity version of the same scenario (shared styled-jsx whose optional @babel/core peer is provided by a sibling at a shallow depth but absent deeper), asserted stable across 16 fresh-HashMap rebuilds.

Squash Commit Body

claimChildrenResolution let a non-owner occurrence of a shared package reuse the
owner's missingPeersOfChildren when `existing.owner.depth >= currentDepth ||
existing.missingPeersOfChildren.resolved`. The second clause made reuse depend on
whether the owner's resolution had settled by claim time. Under concurrent
resolution that timing varies run to run, so a deeper consumer inherited the
owner's missing peers on some runs but not others — flipping an optional
transitive peer (e.g. `@babel/core`, reached via styled-jsx) in and out of a
package's resolved peer suffix and churning the lockfile, with intermittent
`pnpm dedupe --check` failures in CI.

Drop the `.resolved` clause: reuse only when the owner is at the same or a deeper
depth, a function of the dependency graph rather than completion order.

The `.resolved` flag was introduced in pnpm/pnpm#5467 (closing pnpm/pnpm#5454) to
avoid a deadlock where, with auto-install-peers in a workspace, a shared package
awaited its own not-yet-settled missingPeersOfChildren promise. Removing the
clause is strictly more conservative — a shallower owner's promise is never
reused, settled or not, so no unsettled promise is ever awaited and the deadlock
cannot return. The pnpm/pnpm#5454 regression test still passes, as do the peer,
dedupe, and cyclic suites. The deterministic owner selection from pnpm/pnpm#12362
made shared-package ownership order-independent but had moved this timing branch
in verbatim without neutralizing it.

Checklist

The change is implemented in both the TypeScript CLI and the Rust pacquet/ port, or the description notes what still needs porting. — pnpm-only. pacquet needs no change: it resolves a non-owner's children lazily and per-occurrence during peer resolution (realize_children in pacquet/crates/resolving-deps-resolver/src/resolve_peers.rs), with no settled-promise timing gate, so it is deterministic by construction here.
Added a changeset.
Added or updated tests. — unit coverage for the changed line in both stacks (b19c3f5); see the testing note above for why this is a unit test rather than an e2e reproduction.

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

🤖 Generated with Claude Code

Summary by CodeRabbit

Bug Fixes
- Fixed non-deterministic peer dependency resolution that could intermittently add or omit optional transitive peers, leading to lockfile churn and occasional CI failures. Peer inclusion is now deterministic across identical installs.
Tests
- Added regression coverage to verify deterministic optional transitive peer handling.
- Added tests to ensure child resolution reuse rules behave correctly across different resolution depths.

coderabbitai · 2026-06-19T02:55:14Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: af8ef1d5-64f0-4066-982a-6bb46accf42b

📥 Commits

Reviewing files that changed from the base of the PR and between 7202435 and 812db5f.

📒 Files selected for processing (3)

pacquet/crates/resolving-deps-resolver/src/resolve_peers/tests.rs
pnpm11/installing/deps-resolver/src/resolveDependencies.ts
pnpm11/installing/deps-resolver/test/claimChildrenResolution.test.ts

🚧 Files skipped from review as they are similar to previous changes (3)

pnpm11/installing/deps-resolver/test/claimChildrenResolution.test.ts
pacquet/crates/resolving-deps-resolver/src/resolve_peers/tests.rs
pnpm11/installing/deps-resolver/src/resolveDependencies.ts

📝 Walkthrough

Walkthrough

claimChildrenResolution is exported and its reuse condition is narrowed to check owner depth only, eliminating a promise-settlement-timing dependency that could cause nondeterministic optional transitive peer inclusion. Rust and TypeScript regression tests verify determinism across repeated resolutions and depth scenarios. A changeset documents patch releases for @pnpm/installing.deps-resolver and pnpm.

Changes

Deterministic Peer Resolution

Layer / File(s)	Summary
Depth-based peer reuse in claimChildrenResolution `pnpm11/installing/deps-resolver/src/resolveDependencies.ts`	`claimChildrenResolution` is exported and its reuse condition is narrowed to depend only on owner depth (same or deeper), removing the previous promise-settled-based reuse path that could vary with concurrent timing.
Determinism regression and unit tests `pnpm11/installing/deps-resolver/test/claimChildrenResolution.test.ts`, `pacquet/crates/resolving-deps-resolver/src/resolve_peers/tests.rs`	JavaScript unit tests confirm depth-based reuse semantics: deeper consumers do not inherit shallower owners' promises, while same-depth consumers reuse via `importerResolutionOrder`. Rust regression test verifies optional transitive peers resolve consistently across 16 repeated resolutions.
Patch release changeset `.changeset/deterministic-shared-children-peers.md`	Declares patch releases for `@pnpm/installing.deps-resolver` and `pnpm`, documenting removal of the concurrent-resolution race and specifying peer inclusion now depends on graph structure rather than resolution-completion order.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

pnpm/pnpm#12362: Directly related introduction of deterministic "shared children resolution" ownership and locking model via resolveDependencies.ts logic that this PR refines.
pnpm/pnpm#12372: Addresses transitive optional and pending peer suffix handling in the same resolve_peers logic where this PR's Rust regression test validates determinism.

Suggested reviewers

zkochan

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

Warning

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

🔧 ESLint

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

ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration.

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

❤️ Share

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

qodo-free-for-open-source-projects · 2026-06-19T02:56:04Z

PR Summary by Qodo

Fix non-deterministic transitive peer suffix by removing timing-based missing-peers reuse
🐞 Bug fix 🕐 20-40 Minutes

Description

• Make shared-children missing-peers reuse depend only on dependency-graph depth
• Eliminate a resolution-order race that flipped optional transitive peers in peer suffixes
• Add a changeset documenting the determinism fix and CI dedupe impact

Diagram

graph TD
  A["resolveDependencies"] --> B["claimChildrenResolution"] --> C{Owner depth
>= current depth?}
  C -->|"yes"| D["Reuse owner's\nmissingPeersOfChildren"] --> F["Peer suffix\ncomposition"]
  C -->|"no"| E["Do not reuse\n(owner shallower)"] --> F
  B --> G["childrenResolutionByPkgId\n(shared cache)"]

High-Level Assessment

The following are alternative approaches to this PR:

1. Snapshot missing peers (no shared promise)

➕ Avoids sharing a mutable/settling promise across occurrences entirely
➕ Makes reuse semantics explicit (copy resolved data, not resolution state)
➖ Likely requires additional plumbing/storage and careful lifecycle management
➖ Still needs a policy for shallower-owner cases; could reintroduce awaits or deadlock hazards if not designed carefully

2. Always compute missing peers per occurrence

➕ Maximally deterministic: each node computes its own missing-peers context
➕ Avoids cross-occurrence coupling and timing sensitivity
➖ Potentially more expensive (duplicate work) on large graphs
➖ Undermines the purpose of sharing children resolution results

3. Introduce a deterministic scheduler/test seam

➕ Would enable a reliable regression test for the original race
➕ Improves confidence for future concurrency-sensitive resolver changes
➖ Higher implementation cost than the minimal fix
➖ Adds abstraction surface area to performance-critical code

Recommendation: The PR’s approach (remove the .resolved timing gate and rely only on a depth/graph-structural condition) is the best minimal-risk fix: it directly eliminates the only completion-order-dependent input to the reuse decision while preserving the existing deadlock-avoidance intent for shallower owners. The main follow-up worth considering is a deterministic scheduler/test seam to make this class of concurrency bug reproducible in CI.

Files changed (2) +18 / -4

Bug fix (1) +12 / -4

resolveDependencies.tsRemove timing-based missing-peers reuse in claimChildrenResolution +12/-4

Remove timing-based missing-peers reuse in claimChildrenResolution

• Simplifies the reuse condition for a shared package’s 'missingPeersOfChildren' by dropping the 'existing.missingPeersOfChildren.resolved' clause. Adds an explanatory comment detailing how the prior settled-promise check introduced a concurrency race and why gating reuse only by owner depth makes the result deterministic and consistent with prior deadlock avoidance.

installing/deps-resolver/src/resolveDependencies.ts

Documentation (1) +6 / -0

deterministic-shared-children-peers.mdAdd changeset documenting deterministic transitive-peer suffix fix +6/-0

Add changeset documenting deterministic transitive-peer suffix fix

• Introduces a patch changeset for @pnpm/installing.deps-resolver and pnpm. Describes the prior non-deterministic behavior where an optional transitive peer could flip in/out of the peer suffix across identical installs, causing lockfile churn and intermittent 'pnpm dedupe --check' failures.

.changeset/deterministic-shared-children-peers.md

qodo-free-for-open-source-projects · 2026-06-19T03:00:12Z

Code Review by Qodo

🐞 Bugs (4) 📘 Rule violations (0) 📎 Requirement gaps (0) 📜 Skill insights (0)

Context used

✅ Tickets: 🎫 pnpm hangs when encountering a cyclical aliased peer dependency 🎫 [Bug] Installation in monorepos is not working since 7.11

1. Test-driven internal export 🐞 Bug ⚙ Maintainability

Description

claimChildrenResolution is now exported from resolveDependencies.ts mainly to support a unit
test, which modestly increases the internal API surface of the shipped lib/resolveDependencies.js
module and can create additional coupling for internal deep-import consumers over time.

Code

pnpm11/installing/deps-resolver/src/resolveDependencies.ts[R1088-1091]
+export function claimChildrenResolution (
 ctx: ResolutionContext,
 opts: {
   currentDepth: number

Evidence
The diff explicitly changes claimChildrenResolution to an exported function, and the newly added
Jest test imports it from the built lib/resolveDependencies.js, demonstrating the export exists to
support tests rather than as part of the package’s primary public API.
pnpm11/installing/deps-resolver/src/resolveDependencies.ts[1088-1096]
pnpm11/installing/deps-resolver/test/claimChildrenResolution.test.ts[1-6]
pnpm11/installing/deps-resolver/package.json[16-25]

Agent prompt

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

## Issue description
`claimChildrenResolution` was changed from a file-private helper to an exported symbol primarily so a unit test can import it. While this is not exposed via the package `exports` map, it does expand the surface of the shipped `lib/resolveDependencies.js` module and may increase internal coupling / refactor cost.
### Issue Context
- The package publishes `lib/**` files.
- Tests import `claimChildrenResolution` from `../lib/resolveDependencies.js`.
### Fix Focus Areas
- pnpm11/installing/deps-resolver/src/resolveDependencies.ts[1088-1096]
- pnpm11/installing/deps-resolver/test/claimChildrenResolution.test.ts[1-6]
### Suggested fixes
- Prefer testing via a public entry point (e.g. a higher-level exported function) that exercises the behavior.
- Or export through a clearly-internal test hook namespace (e.g. `export const __test = { claimChildrenResolution }`) so it’s explicit this is not a supported API.
- Or move the helper into a dedicated module that is only imported by tests and not included in the published `files` list (if the repo’s build/publish conventions allow it).

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

2. Misstated depth reuse comment 🐞 Bug ⚙ Maintainability

Description

The new comment says reuse happens when the owner is at the “same or a deeper depth”, but owner
selection always prefers the shallowest occurrence (depth is the primary sort key), so in the
non-owner reuse branch existing.owner.depth >= opts.currentDepth is effectively only satisfiable
when the depths are equal. This mismatch can mislead maintainers about when reuse is possible and
why the guard is safe.

Code

installing/deps-resolver/src/resolveDependencies.ts[R1144-1154]

+    // Sharing the owner's missing-peers promise when the owner is shallower than
+    // this consumer previously also happened whenever that promise had already
+    // settled (`existing.missingPeersOfChildren.resolved`). That made reuse depend
+    // on whether the owner had finished by the time this node was claimed — a race
+    // under real concurrent resolution that let a transitive optional peer (e.g.
+    // styled-jsx's `@babel/core`) propagate into a deeper consumer's suffix on some
+    // runs but not others, churning the lockfile non-deterministically. Reusing
+    // only when the owner is at the same or a deeper depth keeps the decision a
+    // function of graph structure rather than completion order; awaiting a strictly
+    // shallower, possibly-unsettled owner is what the deadlock guard in
+    // https://github.com/pnpm/pnpm/issues/11999 avoided anyway.

Evidence

The added comment claims reuse when the owner is “same or deeper depth”. However,
compareChildrenResolutionOwners() sorts by depth first (lower depth wins), and
claimChildrenResolution() replaces the existing owner when the new candidate compares “less”
(i.e., shallower). Therefore, in the else-path (non-owner), the existing owner’s depth cannot be
greater than the current depth, making “deeper owner” wording inaccurate.

installing/deps-resolver/src/resolveDependencies.ts[1088-1136]
installing/deps-resolver/src/resolveDependencies.ts[1144-1154]
installing/deps-resolver/src/resolveDependencies.ts[1164-1172]

Agent prompt

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

## Issue description
The comment in `claimChildrenResolution` states reuse occurs when the owner is at the “same or a deeper depth”, but the owner-selection logic sorts by depth first, so in the non-owner branch the owner cannot be deeper than the current occurrence. This makes the comment misleading.
### Issue Context
`compareChildrenResolutionOwners()` prioritizes lower depth, and `claimChildrenResolution()` installs a new owner when a lower-depth owner is encountered.
### Fix Focus Areas
- installing/deps-resolver/src/resolveDependencies.ts[1144-1154]
### Suggested fix
Update the comment to reflect the effective invariant (reuse only for same-depth non-owner occurrences), or explicitly explain under what (if any) circumstances a deeper owner could exist.

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

3. Misleading issue link 🐞 Bug ⚙ Maintainability

Description

The new comment in claimChildrenResolution attributes the deadlock-avoidance rationale to
pnpm/pnpm#11999, but #11999 is documented in this repo as a cyclic peer dep-path calculation
deadlock in resolvePeers, not as missingPeers-of-children promise reuse. This mismatch can send
maintainers to the wrong thread when reasoning about why this guard exists.

Code

installing/deps-resolver/src/resolveDependencies.ts[R1144-1154]

+    // Sharing the owner's missing-peers promise when the owner is shallower than
+    // this consumer previously also happened whenever that promise had already
+    // settled (`existing.missingPeersOfChildren.resolved`). That made reuse depend
+    // on whether the owner had finished by the time this node was claimed — a race
+    // under real concurrent resolution that let a transitive optional peer (e.g.
+    // styled-jsx's `@babel/core`) propagate into a deeper consumer's suffix on some
+    // runs but not others, churning the lockfile non-deterministically. Reusing
+    // only when the owner is at the same or a deeper depth keeps the decision a
+    // function of graph structure rather than completion order; awaiting a strictly
+    // shallower, possibly-unsettled owner is what the deadlock guard in
+    // https://github.com/pnpm/pnpm/issues/11999 avoided anyway.

Evidence
The added comment explicitly links its deadlock rationale to #11999, but another in-repo comment
uses #11999 for a different deadlock mechanism in cyclic peer dep-path calculation, indicating the
reference here is likely incorrect/misleading.
installing/deps-resolver/src/resolveDependencies.ts[1144-1154]
installing/deps-resolver/src/resolvePeers.ts[657-663]

Agent prompt

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

## Issue description
A comment added to `claimChildrenResolution` links the deadlock explanation to `pnpm/pnpm#11999`, but in this codebase `#11999` is referenced in `resolvePeers.ts` for a different deadlock/cycle scenario. This makes the comment misleading for future maintainers.
### Issue Context
- The comment in `claimChildrenResolution` mentions “deadlock guard” and links to `#11999`.
- `resolvePeers.ts` also links to `#11999`, describing it as a cyclic peer calculateDepPath/cache-release issue.
### Fix Focus Areas
- installing/deps-resolver/src/resolveDependencies.ts[1144-1154]
### Suggested fix
Update the comment to either:
1) remove the issue link entirely, or
2) replace it with the issue/PR that actually motivated the `missingPeersOfChildren` reuse guard (i.e., the deadlock related to sharing/awaiting the missing-peers promise), and adjust the wording so it matches the linked reference.

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

View more (1)

4. Unused resolved flag 🐞 Bug ⚙ Maintainability

Description

MissingPeersOfChildren.resolved is still defined and set, but after this PR removed the
existing.missingPeersOfChildren.resolved condition in claimChildrenResolution, the flag is no
longer read anywhere. Keeping this dead state (and a comment referencing it) increases confusion and
makes future refactors riskier because it looks like a meaningful synchronization signal when it no
longer is.

Code

installing/deps-resolver/src/resolveDependencies.ts[R1139-1156]

ctx.hoistPeers &&
!opts.parentIds.includes(opts.pkgId) &&
existing.missingPeersOfChildren &&
-    (
-      existing.owner.depth >= opts.currentDepth ||
-      existing.missingPeersOfChildren.resolved
-    )
+    existing.owner.depth >= opts.currentDepth
) {
+    // Sharing the owner's missing-peers promise when the owner is shallower than
+    // this consumer previously also happened whenever that promise had already
+    // settled (`existing.missingPeersOfChildren.resolved`). That made reuse depend
+    // on whether the owner had finished by the time this node was claimed — a race
+    // under real concurrent resolution that let a transitive optional peer (e.g.
+    // styled-jsx's `@babel/core`) propagate into a deeper consumer's suffix on some
+    // runs but not others, churning the lockfile non-deterministically. Reusing
+    // only when the owner is at the same or a deeper depth keeps the decision a
+    // function of graph structure rather than completion order; awaiting a strictly
+    // shallower, possibly-unsettled owner is what the deadlock guard in
+    // https://github.com/pnpm/pnpm/issues/11999 avoided anyway.
missingPeersOfChildren = existing.missingPeersOfChildren
}

Evidence
The PR removed the only logical read of existing.missingPeersOfChildren.resolved in
claimChildrenResolution, but the resolved property remains on the type and is still assigned
when resolving and when transferring ownership, making it dead state.
installing/deps-resolver/src/resolveDependencies.ts[252-257]
installing/deps-resolver/src/resolveDependencies.ts[1022-1025]
installing/deps-resolver/src/resolveDependencies.ts[1119-1128]
installing/deps-resolver/src/resolveDependencies.ts[1137-1156]

Agent prompt

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

## Issue description
After removing the `existing.missingPeersOfChildren.resolved` reuse branch, the `MissingPeersOfChildren.resolved` flag is no longer consulted anywhere, but it is still written in several places and referenced in the new comment. This is now dead state that can mislead future contributors.
### Issue Context
- `MissingPeersOfChildren` still defines `resolved?: boolean`.
- The flag is still set when children finish resolving and when ownership changes.
- The only prior read site in `claimChildrenResolution` was removed in this PR.
### Fix Focus Areas
- Remove the unused `resolved` property from `MissingPeersOfChildren` and delete its assignments, **or** (if you want to keep it for debugging) add a clear comment explaining it is intentionally unused.
- Update/adjust the new explanatory comment so it no longer references `existing.missingPeersOfChildren.resolved` if the property is removed.
- installing/deps-resolver/src/resolveDependencies.ts[252-257]
- installing/deps-resolver/src/resolveDependencies.ts[1022-1025]
- installing/deps-resolver/src/resolveDependencies.ts[1119-1128]
- installing/deps-resolver/src/resolveDependencies.ts[1137-1156]

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

Previous review results

Review updated until commit 2cbfb73

Results up to commit 2cbfb73

🐞 Bugs (4) 📘 Rule violations (0) 📎 Requirement gaps (0) 📜 Skill insights (0)

Context used

✅ Tickets: 🎫 pnpm hangs when encountering a cyclical aliased peer dependency 🎫 [Bug] Installation in monorepos is not working since 7.11

1. Test-driven internal export 🐞 Bug ⚙ Maintainability ⭐ New

Description

claimChildrenResolution is now exported from resolveDependencies.ts mainly to support a unit
test, which modestly increases the internal API surface of the shipped lib/resolveDependencies.js
module and can create additional coupling for internal deep-import consumers over time.

Code

pnpm11/installing/deps-resolver/src/resolveDependencies.ts[R1088-1091]
+export function claimChildrenResolution (
  ctx: ResolutionContext,
  opts: {
    currentDepth: number

Evidence
The diff explicitly changes claimChildrenResolution to an exported function, and the newly added
Jest test imports it from the built lib/resolveDependencies.js, demonstrating the export exists to
support tests rather than as part of the package’s primary public API.
pnpm11/installing/deps-resolver/src/resolveDependencies.ts[1088-1096]
pnpm11/installing/deps-resolver/test/claimChildrenResolution.test.ts[1-6]
pnpm11/installing/deps-resolver/package.json[16-25]

Agent prompt

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

### Issue description
`claimChildrenResolution` was changed from a file-private helper to an exported symbol primarily so a unit test can import it. While this is not exposed via the package `exports` map, it does expand the surface of the shipped `lib/resolveDependencies.js` module and may increase internal coupling / refactor cost.

### Issue Context
- The package publishes `lib/**` files.
- Tests import `claimChildrenResolution` from `../lib/resolveDependencies.js`.

### Fix Focus Areas
- pnpm11/installing/deps-resolver/src/resolveDependencies.ts[1088-1096]
- pnpm11/installing/deps-resolver/test/claimChildrenResolution.test.ts[1-6]

### Suggested fixes
- Prefer testing via a public entry point (e.g. a higher-level exported function) that exercises the behavior.
- Or export through a clearly-internal test hook namespace (e.g. `export const __test = { claimChildrenResolution }`) so it’s explicit this is not a supported API.
- Or move the helper into a dedicated module that is only imported by tests and not included in the published `files` list (if the repo’s build/publish conventions allow it).

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

2. Misstated depth reuse comment 🐞 Bug ⚙ Maintainability

Description

The new comment says reuse happens when the owner is at the “same or a deeper depth”, but owner
selection always prefers the shallowest occurrence (depth is the primary sort key), so in the
non-owner reuse branch existing.owner.depth >= opts.currentDepth is effectively only satisfiable
when the depths are equal. This mismatch can mislead maintainers about when reuse is possible and
why the guard is safe.

Code

installing/deps-resolver/src/resolveDependencies.ts[R1144-1154]

+    // Sharing the owner's missing-peers promise when the owner is shallower than
+    // this consumer previously also happened whenever that promise had already
+    // settled (`existing.missingPeersOfChildren.resolved`). That made reuse depend
+    // on whether the owner had finished by the time this node was claimed — a race
+    // under real concurrent resolution that let a transitive optional peer (e.g.
+    // styled-jsx's `@babel/core`) propagate into a deeper consumer's suffix on some
+    // runs but not others, churning the lockfile non-deterministically. Reusing
+    // only when the owner is at the same or a deeper depth keeps the decision a
+    // function of graph structure rather than completion order; awaiting a strictly
+    // shallower, possibly-unsettled owner is what the deadlock guard in
+    // https://github.com/pnpm/pnpm/issues/11999 avoided anyway.

Evidence

The added comment claims reuse when the owner is “same or deeper depth”. However,
compareChildrenResolutionOwners() sorts by depth first (lower depth wins), and
claimChildrenResolution() replaces the existing owner when the new candidate compares “less”
(i.e., shallower). Therefore, in the else-path (non-owner), the existing owner’s depth cannot be
greater than the current depth, making “deeper owner” wording inaccurate.

installing/deps-resolver/src/resolveDependencies.ts[1088-1136]
installing/deps-resolver/src/resolveDependencies.ts[1144-1154]
installing/deps-resolver/src/resolveDependencies.ts[1164-1172]

Agent prompt

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

## Issue description
The comment in `claimChildrenResolution` states reuse occurs when the owner is at the “same or a deeper depth”, but the owner-selection logic sorts by depth first, so in the non-owner branch the owner cannot be deeper than the current occurrence. This makes the comment misleading.
### Issue Context
`compareChildrenResolutionOwners()` prioritizes lower depth, and `claimChildrenResolution()` installs a new owner when a lower-depth owner is encountered.
### Fix Focus Areas
- installing/deps-resolver/src/resolveDependencies.ts[1144-1154]
### Suggested fix
Update the comment to reflect the effective invariant (reuse only for same-depth non-owner occurrences), or explicitly explain under what (if any) circumstances a deeper owner could exist.

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

3. Misleading issue link 🐞 Bug ⚙ Maintainability

Description

The new comment in claimChildrenResolution attributes the deadlock-avoidance rationale to
pnpm/pnpm#11999, but #11999 is documented in this repo as a cyclic peer dep-path calculation
deadlock in resolvePeers, not as missingPeers-of-children promise reuse. This mismatch can send
maintainers to the wrong thread when reasoning about why this guard exists.

Code

installing/deps-resolver/src/resolveDependencies.ts[R1144-1154]

+    // Sharing the owner's missing-peers promise when the owner is shallower than
+    // this consumer previously also happened whenever that promise had already
+    // settled (`existing.missingPeersOfChildren.resolved`). That made reuse depend
+    // on whether the owner had finished by the time this node was claimed — a race
+    // under real concurrent resolution that let a transitive optional peer (e.g.
+    // styled-jsx's `@babel/core`) propagate into a deeper consumer's suffix on some
+    // runs but not others, churning the lockfile non-deterministically. Reusing
+    // only when the owner is at the same or a deeper depth keeps the decision a
+    // function of graph structure rather than completion order; awaiting a strictly
+    // shallower, possibly-unsettled owner is what the deadlock guard in
+    // https://github.com/pnpm/pnpm/issues/11999 avoided anyway.

Evidence
The added comment explicitly links its deadlock rationale to #11999, but another in-repo comment
uses #11999 for a different deadlock mechanism in cyclic peer dep-path calculation, indicating the
reference here is likely incorrect/misleading.
installing/deps-resolver/src/resolveDependencies.ts[1144-1154]
installing/deps-resolver/src/resolvePeers.ts[657-663]

Agent prompt

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

## Issue description
A comment added to `claimChildrenResolution` links the deadlock explanation to `pnpm/pnpm#11999`, but in this codebase `#11999` is referenced in `resolvePeers.ts` for a different deadlock/cycle scenario. This makes the comment misleading for future maintainers.
### Issue Context
- The comment in `claimChildrenResolution` mentions “deadlock guard” and links to `#11999`.
- `resolvePeers.ts` also links to `#11999`, describing it as a cyclic peer calculateDepPath/cache-release issue.
### Fix Focus Areas
- installing/deps-resolver/src/resolveDependencies.ts[1144-1154]
### Suggested fix
Update the comment to either:
1) remove the issue link entirely, or
2) replace it with the issue/PR that actually motivated the `missingPeersOfChildren` reuse guard (i.e., the deadlock related to sharing/awaiting the missing-peers promise), and adjust the wording so it matches the linked reference.

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

View more (1)

4. Unused resolved flag 🐞 Bug ⚙ Maintainability

Description

MissingPeersOfChildren.resolved is still defined and set, but after this PR removed the
existing.missingPeersOfChildren.resolved condition in claimChildrenResolution, the flag is no
longer read anywhere. Keeping this dead state (and a comment referencing it) increases confusion and
makes future refactors riskier because it looks like a meaningful synchronization signal when it no
longer is.

Code

installing/deps-resolver/src/resolveDependencies.ts[R1139-1156]

ctx.hoistPeers &&
!opts.parentIds.includes(opts.pkgId) &&
existing.missingPeersOfChildren &&
-    (
-      existing.owner.depth >= opts.currentDepth ||
-      existing.missingPeersOfChildren.resolved
-    )
+    existing.owner.depth >= opts.currentDepth
) {
+    // Sharing the owner's missing-peers promise when the owner is shallower than
+    // this consumer previously also happened whenever that promise had already
+    // settled (`existing.missingPeersOfChildren.resolved`). That made reuse depend
+    // on whether the owner had finished by the time this node was claimed — a race
+    // under real concurrent resolution that let a transitive optional peer (e.g.
+    // styled-jsx's `@babel/core`) propagate into a deeper consumer's suffix on some
+    // runs but not others, churning the lockfile non-deterministically. Reusing
+    // only when the owner is at the same or a deeper depth keeps the decision a
+    // function of graph structure rather than completion order; awaiting a strictly
+    // shallower, possibly-unsettled owner is what the deadlock guard in
+    // https://github.com/pnpm/pnpm/issues/11999 avoided anyway.
missingPeersOfChildren = existing.missingPeersOfChildren
}

Evidence
The PR removed the only logical read of existing.missingPeersOfChildren.resolved in
claimChildrenResolution, but the resolved property remains on the type and is still assigned
when resolving and when transferring ownership, making it dead state.
installing/deps-resolver/src/resolveDependencies.ts[252-257]
installing/deps-resolver/src/resolveDependencies.ts[1022-1025]
installing/deps-resolver/src/resolveDependencies.ts[1119-1128]
installing/deps-resolver/src/resolveDependencies.ts[1137-1156]

Agent prompt

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

## Issue description
After removing the `existing.missingPeersOfChildren.resolved` reuse branch, the `MissingPeersOfChildren.resolved` flag is no longer consulted anywhere, but it is still written in several places and referenced in the new comment. This is now dead state that can mislead future contributors.
### Issue Context
- `MissingPeersOfChildren` still defines `resolved?: boolean`.
- The flag is still set when children finish resolving and when ownership changes.
- The only prior read site in `claimChildrenResolution` was removed in this PR.
### Fix Focus Areas
- Remove the unused `resolved` property from `MissingPeersOfChildren` and delete its assignments, **or** (if you want to keep it for debugging) add a clear comment explaining it is intentionally unused.
- Update/adjust the new explanatory comment so it no longer references `existing.missingPeersOfChildren.resolved` if the property is removed.
- installing/deps-resolver/src/resolveDependencies.ts[252-257]
- installing/deps-resolver/src/resolveDependencies.ts[1022-1025]
- installing/deps-resolver/src/resolveDependencies.ts[1119-1128]
- installing/deps-resolver/src/resolveDependencies.ts[1137-1156]

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

Results up to commit 812db5f

🐞 Bugs (3) 📘 Rule violations (0) 📎 Requirement gaps (0) 📜 Skill insights (0)

Context used

✅ Tickets: 🎫 pnpm hangs when encountering a cyclical aliased peer dependency 🎫 [Bug] Installation in monorepos is not working since 7.11

1. Misstated depth reuse comment 🐞 Bug ⚙ Maintainability

Description

The new comment says reuse happens when the owner is at the “same or a deeper depth”, but owner
selection always prefers the shallowest occurrence (depth is the primary sort key), so in the
non-owner reuse branch existing.owner.depth >= opts.currentDepth is effectively only satisfiable
when the depths are equal. This mismatch can mislead maintainers about when reuse is possible and
why the guard is safe.

Code

installing/deps-resolver/src/resolveDependencies.ts[R1144-1154]

+    // Sharing the owner's missing-peers promise when the owner is shallower than
+    // this consumer previously also happened whenever that promise had already
+    // settled (`existing.missingPeersOfChildren.resolved`). That made reuse depend
+    // on whether the owner had finished by the time this node was claimed — a race
+    // under real concurrent resolution that let a transitive optional peer (e.g.
+    // styled-jsx's `@babel/core`) propagate into a deeper consumer's suffix on some
+    // runs but not others, churning the lockfile non-deterministically. Reusing
+    // only when the owner is at the same or a deeper depth keeps the decision a
+    // function of graph structure rather than completion order; awaiting a strictly
+    // shallower, possibly-unsettled owner is what the deadlock guard in
+    // https://github.com/pnpm/pnpm/issues/11999 avoided anyway.

Evidence

The added comment claims reuse when the owner is “same or deeper depth”. However,
compareChildrenResolutionOwners() sorts by depth first (lower depth wins), and
claimChildrenResolution() replaces the existing owner when the new candidate compares “less”
(i.e., shallower). Therefore, in the else-path (non-owner), the existing owner’s depth cannot be
greater than the current depth, making “deeper owner” wording inaccurate.

installing/deps-resolver/src/resolveDependencies.ts[1088-1136]
installing/deps-resolver/src/resolveDependencies.ts[1144-1154]
installing/deps-resolver/src/resolveDependencies.ts[1164-1172]

Agent prompt

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

## Issue description
The comment in `claimChildrenResolution` states reuse occurs when the owner is at the “same or a deeper depth”, but the owner-selection logic sorts by depth first, so in the non-owner branch the owner cannot be deeper than the current occurrence. This makes the comment misleading.
### Issue Context
`compareChildrenResolutionOwners()` prioritizes lower depth, and `claimChildrenResolution()` installs a new owner when a lower-depth owner is encountered.
### Fix Focus Areas
- installing/deps-resolver/src/resolveDependencies.ts[1144-1154]
### Suggested fix
Update the comment to reflect the effective invariant (reuse only for same-depth non-owner occurrences), or explicitly explain under what (if any) circumstances a deeper owner could exist.

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

2. Misleading issue link 🐞 Bug ⚙ Maintainability

Description

The new comment in claimChildrenResolution attributes the deadlock-avoidance rationale to
pnpm/pnpm#11999, but #11999 is documented in this repo as a cyclic peer dep-path calculation
deadlock in resolvePeers, not as missingPeers-of-children promise reuse. This mismatch can send
maintainers to the wrong thread when reasoning about why this guard exists.

Code

installing/deps-resolver/src/resolveDependencies.ts[R1144-1154]

+    // Sharing the owner's missing-peers promise when the owner is shallower than
+    // this consumer previously also happened whenever that promise had already
+    // settled (`existing.missingPeersOfChildren.resolved`). That made reuse depend
+    // on whether the owner had finished by the time this node was claimed — a race
+    // under real concurrent resolution that let a transitive optional peer (e.g.
+    // styled-jsx's `@babel/core`) propagate into a deeper consumer's suffix on some
+    // runs but not others, churning the lockfile non-deterministically. Reusing
+    // only when the owner is at the same or a deeper depth keeps the decision a
+    // function of graph structure rather than completion order; awaiting a strictly
+    // shallower, possibly-unsettled owner is what the deadlock guard in
+    // https://github.com/pnpm/pnpm/issues/11999 avoided anyway.

Evidence
The added comment explicitly links its deadlock rationale to #11999, but another in-repo comment
uses #11999 for a different deadlock mechanism in cyclic peer dep-path calculation, indicating the
reference here is likely incorrect/misleading.
installing/deps-resolver/src/resolveDependencies.ts[1144-1154]
installing/deps-resolver/src/resolvePeers.ts[657-663]

Agent prompt

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

## Issue description
A comment added to `claimChildrenResolution` links the deadlock explanation to `pnpm/pnpm#11999`, but in this codebase `#11999` is referenced in `resolvePeers.ts` for a different deadlock/cycle scenario. This makes the comment misleading for future maintainers.
### Issue Context
- The comment in `claimChildrenResolution` mentions “deadlock guard” and links to `#11999`.
- `resolvePeers.ts` also links to `#11999`, describing it as a cyclic peer calculateDepPath/cache-release issue.
### Fix Focus Areas
- installing/deps-resolver/src/resolveDependencies.ts[1144-1154]
### Suggested fix
Update the comment to either:
1) remove the issue link entirely, or
2) replace it with the issue/PR that actually motivated the `missingPeersOfChildren` reuse guard (i.e., the deadlock related to sharing/awaiting the missing-peers promise), and adjust the wording so it matches the linked reference.

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

3. Unused resolved flag 🐞 Bug ⚙ Maintainability

Description

MissingPeersOfChildren.resolved is still defined and set, but after this PR removed the
existing.missingPeersOfChildren.resolved condition in claimChildrenResolution, the flag is no
longer read anywhere. Keeping this dead state (and a comment referencing it) increases confusion and
makes future refactors riskier because it looks like a meaningful synchronization signal when it no
longer is.

Code

installing/deps-resolver/src/resolveDependencies.ts[R1139-1156]

ctx.hoistPeers &&
!opts.parentIds.includes(opts.pkgId) &&
existing.missingPeersOfChildren &&
-    (
-      existing.owner.depth >= opts.currentDepth ||
-      existing.missingPeersOfChildren.resolved
-    )
+    existing.owner.depth >= opts.currentDepth
) {
+    // Sharing the owner's missing-peers promise when the owner is shallower than
+    // this consumer previously also happened whenever that promise had already
+    // settled (`existing.missingPeersOfChildren.resolved`). That made reuse depend
+    // on whether the owner had finished by the time this node was claimed — a race
+    // under real concurrent resolution that let a transitive optional peer (e.g.
+    // styled-jsx's `@babel/core`) propagate into a deeper consumer's suffix on some
+    // runs but not others, churning the lockfile non-deterministically. Reusing
+    // only when the owner is at the same or a deeper depth keeps the decision a
+    // function of graph structure rather than completion order; awaiting a strictly
+    // shallower, possibly-unsettled owner is what the deadlock guard in
+    // https://github.com/pnpm/pnpm/issues/11999 avoided anyway.
missingPeersOfChildren = existing.missingPeersOfChildren
}

Evidence
The PR removed the only logical read of existing.missingPeersOfChildren.resolved in
claimChildrenResolution, but the resolved property remains on the type and is still assigned
when resolving and when transferring ownership, making it dead state.
installing/deps-resolver/src/resolveDependencies.ts[252-257]
installing/deps-resolver/src/resolveDependencies.ts[1022-1025]
installing/deps-resolver/src/resolveDependencies.ts[1119-1128]
installing/deps-resolver/src/resolveDependencies.ts[1137-1156]

Agent prompt

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

## Issue description
After removing the `existing.missingPeersOfChildren.resolved` reuse branch, the `MissingPeersOfChildren.resolved` flag is no longer consulted anywhere, but it is still written in several places and referenced in the new comment. This is now dead state that can mislead future contributors.
### Issue Context
- `MissingPeersOfChildren` still defines `resolved?: boolean`.
- The flag is still set when children finish resolving and when ownership changes.
- The only prior read site in `claimChildrenResolution` was removed in this PR.
### Fix Focus Areas
- Remove the unused `resolved` property from `MissingPeersOfChildren` and delete its assignments, **or** (if you want to keep it for debugging) add a clear comment explaining it is intentionally unused.
- Update/adjust the new explanatory comment so it no longer references `existing.missingPeersOfChildren.resolved` if the property is removed.
- installing/deps-resolver/src/resolveDependencies.ts[252-257]
- installing/deps-resolver/src/resolveDependencies.ts[1022-1025]
- installing/deps-resolver/src/resolveDependencies.ts[1119-1128]
- installing/deps-resolver/src/resolveDependencies.ts[1137-1156]

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

Results up to commit 812db5f

🐞 Bugs (3) 📘 Rule violations (0) 📎 Requirement gaps (0) 📜 Skill insights (0)

Context used

✅ Tickets: 🎫 pnpm hangs when encountering a cyclical aliased peer dependency 🎫 [Bug] Installation in monorepos is not working since 7.11

1. Misstated depth reuse comment 🐞 Bug ⚙ Maintainability

Description

The new comment says reuse happens when the owner is at the “same or a deeper depth”, but owner
selection always prefers the shallowest occurrence (depth is the primary sort key), so in the
non-owner reuse branch existing.owner.depth >= opts.currentDepth is effectively only satisfiable
when the depths are equal. This mismatch can mislead maintainers about when reuse is possible and
why the guard is safe.

Code

installing/deps-resolver/src/resolveDependencies.ts[R1144-1154]

+    // Sharing the owner's missing-peers promise when the owner is shallower than
+    // this consumer previously also happened whenever that promise had already
+    // settled (`existing.missingPeersOfChildren.resolved`). That made reuse depend
+    // on whether the owner had finished by the time this node was claimed — a race
+    // under real concurrent resolution that let a transitive optional peer (e.g.
+    // styled-jsx's `@babel/core`) propagate into a deeper consumer's suffix on some
+    // runs but not others, churning the lockfile non-deterministically. Reusing
+    // only when the owner is at the same or a deeper depth keeps the decision a
+    // function of graph structure rather than completion order; awaiting a strictly
+    // shallower, possibly-unsettled owner is what the deadlock guard in
+    // https://github.com/pnpm/pnpm/issues/11999 avoided anyway.

Evidence

The added comment claims reuse when the owner is “same or deeper depth”. However,
compareChildrenResolutionOwners() sorts by depth first (lower depth wins), and
claimChildrenResolution() replaces the existing owner when the new candidate compares “less”
(i.e., shallower). Therefore, in the else-path (non-owner), the existing owner’s depth cannot be
greater than the current depth, making “deeper owner” wording inaccurate.

installing/deps-resolver/src/resolveDependencies.ts[1088-1136]
installing/deps-resolver/src/resolveDependencies.ts[1144-1154]
installing/deps-resolver/src/resolveDependencies.ts[1164-1172]

Agent prompt

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

## Issue description
The comment in `claimChildrenResolution` states reuse occurs when the owner is at the “same or a deeper depth”, but the owner-selection logic sorts by depth first, so in the non-owner branch the owner cannot be deeper than the current occurrence. This makes the comment misleading.
### Issue Context
`compareChildrenResolutionOwners()` prioritizes lower depth, and `claimChildrenResolution()` installs a new owner when a lower-depth owner is encountered.
### Fix Focus Areas
- installing/deps-resolver/src/resolveDependencies.ts[1144-1154]
### Suggested fix
Update the comment to reflect the effective invariant (reuse only for same-depth non-owner occurrences), or explicitly explain under what (if any) circumstances a deeper owner could exist.

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

2. Misleading issue link 🐞 Bug ⚙ Maintainability

Description

The new comment in claimChildrenResolution attributes the deadlock-avoidance rationale to
pnpm/pnpm#11999, but #11999 is documented in this repo as a cyclic peer dep-path calculation
deadlock in resolvePeers, not as missingPeers-of-children promise reuse. This mismatch can send
maintainers to the wrong thread when reasoning about why this guard exists.

Code

installing/deps-resolver/src/resolveDependencies.ts[R1144-1154]

+    // Sharing the owner's missing-peers promise when the owner is shallower than
+    // this consumer previously also happened whenever that promise had already
+    // settled (`existing.missingPeersOfChildren.resolved`). That made reuse depend
+    // on whether the owner had finished by the time this node was claimed — a race
+    // under real concurrent resolution that let a transitive optional peer (e.g.
+    // styled-jsx's `@babel/core`) propagate into a deeper consumer's suffix on some
+    // runs but not others, churning the lockfile non-deterministically. Reusing
+    // only when the owner is at the same or a deeper depth keeps the decision a
+    // function of graph structure rather than completion order; awaiting a strictly
+    // shallower, possibly-unsettled owner is what the deadlock guard in
+    // https://github.com/pnpm/pnpm/issues/11999 avoided anyway.

Evidence
The added comment explicitly links its deadlock rationale to #11999, but another in-repo comment
uses #11999 for a different deadlock mechanism in cyclic peer dep-path calculation, indicating the
reference here is likely incorrect/misleading.
installing/deps-resolver/src/resolveDependencies.ts[1144-1154]
installing/deps-resolver/src/resolvePeers.ts[657-663]

Agent prompt

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

## Issue description
A comment added to `claimChildrenResolution` links the deadlock explanation to `pnpm/pnpm#11999`, but in this codebase `#11999` is referenced in `resolvePeers.ts` for a different deadlock/cycle scenario. This makes the comment misleading for future maintainers.
### Issue Context
- The comment in `claimChildrenResolution` mentions “deadlock guard” and links to `#11999`.
- `resolvePeers.ts` also links to `#11999`, describing it as a cyclic peer calculateDepPath/cache-release issue.
### Fix Focus Areas
- installing/deps-resolver/src/resolveDependencies.ts[1144-1154]
### Suggested fix
Update the comment to either:
1) remove the issue link entirely, or
2) replace it with the issue/PR that actually motivated the `missingPeersOfChildren` reuse guard (i.e., the deadlock related to sharing/awaiting the missing-peers promise), and adjust the wording so it matches the linked reference.

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

3. Unused resolved flag 🐞 Bug ⚙ Maintainability

Description

MissingPeersOfChildren.resolved is still defined and set, but after this PR removed the
existing.missingPeersOfChildren.resolved condition in claimChildrenResolution, the flag is no
longer read anywhere. Keeping this dead state (and a comment referencing it) increases confusion and
makes future refactors riskier because it looks like a meaningful synchronization signal when it no
longer is.

Code

installing/deps-resolver/src/resolveDependencies.ts[R1139-1156]

ctx.hoistPeers &&
!opts.parentIds.includes(opts.pkgId) &&
existing.missingPeersOfChildren &&
-    (
-      existing.owner.depth >= opts.currentDepth ||
-      existing.missingPeersOfChildren.resolved
-    )
+    existing.owner.depth >= opts.currentDepth
) {
+    // Sharing the owner's missing-peers promise when the owner is shallower than
+    // this consumer previously also happened whenever that promise had already
+    // settled (`existing.missingPeersOfChildren.resolved`). That made reuse depend
+    // on whether the owner had finished by the time this node was claimed — a race
+    // under real concurrent resolution that let a transitive optional peer (e.g.
+    // styled-jsx's `@babel/core`) propagate into a deeper consumer's suffix on some
+    // runs but not others, churning the lockfile non-deterministically. Reusing
+    // only when the owner is at the same or a deeper depth keeps the decision a
+    // function of graph structure rather than completion order; awaiting a strictly
+    // shallower, possibly-unsettled owner is what the deadlock guard in
+    // https://github.com/pnpm/pnpm/issues/11999 avoided anyway.
missingPeersOfChildren = existing.missingPeersOfChildren
}

Evidence
The PR removed the only logical read of existing.missingPeersOfChildren.resolved in
claimChildrenResolution, but the resolved property remains on the type and is still assigned
when resolving and when transferring ownership, making it dead state.
installing/deps-resolver/src/resolveDependencies.ts[252-257]
installing/deps-resolver/src/resolveDependencies.ts[1022-1025]
installing/deps-resolver/src/resolveDependencies.ts[1119-1128]
installing/deps-resolver/src/resolveDependencies.ts[1137-1156]

Agent prompt

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

## Issue description
After removing the `existing.missingPeersOfChildren.resolved` reuse branch, the `MissingPeersOfChildren.resolved` flag is no longer consulted anywhere, but it is still written in several places and referenced in the new comment. This is now dead state that can mislead future contributors.
### Issue Context
- `MissingPeersOfChildren` still defines `resolved?: boolean`.
- The flag is still set when children finish resolving and when ownership changes.
- The only prior read site in `claimChildrenResolution` was removed in this PR.
### Fix Focus Areas
- Remove the unused `resolved` property from `MissingPeersOfChildren` and delete its assignments, **or** (if you want to keep it for debugging) add a clear comment explaining it is intentionally unused.
- Update/adjust the new explanatory comment so it no longer references `existing.missingPeersOfChildren.resolved` if the property is removed.
- installing/deps-resolver/src/resolveDependencies.ts[252-257]
- installing/deps-resolver/src/resolveDependencies.ts[1022-1025]
- installing/deps-resolver/src/resolveDependencies.ts[1119-1128]
- installing/deps-resolver/src/resolveDependencies.ts[1137-1156]

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

Results up to commit 7202435

🐞 Bugs (3) 📘 Rule violations (0) 📎 Requirement gaps (0) 📜 Skill insights (0)

Context used

✅ Tickets: 🎫 [Bug] Installation in monorepos is not working since 7.11 🎫 pnpm hangs when encountering a cyclical aliased peer dependency

1. Misstated depth reuse comment 🐞 Bug ⚙ Maintainability

Description

The new comment says reuse happens when the owner is at the “same or a deeper depth”, but owner
selection always prefers the shallowest occurrence (depth is the primary sort key), so in the
non-owner reuse branch existing.owner.depth >= opts.currentDepth is effectively only satisfiable
when the depths are equal. This mismatch can mislead maintainers about when reuse is possible and
why the guard is safe.

Code

installing/deps-resolver/src/resolveDependencies.ts[R1144-1154]

+    // Sharing the owner's missing-peers promise when the owner is shallower than
+    // this consumer previously also happened whenever that promise had already
+    // settled (`existing.missingPeersOfChildren.resolved`). That made reuse depend
+    // on whether the owner had finished by the time this node was claimed — a race
+    // under real concurrent resolution that let a transitive optional peer (e.g.
+    // styled-jsx's `@babel/core`) propagate into a deeper consumer's suffix on some
+    // runs but not others, churning the lockfile non-deterministically. Reusing
+    // only when the owner is at the same or a deeper depth keeps the decision a
+    // function of graph structure rather than completion order; awaiting a strictly
+    // shallower, possibly-unsettled owner is what the deadlock guard in
+    // https://github.com/pnpm/pnpm/issues/11999 avoided anyway.

Evidence

The added comment claims reuse when the owner is “same or deeper depth”. However,
compareChildrenResolutionOwners() sorts by depth first (lower depth wins), and
claimChildrenResolution() replaces the existing owner when the new candidate compares “less”
(i.e., shallower). Therefore, in the else-path (non-owner), the existing owner’s depth cannot be
greater than the current depth, making “deeper owner” wording inaccurate.

[installing/deps-resolver/src/resolveDependencies.ts[1088-1136]](https://github.com/pnpm/pnpm/blo...

qodo-free-for-open-source-projects · 2026-06-19T03:05:58Z

Code review by qodo was updated up to the latest commit 824c3da

qodo-free-for-open-source-projects · 2026-06-19T03:12:07Z

Code review by qodo was updated up to the latest commit eebfd99

github-actions · 2026-06-19T06:30:25Z

Benchmark Results

#	Scenario	main	HEAD
1	Isolated linker: fresh restore, hot cache + hot store	2.462s ± 0.056s	2.445s ± 0.063s
2	Isolated linker: fresh add new dep, hot cache + hot store	5.273s ± 0.08s	6.4s ± 0.096s
3	Isolated linker: fresh install, hot cache + hot store	6.353s ± 0.077s	6.6s ± 0.065s
4	Isolated linker: fresh restore, cold cache + cold store	7.83s ± 0.055s	7.735s ± 0.157s
5	Isolated linker: fresh install, cold cache + cold store	12.907s ± 1.628s	12.792s ± 0.215s
6	GVS linker: fresh restore, hot cache + hot store	1.235s ± 0.04s	1.224s ± 0.017s

Run 27821711939 · 30 runs per scenario · triggered by @zkochan

qodo-free-for-open-source-projects · 2026-06-20T00:49:36Z

Code review by qodo was updated up to the latest commit 4c6dc24

qodo-free-for-open-source-projects · 2026-06-20T00:54:36Z

Code review by qodo was updated up to the latest commit 4c6dc24

zkochan · 2026-06-20T10:05:43Z

See the benchmark results. Looks like this makes the "Isolated linker: fresh add new dep, hot cache + hot store" scenario significantly slower. We probably still need to do the change, if it improves correctness and determinism, but we should think about maybe ways to make it faster.

davidbarratt · 2026-06-20T13:51:50Z

I tried to reproduce the fresh add new dep slowdown and could not — the change comes out neutral-to-faster by every measure I have. Details below so you can verify.

Running the same harness CI uses

Using integrated-benchmark against the real npm registry, building both revisions from git, isolated-linker.fresh-add-dep.hot-cache.hot-store, 12 runs:

order	main	HEAD	result
`pnpm@main pnpm@HEAD`	4.539s ± 0.045s	3.999s ± 0.173s	HEAD 1.14× faster
`pnpm@HEAD pnpm@main`	4.521s ± 0.037s	3.957s ± 0.028s	HEAD 1.14× faster

Both run-orders, so it isn't warmup drift. A separate full pnpm add is-odd (clone import, warm store, 25 runs via hyperfine) was a tie: main 7.001s ± 0.110s vs HEAD 6.960s ± 0.123s.

Why HEAD can't be doing more work

The dropped clause only affects the non-owner reuse branch in claimChildrenResolution. A non-owner returns early without re-resolving its children either way — reuse just adds an await on the owner's already-pending missingPeersOfChildren. Dropping it removes work; it never adds any.

Instrumenting add is-odd on benchmarks/fixture confirms it (main-logic vs HEAD-logic, same build):

Resolved graph: byte-identical lockfile.
resolveDependency calls: HEAD 2632 vs main 2644.
dependenciesTree nodes: HEAD 3105 vs main 3130.
Peer-hoist fixpoint passes: identical (1 and 1).
Reuse decisions diverge a lot (HEAD reuses 228 of ~1264 claims vs main's 955) — yet the output and the call counts are unchanged, because those reuses were redundant.

So HEAD consistently does equal-or-slightly-less resolution work for an identical result.

What I think the CI number was

The benchmark runs on ubuntu-latest — the shared 4-vCPU hosted runners, which aren't reliable for cross-build timing: Akinshin's measurements find even CPU-bound averages "up to three times different across builds" and conclude the default pool "can not be trusted" for performance comparisons (it's the premise behind tools like CodSpeed and Bencher). In run 27821711939, main was only ~17% slower than my local main, but HEAD was ~60% slower than my local HEAD — i.e. HEAD's 30-iteration block specifically landed on a degraded host (the small stddev just means the contention was sustained across that block). That run also predates the current commit 4c6dc24, though the resolver code is identical to the benchmarked push, so it's a fair comparison either way.

Suggestion

Since the regression doesn't reproduce and the resolver provably does less work, I don't think a perf change is warranted here. Could we re-trigger the benchmark workflow for a second data point? If it still shows a regression on ubuntu-latest, I'm happy to profile there and add a deterministic reuse path that recovers main's reuse without the timing race — but right now there's nothing to recover.

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

… resolution order `claimChildrenResolution` lets a non-owner occurrence of a shared package reuse the owner's `missingPeersOfChildren`. The reuse condition was existing.owner.depth >= opts.currentDepth || existing.missingPeersOfChildren.resolved The second clause made reuse depend on whether the owner's resolution had already settled by the time this node was claimed. Under concurrent resolution that settling time varies run to run, so a deeper consumer would inherit the owner's missing peers on some runs but not others. For a package like `next`, whose dependency styled-jsx declares an optional `@babel/core` peer, that flipped `@babel/core` in and out of the resolved peer suffix between otherwise identical installs, churning the lockfile and producing intermittent `pnpm dedupe --check` failures. Drop the `.resolved` clause: reuse the owner's missing peers only when the owner is at the same or a deeper depth, which is a function of the dependency graph rather than completion order. The `.resolved` flag was introduced in #5467 (closing #5454) to avoid a deadlock: with auto-install-peers in a workspace, a shared package reached through multiple importers could await its own not-yet-settled `missingPeersOfChildren` promise and hang. The flag let the code reuse that promise only once settled, otherwise re-resolve. Removing the clause is strictly more conservative — a shallower owner's promise is now never reused, settled or not, so no unsettled promise is ever awaited and the deadlock cannot return. The #5454 regression test ("installation on a workspace with many complex circular dependencies does not fail when auto install peers is on") still passes, as do the peer, dedupe, and cyclic suites. The deterministic owner selection added in #12362 made shared-package ownership order-independent but had moved this timing branch in verbatim without neutralizing it. pacquet needs no matching change: it resolves a non-owner's children lazily and per-occurrence during peer resolution (`realize_children`), with no equivalent settled-promise timing gate, so it is deterministic by construction here. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

zkochan · 2026-06-20T13:55:49Z

Is it possible to cover this with a test? And if yes, also add the same test to pacquet, even if it works as expected now

qodo-free-for-open-source-projects · 2026-06-20T14:02:52Z

Code review by qodo was updated up to the latest commit 09b5433

qodo-free-for-open-source-projects · 2026-06-20T14:07:43Z

Code review by qodo was updated up to the latest commit 09b5433

Add regression coverage for the determinism fix in #12514. pnpm: a focused unit test on `claimChildrenResolution`. The mock-registry e2e harness serializes resolution through the depth barrier (`waitForPackageResolutionTurn`), so it cannot reproduce the completion-order race end to end. The changed line is testable directly, though: a deeper consumer of a shared package must never inherit a strictly shallower owner's `missingPeersOfChildren`, even when that owner's promise has already settled (`resolved = true`) — the only run-to-run-variable input the dropped clause keyed on. The test fails on the pre-fix condition and passes after. A second case pins the retained same-depth structural sharing. `claimChildrenResolution` is exported so the test can drive it. The function's only external inputs are fields on a minimal `ResolutionContext`. pacquet: a parity test that builds the same shape — a shared `styled-jsx` occurrence whose optional `@babel/core` peer is provided by a sibling at a shallow depth but absent at a deeper one. pacquet resolves the tree in a single deterministic walk, so the deeper occurrence keeps the bare suffix while the shallow one carries `(@babel/core@7.0.0)`. Rebuilding the tree (fresh `HashMap`s) on each of 16 iterations guards against hashing order leaking into the graph the way completion order did upstream. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

qodo-free-for-open-source-projects · 2026-06-20T14:28:14Z

Code Review by Qodo

New Review Started

This review has been superseded by a new analysis

davidbarratt · 2026-06-20T14:28:24Z

Yes — done in b19c3f5.

pnpm. The end-to-end mock-registry harness still can't reproduce the flake: it serializes resolution through the depth barrier (waitForPackageResolutionTurn), so completion order is fixed regardless of response timing. But the line that changed is testable on its own. claimChildrenResolution now reuses an owner's missingPeersOfChildren purely as a function of depth, so a focused unit test pins exactly that: a deeper consumer of a shared package must not inherit a strictly shallower owner's missing peers even when the owner's promise has already settled (resolved = true) — which was the only run-to-run-variable input the dropped clause keyed on. It fails with the old || existing.missingPeersOfChildren.resolved clause restored and passes after. A second case keeps the legitimate same-depth structural sharing honest. claimChildrenResolution is exported for the test; its only inputs are fields on a minimal ResolutionContext.

pacquet. Same scenario, expressed at the level pacquet resolves at: a shared styled-jsx occurrence whose optional @babel/core peer is satisfied by a sibling at a shallow depth but has no provider in scope at a deeper one. The shallow occurrence carries (@babel/core@7.0.0); the deeper one keeps the bare suffix. Because pacquet walks the tree once and deterministically, the analogous risk is HashMap iteration order leaking into the result rather than promise-completion order — so the tree is rebuilt with fresh maps on each of 16 iterations and the graph is asserted byte-stable. It passes as expected; flipping the deeper-occurrence expectation confirms the assertion has teeth.

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

The perfectionist::bare-url dylint flagged the parity-check comment's bare PR URL under -D warnings. Wrap it in angle brackets. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

qodo-free-for-open-source-projects · 2026-06-20T14:34:58Z

Code Review by Qodo

New Review Started

This review has been superseded by a new analysis

qodo-free-for-open-source-projects · 2026-06-20T14:35:04Z

Code Review by Qodo

New Review Started

This review has been superseded by a new analysis

davidbarratt · 2026-06-20T14:38:04Z

@coderabbitai resume

coderabbitai · 2026-06-20T14:38:11Z

✅ Action performed

Reviews resumed.

codecov-commenter · 2026-06-20T14:44:24Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.14%. Comparing base (8c3a372) to head (2cbfb73).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #12514      +/-   ##
==========================================
- Coverage   88.14%   88.14%   -0.01%     
==========================================
  Files         314      314              
  Lines       43559    43559              
==========================================
- Hits        38397    38395       -2     
- Misses       5162     5164       +2

☔ View full report in Codecov by Harness.
📢 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.

qodo-free-for-open-source-projects · 2026-06-20T14:44:24Z

Code review by qodo was updated up to the latest commit 7202435

github-actions · 2026-06-20T15:09:17Z

Integrated-Benchmark Report (Linux)

Each scenario reports direct installs and pnpr installs. Bencher consumes pacquet@HEAD and pnpr@HEAD.

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

Command	Mean [s]	Min [s]	Max [s]	Relative
`pacquet@HEAD`	3.581 ± 0.185	3.399	3.907	1.87 ± 0.18
`pacquet@main`	3.594 ± 0.229	3.289	3.987	1.88 ± 0.19
`pnpr@HEAD`	1.915 ± 0.154	1.751	2.155	1.00
`pnpr@main`	1.964 ± 0.160	1.716	2.215	1.03 ± 0.12

BENCHMARK_REPORT.json

{
  "results": [
    {
      "command": "pacquet@HEAD",
      "mean": 3.58121869738,
      "stddev": 0.18538156775491088,
      "median": 3.50062788498,
      "user": 3.8750497599999996,
      "system": 2.0790732,
      "min": 3.39853485098,
      "max": 3.90737059798,
      "times": [
        3.5125954359800002,
        3.42880965798,
        3.8573191169800003,
        3.44021715698,
        3.4886603339800004,
        3.90737059798,
        3.71554672398,
        3.4480315369800003,
        3.6151015619800004,
        3.39853485098
      ]
    },
    {
      "command": "pacquet@main",
      "mean": 3.594245937880001,
      "stddev": 0.22877024655356243,
      "median": 3.54689953898,
      "user": 3.8658455599999995,
      "system": 2.0543262,
      "min": 3.2890915719800002,
      "max": 3.9865354389800003,
      "times": [
        3.9865354389800003,
        3.2890915719800002,
        3.4052262309800003,
        3.4299107599800003,
        3.8136801029800003,
        3.47657651498,
        3.8554145609800003,
        3.4262796039800003,
        3.6425220309800004,
        3.6172225629800003
      ]
    },
    {
      "command": "pnpr@HEAD",
      "mean": 1.9149450253800002,
      "stddev": 0.15370446564739537,
      "median": 1.88496811148,
      "user": 2.8831070599999995,
      "system": 1.8137563999999997,
      "min": 1.75068719498,
      "max": 2.1550885609800003,
      "times": [
        2.1550885609800003,
        2.13222377698,
        1.75068719498,
        1.93205998198,
        1.84559599498,
        1.7692384079799999,
        1.7949870669799999,
        1.92434022798,
        1.78148071098,
        2.06374832998
      ]
    },
    {
      "command": "pnpr@main",
      "mean": 1.96362618508,
      "stddev": 0.160475793466554,
      "median": 1.96260997648,
      "user": 2.8985471599999997,
      "system": 1.8034993,
      "min": 1.71602321298,
      "max": 2.21546259098,
      "times": [
        2.2023569599800004,
        1.83991807898,
        1.82371768198,
        2.03249047598,
        1.9996217319799998,
        1.99710837198,
        2.21546259098,
        1.71602321298,
        1.8814511649799999,
        1.92811158098
      ]
    }
  ]
}

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

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`pacquet@HEAD`	463.6 ± 10.3	452.9	483.7	1.00
`pacquet@main`	473.3 ± 11.3	461.4	487.1	1.02 ± 0.03
`pnpr@HEAD`	570.1 ± 91.9	482.4	723.7	1.23 ± 0.20
`pnpr@main`	498.8 ± 12.4	483.7	515.2	1.08 ± 0.04

BENCHMARK_REPORT.json

{
  "results": [
    {
      "command": "pacquet@HEAD",
      "mean": 0.46362338674000003,
      "stddev": 0.010296231818236506,
      "median": 0.46018548064000003,
      "user": 0.37434978,
      "system": 0.78483088,
      "min": 0.45294506064,
      "max": 0.48368352164,
      "times": [
        0.48368352164,
        0.47907050164000003,
        0.45601803564000004,
        0.45767189064,
        0.45837585264,
        0.46199510864000004,
        0.46290458364000003,
        0.46762992664,
        0.45593938564000003,
        0.45294506064
      ]
    },
    {
      "command": "pacquet@main",
      "mean": 0.4733073533399999,
      "stddev": 0.011321123999557996,
      "median": 0.47116971014000003,
      "user": 0.3798210799999999,
      "system": 0.7803454799999999,
      "min": 0.46141337864000004,
      "max": 0.48706826464,
      "times": [
        0.48361558164,
        0.46279223564,
        0.48515957764,
        0.46225726164000003,
        0.47855669364000003,
        0.46364611264,
        0.46378272664000003,
        0.48706826464,
        0.48478170064000004,
        0.46141337864000004
      ]
    },
    {
      "command": "pnpr@HEAD",
      "mean": 0.57006381644,
      "stddev": 0.09192466908287077,
      "median": 0.51512165264,
      "user": 0.39104387999999995,
      "system": 0.80295668,
      "min": 0.48240875864000005,
      "max": 0.72374137664,
      "times": [
        0.51685340964,
        0.51338989564,
        0.5108038546400001,
        0.48240875864000005,
        0.49095836264000003,
        0.6849745236400001,
        0.72374137664,
        0.6500372376400001,
        0.6317724496400001,
        0.49569829564
      ]
    },
    {
      "command": "pnpr@main",
      "mean": 0.49880939253999995,
      "stddev": 0.012423963787321668,
      "median": 0.49449740714,
      "user": 0.3878591799999999,
      "system": 0.7962367799999999,
      "min": 0.48369726164000004,
      "max": 0.5152148496400001,
      "times": [
        0.51449576364,
        0.49728614164,
        0.5095902096400001,
        0.5152148496400001,
        0.49013322364,
        0.49170867264,
        0.48483659264,
        0.49056645564,
        0.5105647546400001,
        0.48369726164000004
      ]
    }
  ]
}

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

Command	Mean [s]	Min [s]	Max [s]	Relative
`pacquet@HEAD`	4.018 ± 0.044	3.953	4.094	2.04 ± 0.16
`pacquet@main`	4.008 ± 0.044	3.936	4.053	2.03 ± 0.16
`pnpr@HEAD`	1.973 ± 0.154	1.759	2.251	1.00
`pnpr@main`	2.110 ± 0.106	1.916	2.255	1.07 ± 0.10

BENCHMARK_REPORT.json

{
  "results": [
    {
      "command": "pacquet@HEAD",
      "mean": 4.01794396254,
      "stddev": 0.044013015560698865,
      "median": 4.006837190840001,
      "user": 4.0284491000000004,
      "system": 2.0972571799999997,
      "min": 3.9531614093400003,
      "max": 4.094106147340001,
      "times": [
        3.9997182213399998,
        3.9531614093400003,
        4.01831827034,
        4.094106147340001,
        3.98386556834,
        3.99025732734,
        4.0436211473400006,
        4.012962674340001,
        4.082717152340001,
        4.000711707340001
      ]
    },
    {
      "command": "pacquet@main",
      "mean": 4.00775326564,
      "stddev": 0.04367261038319079,
      "median": 4.01834257084,
      "user": 4.050701699999999,
      "system": 2.0700961799999997,
      "min": 3.93566261334,
      "max": 4.05327183134,
      "times": [
        4.01515257334,
        3.93566261334,
        3.94160186634,
        4.0215325683400005,
        3.98582693534,
        4.0463230903400005,
        4.04933684834,
        4.04170476434,
        4.05327183134,
        3.98711956534
      ]
    },
    {
      "command": "pnpr@HEAD",
      "mean": 1.97277772724,
      "stddev": 0.15353561573366584,
      "median": 1.9598801403400001,
      "user": 2.7226472999999998,
      "system": 1.76079838,
      "min": 1.75930076734,
      "max": 2.2511325373399997,
      "times": [
        2.00091895734,
        2.1004654343399998,
        1.94914227834,
        2.1204937733399998,
        1.81614732134,
        1.75930076734,
        1.9706180023400002,
        2.2511325373399997,
        1.81734964734,
        1.94220855334
      ]
    },
    {
      "command": "pnpr@main",
      "mean": 2.11025618804,
      "stddev": 0.10574271800963318,
      "median": 2.1044396178399998,
      "user": 2.6989381999999997,
      "system": 1.7700796799999998,
      "min": 1.91582453334,
      "max": 2.25490784534,
      "times": [
        1.9997978273400001,
        2.23106148234,
        2.12849984534,
        2.21899483434,
        2.07094438034,
        2.25490784534,
        2.10370435734,
        2.07365189634,
        1.91582453334,
        2.10517487834
      ]
    }
  ]
}

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

Command	Mean [s]	Min [s]	Max [s]	Relative
`pacquet@HEAD`	1.155 ± 0.013	1.143	1.183	2.36 ± 0.04
`pacquet@main`	1.175 ± 0.091	1.129	1.433	2.40 ± 0.19
`pnpr@HEAD`	0.490 ± 0.005	0.485	0.501	1.00
`pnpr@main`	0.502 ± 0.010	0.493	0.521	1.03 ± 0.02

BENCHMARK_REPORT.json

{
  "results": [
    {
      "command": "pacquet@HEAD",
      "mean": 1.1550678856799999,
      "stddev": 0.013229961869302172,
      "median": 1.15157244038,
      "user": 1.34085074,
      "system": 1.02538278,
      "min": 1.14271937688,
      "max": 1.18336185288,
      "times": [
        1.16034997288,
        1.15475992288,
        1.14507664488,
        1.14883670688,
        1.17106376688,
        1.14559657288,
        1.18336185288,
        1.14460586588,
        1.15430817388,
        1.14271937688
      ]
    },
    {
      "command": "pacquet@main",
      "mean": 1.17518811658,
      "stddev": 0.09125266625947039,
      "median": 1.14595709188,
      "user": 1.33441444,
      "system": 1.03705988,
      "min": 1.12860541688,
      "max": 1.43330017788,
      "times": [
        1.14613185088,
        1.14604733688,
        1.14586684688,
        1.16306015288,
        1.12860541688,
        1.16224877188,
        1.13932205488,
        1.43330017788,
        1.14162638888,
        1.14567216788
      ]
    },
    {
      "command": "pnpr@HEAD",
      "mean": 0.48962150038,
      "stddev": 0.005080341198382143,
      "median": 0.48745687388000003,
      "user": 0.34125054,
      "system": 0.7545125799999999,
      "min": 0.48501781488,
      "max": 0.50056076488,
      "times": [
        0.49485673388,
        0.48566462188000004,
        0.48501781488,
        0.48741744988,
        0.49258969488000004,
        0.50056076488,
        0.48658790688000003,
        0.48749629788000004,
        0.48520172288,
        0.49082199588000003
      ]
    },
    {
      "command": "pnpr@main",
      "mean": 0.5018740772799999,
      "stddev": 0.010339440300965173,
      "median": 0.49812853888,
      "user": 0.34582373999999994,
      "system": 0.7600728799999998,
      "min": 0.49299771788,
      "max": 0.5207951938800001,
      "times": [
        0.50252948688,
        0.49885141088,
        0.52069468188,
        0.5207951938800001,
        0.49523705188,
        0.49299771788,
        0.49650991588000004,
        0.49740566688000004,
        0.49382007988000004,
        0.49989956688000003
      ]
    }
  ]
}

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

Command	Mean [s]	Min [s]	Max [s]	Relative
`pacquet@HEAD`	2.861 ± 0.037	2.813	2.943	5.80 ± 0.10
`pacquet@main`	2.884 ± 0.030	2.840	2.936	5.85 ± 0.09
`pnpr@HEAD`	0.504 ± 0.010	0.494	0.530	1.02 ± 0.02
`pnpr@main`	0.493 ± 0.005	0.484	0.500	1.00

BENCHMARK_REPORT.json

{
  "results": [
    {
      "command": "pacquet@HEAD",
      "mean": 2.86124332228,
      "stddev": 0.037317750181058595,
      "median": 2.8458546570800003,
      "user": 1.8305473799999998,
      "system": 1.2101905,
      "min": 2.81283014758,
      "max": 2.94320154058,
      "times": [
        2.90503433358,
        2.8439843605800004,
        2.8462884445800003,
        2.81283014758,
        2.84143102158,
        2.8454208695800003,
        2.86981845758,
        2.86098224458,
        2.84344180258,
        2.94320154058
      ]
    },
    {
      "command": "pacquet@main",
      "mean": 2.88384369098,
      "stddev": 0.029890202591214235,
      "median": 2.87455981658,
      "user": 1.8349770799999998,
      "system": 1.2262219999999997,
      "min": 2.8403672825800004,
      "max": 2.93610762058,
      "times": [
        2.85901764358,
        2.86248014558,
        2.8403672825800004,
        2.86923416758,
        2.91792431458,
        2.87988546558,
        2.8685837595800003,
        2.90647280658,
        2.8983637035800003,
        2.93610762058
      ]
    },
    {
      "command": "pnpr@HEAD",
      "mean": 0.5038358027800001,
      "stddev": 0.009842756483349289,
      "median": 0.50177373258,
      "user": 0.34095668,
      "system": 0.7795753,
      "min": 0.49417674158,
      "max": 0.5300725685800001,
      "times": [
        0.49992659658,
        0.5072658215800001,
        0.49820565258000005,
        0.49932365658,
        0.5024271025800001,
        0.5031564755800001,
        0.50112036258,
        0.5300725685800001,
        0.5026830495800001,
        0.49417674158
      ]
    },
    {
      "command": "pnpr@main",
      "mean": 0.4929381340800001,
      "stddev": 0.00536334706160918,
      "median": 0.49445192108,
      "user": 0.34238778000000003,
      "system": 0.7530076000000001,
      "min": 0.48408037958,
      "max": 0.49986889058,
      "times": [
        0.49986889058,
        0.48664718758000003,
        0.49806744158000005,
        0.49477093858000004,
        0.49261832658000004,
        0.49533363258,
        0.49711983358,
        0.49413290358,
        0.48408037958,
        0.48674180658000005
      ]
    }
  ]
}

github-actions · 2026-06-20T15:09:19Z

Bencher Report

Branch	pr/12514
Testbed	pacquet

Click to view all benchmark results

Benchmark	Latency	Benchmark Result milliseconds (ms) (Result Δ%)	Upper Boundary milliseconds (ms) (Limit %)
isolated-linker.fresh-install.cold-cache.cold-store	📈 view plot 🚷 view threshold	4,017.94 ms (-4.89%) Baseline: 4,224.53 ms	5,069.44 ms (79.26%)
isolated-linker.fresh-install.cold-cache.hot-store	📈 view plot 🚷 view threshold	2,861.24 ms (-4.62%) Baseline: 2,999.99 ms	3,599.99 ms (79.48%)
isolated-linker.fresh-install.hot-cache.hot-store	📈 view plot 🚷 view threshold	1,155.07 ms (-13.11%) Baseline: 1,329.33 ms	1,595.20 ms (72.41%)
isolated-linker.fresh-restore.cold-cache.cold-store	📈 view plot 🚷 view threshold	3,581.22 ms (-13.19%) Baseline: 4,125.34 ms	4,950.41 ms (72.34%)
isolated-linker.fresh-restore.hot-cache.hot-store	📈 view plot 🚷 view threshold	463.62 ms (-25.43%) Baseline: 621.76 ms	746.12 ms (62.14%)

🐰 View full continuous benchmarking report in Bencher

github-actions · 2026-06-20T15:09:20Z

Bencher Report

Branch	pr/12514
Testbed	pnpr

⚠️ WARNING: No Threshold found!
Without a Threshold, no Alerts will ever be generated.
Latency (nanoseconds (ns))
Click here to create a new Threshold
For more information, see the Threshold documentation.
To only post results if a Threshold exists, set the --ci-only-thresholds flag.

Click to view all benchmark results

Benchmark	Latency	milliseconds (ms)
isolated-linker.fresh-install.cold-cache.cold-store	📈 view plot ⚠️ NO THRESHOLD	1,972.78 ms
isolated-linker.fresh-install.cold-cache.hot-store	📈 view plot ⚠️ NO THRESHOLD	503.84 ms
isolated-linker.fresh-install.hot-cache.hot-store	📈 view plot ⚠️ NO THRESHOLD	489.62 ms
isolated-linker.fresh-restore.cold-cache.cold-store	📈 view plot ⚠️ NO THRESHOLD	1,914.95 ms
isolated-linker.fresh-restore.hot-cache.hot-store	📈 view plot ⚠️ NO THRESHOLD	570.06 ms

🐰 View full continuous benchmarking report in Bencher

zkochan · 2026-06-20T15:10:43Z

Good overall but the comments don't adhere the styleguide we have in this repo. See the latest styleguide on main branch and instructions related to docs in code

…ior code The comments on the determinism fix narrated the removed `.resolved` clause and the bug it caused, which belongs in the commit history rather than the source. Rewrite the source and test comments to state the current invariant — reuse of a shared package's missing-peers promise is gated on the owner's depth alone — and the failure mode each test guards, per the repository comment style guide. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

davidbarratt · 2026-06-20T15:31:51Z

Addressed in 812db5f. The comments violated the "don't record past implementation shape / refactor history / 'the previous code did X' framing" rule in the comment style guide — they narrated the removed .resolved clause and the bug it caused. Rewrote the source and test comments to describe the current contract instead:

resolveDependencies.ts: the comment now states the invariant (reuse of a shared package's missing-peers promise is gated on the owner's depth, so an optional transitive peer's presence in the suffix follows graph structure, and a strictly shallower owner is excluded because awaiting its possibly-unsettled promise can deadlock). Also corrected the linked issue to the actual deadlock one (#5454).
claimChildrenResolution.test.ts and the pacquet parity test: dropped the "the buggy condition keyed on" / "the way completion order did upstream" narration; the comments now state the invariant and what each case guards.

The full historical rationale (the .resolved clause, the #5467/#5454 deadlock) stays in the fix commit's message, where the guide says it belongs.

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

qodo-free-for-open-source-projects · 2026-06-20T15:38:49Z

Code review by qodo was updated up to the latest commit 812db5f

qodo-free-for-open-source-projects · 2026-06-20T15:45:17Z

Code review by qodo was updated up to the latest commit 812db5f

…ng-peers

qodo-free-for-open-source-projects · 2026-06-20T18:52:54Z

Code review by qodo was updated up to the latest commit 2cbfb73

qodo-free-for-open-source-projects · 2026-06-20T18:55:23Z

Code review by qodo was updated up to the latest commit 2cbfb73

davidbarratt requested a review from zkochan as a code owner June 19, 2026 02:54

coderabbitai Bot added the product: pnpm@11 label Jun 19, 2026

coderabbitai Bot approved these changes Jun 19, 2026

View reviewed changes

github-actions Bot added the reviewed: coderabbit CodeRabbit submitted an approving review label Jun 19, 2026

davidbarratt force-pushed the fix/deterministic-shared-children-missing-peers branch from 824c3da to eebfd99 Compare June 19, 2026 03:08

zkochan force-pushed the fix/deterministic-shared-children-missing-peers branch from eebfd99 to 4c6dc24 Compare June 20, 2026 00:45

davidbarratt force-pushed the fix/deterministic-shared-children-missing-peers branch from 4c6dc24 to 09b5433 Compare June 20, 2026 13:57

coderabbitai Bot added the product: pacquet label Jun 20, 2026

fix(resolver): wrap bare URL in test comment to satisfy dylint

7202435

The perfectionist::bare-url dylint flagged the parity-check comment's bare PR URL under -D warnings. Wrap it in angle brackets. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

zkochan approved these changes Jun 20, 2026

View reviewed changes

github-actions Bot added the state: automerge label Jun 20, 2026

zkochan enabled auto-merge (squash) June 20, 2026 16:03

Merge branch 'pnpm:main' into fix/deterministic-shared-children-missi…

2cbfb73

…ng-peers

zkochan disabled auto-merge June 20, 2026 19:45

zkochan merged commit 1cbb5f2 into pnpm:main Jun 20, 2026
26 of 28 checks passed

Uh oh!

Uh oh!

Conversation

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

Uh oh!

Summary

The problem

Root cause

Why the clause existed

The fix

How to verify the analysis

Testing note

Squash Commit Body

Checklist

Summary by CodeRabbit

Uh oh!

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

Uh oh!

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested reviewers

Uh oh!

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

PR Summary by Qodo

Uh oh!

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

Uh oh!

Code Review by Qodo

Previous review results

Uh oh!

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

Uh oh!

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

Uh oh!

github-actions Bot commented Jun 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Benchmark Results

Uh oh!

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

Uh oh!

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

Uh oh!

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

Uh oh!

Uh oh!

davidbarratt commented Jun 20, 2026

Running the same harness CI uses

Why HEAD can't be doing more work

What I think the CI number was

Suggestion

Uh oh!

zkochan commented Jun 20, 2026

Uh oh!

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

Uh oh!

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

Uh oh!

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

Uh oh!

Code Review by Qodo

New Review Started

Uh oh!

davidbarratt commented Jun 20, 2026

Uh oh!

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

Uh oh!

Code Review by Qodo

New Review Started

Uh oh!

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

Uh oh!

Code Review by Qodo

New Review Started

Uh oh!

davidbarratt commented Jun 20, 2026

Uh oh!

coderabbitai Bot commented Jun 20, 2026

Uh oh!

davidbarratt commented Jun 19, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jun 19, 2026 •

edited

Loading

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

edited

Loading

github-actions Bot commented Jun 19, 2026 •

edited

Loading

zkochan commented Jun 20, 2026 •

edited

Loading

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

edited

Loading

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

edited

Loading

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

edited

Loading

codecov-commenter commented Jun 20, 2026 •

edited

Loading

github-actions Bot commented Jun 20, 2026 •

edited

Loading

github-actions Bot commented Jun 20, 2026 •

edited

Loading

github-actions Bot commented Jun 20, 2026 •

edited

Loading