fix(installing.deps-resolver): deterministically order cyclic peer suffixes by davidbarratt · Pull Request #11826 · pnpm/pnpm

davidbarratt · 2026-05-21T16:11:01Z

Summary

Fixes #8155 — pnpm dedupe and pnpm install produce a different lockfile on consecutive runs when two transitive dependencies declare each other as peer dependencies (e.g. @aws-sdk/client-sts ↔ @aws-sdk/client-sso-oidc) and auto-install-peers is enabled.

The bug

Run the reproduction from the original issue — pnpm add @aws-sdk/client-s3@3.588.0, then loop pnpm dedupe --check — and the lockfile flip-flops between two equally-valid forms:

'@aws-sdk/client-sso-oidc': 3.588.0                                   vs   '@aws-sdk/client-sso-oidc': 3.588.0(@aws-sdk/client-sts@3.588.0)
'@aws-sdk/client-sts':      3.588.0(@aws-sdk/client-sso-oidc@3.588.0)      '@aws-sdk/client-sts':      3.588.0

On a CI pipeline that runs pnpm dedupe --check, this fails roughly half the time. The issue has been open since May 2024 and reproduces on every released version from 9.1.1 through 11.2.2 (reporter's own confirmation timeline and most recent independent confirmation on 10.17.1).

A commenter on the issue traced the regression to #8058 (perf change to peers caching, shipped in 9.1.1), and another commenter noted that auto-install-peers = false makes the issue go away.

The earlier non-determinism fix in #11110 addresses a different scenario (#10626: newly-published registry versions affecting dedupe), not this one.

Root cause

installing/deps-resolver/src/resolveDependencies.ts:686-708 was using a Promise.all with push inside the callback:

await Promise.all(
  extendedWantedDeps.map(async (extendedWantedDep) => {
    const { resolveDependencyResult, postponedResolution, postponedPeersResolution } =
      await resolveDependenciesOfDependency(ctx, preferredVersions, options, extendedWantedDep)
    if (resolveDependencyResult)   pkgAddresses.push(resolveDependencyResult as PkgAddress)
    if (postponedResolution)       postponedResolutionsQueue.push(postponedResolution)
    if (postponedPeersResolution)  postponedPeersResolutionQueue.push(postponedPeersResolution)
  })
)

Promise.all runs the callbacks concurrently; pushing the result from inside each callback means the order of items in pkgAddresses / postponedResolutionsQueue / postponedPeersResolutionQueue is the order of resolveDependenciesOfDependency completion, not the order of extendedWantedDeps. (MDN reference for Promise.all order semantics — the result array preserves input order, but side-effects inside the callbacks do not.)

That completion-order ordering then flows downstream:

pkgAddresses becomes the seed for currentParentPkgAliases and parentPkgAliases (line 711).
The postponed queues feed resolvePeers via startResolvingPeers, which builds the missingPeers set whose iteration order then drives the hoist loop in resolveRootDependencies.
Eventually resolvePeersOfChildren walks children in that order, and calculateDepPath in installing/deps-resolver/src/resolvePeers.ts:553-587 breaks the cycle by collapsing the second cyclic peer it visits to a suffix-less name@version. Whichever side of the cycle is processed first gets the full (other@version) suffix; the other side gets the bare form. So when timing jitter flips the processing order, the lockfile flips.

The existing two-circular-peers test at peerDependencies.ts:1832 doesn't exhibit the bug because both packages there are direct deps of the root project, so the hoist loop never re-runs and the cycle never travels through the racy code path. The bug specifically requires the cycle members to be discovered via auto-install-peers hoisting.

The exact "await Promise.all to a temporary, then for…of over it" pattern is already used 200 lines earlier in resolveDependenciesOfImporters — resolveDependencies was the one place that diverged from it.

The fix

One structural change: drain the Promise.all results in a separate for…of so they land in input order.

const resolvedDependencies = await Promise.all(
  extendedWantedDeps.map(d => resolveDependenciesOfDependency(ctx, preferredVersions, options, d))
)
for (const { resolveDependencyResult, postponedResolution, postponedPeersResolution } of resolvedDependencies) {
  if (resolveDependencyResult)   pkgAddresses.push(resolveDependencyResult as PkgAddress)
  if (postponedResolution)       postponedResolutionsQueue.push(postponedResolution)
  if (postponedPeersResolution)  postponedPeersResolutionQueue.push(postponedPeersResolution)
}

That's it — no behavioral change in resolution, just removing the order leak. Promise.all's contract guarantees the result array is in input order; for…of preserves that.

Empirical validation

End-to-end reproduction from the issue, bundled with pnpm/dist/pnpm.mjs built from this branch:

mkdir /tmp/repro && cd /tmp/repro
cat > package.json <<'JSON'
{ "dependencies": { "@aws-sdk/client-s3": "3.588.0" } }
JSON
pnpm install
fail=0
for i in $(seq 1 50); do
  pnpm dedupe --check >/dev/null 2>&1 || fail=$((fail+1))
done
echo "failures: $fail / 50"

Build	`pnpm dedupe --check` failures
`pnpm@11.2.2` (unfixed)	33 / 50 (~66%)
this PR	0 / 50
this PR (longer stress run)	0 / 100

Regression test

Added installing/deps-installer/test/install/cyclicPeerDeterminism.ts. It uses @pnpm/testing.mock-agent (same pattern as the prereleaseWeights.ts regression test for #10626) to mock three fake packages — a wrapper that pulls in two transitive deps that peer each other — and runs install 30 times, asserting the lockfile snapshot keys are byte-identical across runs.

Why pacquet does not have the same problem

I audited the pacquet port to check whether the same fix needs to land there. It doesn't — pacquet's analogous code already preserves input order by construction:

pacquet/crates/resolving-deps-resolver/src/resolve_dependency_tree.rs:270-283 (extend_tree) and L395-L421 (resolve_node) parallelise children via future::try_join_all, whose contract is "preserves the input order" — the Rust idiom returns values from each future and collects them, rather than push-ing from inside the future, which structurally rules out the JS race.
pacquet/crates/resolving-deps-resolver/src/resolve_importer.rs collects missing_required from a BTreeMap, and hoisted peers come from hoist_peers which also returns a BTreeMap — both ordered alphabetically by key, per the Rust stdlib contract.

So pacquet's order is twice-canonical (input-preserving + lexicographic on hoist) where pnpm's was completion-order. No pacquet-side change needed.

Test plan

CI runs the existing '2 circular peers', '3 circular peers', '3 circular peers in workspace root', and 'resolves complex circular deps' tests in peerDependencies.ts — they should still pass.
CI runs the new cyclicPeerDeterminism.ts regression test — should be green.
Manually verify the issue's reproduction (pnpm add @aws-sdk/client-s3@3.588.0, then loop pnpm dedupe --check) succeeds every iteration on a build from this branch.

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

Summary by CodeRabbit

Bug Fixes
- Fixed non-deterministic lockfile generation during installs and dedupe when auto-install-peers is enabled with mutually related transitive peer dependencies; lockfiles no longer flip between valid forms across runs.
Tests
- Added a regression test that repeats installs multiple times to verify deterministic lockfile/snapshot ordering.

…ffixes (#8155) `resolveDependencies` was pushing onto `pkgAddresses`, `postponedResolutionsQueue`, and `postponedPeersResolutionQueue` from inside `Promise.all`-spawned callbacks, so the order of items in those arrays reflected completion timing rather than the order of `extendedWantedDeps`. That ordering then flowed downstream into `resolvePeers` and the cyclic-peer suffix assignment, so two packages with transitive peer dependencies on each other (e.g. `@aws-sdk/client-sts` and `@aws-sdk/client-sso-oidc`) flipped between two equally-valid lockfile forms across consecutive installs. The fix awaits `Promise.all` to a temporary array and drains it with `for…of` so the per-edge results land in input order. This matches the existing pattern 200 lines earlier in `resolveDependenciesOfImporters`. End-to-end repro from the issue (`pnpm add @aws-sdk/client-s3@3.588.0` then loop `pnpm dedupe --check`): 33/50 failures without the fix → 0/100 with it. --- Written by an agent (Claude Code, claude-opus-4-7). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

welcome · 2026-05-21T16:11:11Z

💖 Thanks for opening this pull request! 💖
Please be patient and we will get back to you as soon as we can.

qodo-code-review · 2026-05-21T16:11:22Z

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

coderabbitai · 2026-05-21T16:11:27Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 960deec8-f4ed-4f5c-9e45-a15ea5e4e8ac

📥 Commits

Reviewing files that changed from the base of the PR and between 266ad5e and 85b78aa.

📒 Files selected for processing (1)

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

📜 Recent review details

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

GitHub Check: Compile & Lint

🧰 Additional context used

📓 Path-based instructions (1)

**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ts,tsx}: Use trailing commas in TypeScript code
Prefer functions over classes in TypeScript code
Declare functions after they are used, relying on function hoisting in TypeScript
Functions should have no more than two or three arguments; use a single options object instead for multiple parameters in TypeScript
Follow import order in TypeScript: 1) standard libraries, 2) external dependencies (alphabetically sorted), 3) relative imports
Write self-explanatory code in TypeScript; avoid comments that restate what the code already says. Use comments only for non-obvious reasons, hidden invariants, or workarounds
Use JSDoc for function contracts (preconditions, postconditions, edge cases), not for re-narrating the function body in TypeScript

Files:

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

🧠 Learnings (1)

📚 Learning: 2026-05-14T09:04:00.133Z

Learnt from: zkochan
Repo: pnpm/pnpm PR: 11622
File: resolving/npm-resolver/test/publishedBy.test.ts:350-354
Timestamp: 2026-05-14T09:04:00.133Z
Learning: In the pnpm/pnpm repository, ESLint is the authoritative style linter. Do not raise review findings for missing trailing commas in multiline function calls (e.g., `fs.writeFileSync(...)`) when this repo’s ESLint configuration does not report them and lint passes. Prefer deferring to the ESLint results for this specific trailing-comma rule rather than enforcing it manually in code review.

Applied to files:

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

🔇 Additional comments (1)

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

80-84: LGTM!

Also applies to: 92-92, 108-108

📝 Walkthrough

Walkthrough

Reorders accumulation in resolveDependencies to collect parallel dependency resolutions and drain them in deterministic input order; adds a regression test exercising cyclic transitive peer dependencies and a changeset noting the fix (addresses issue #8155).

Changes

Cyclic Peer Resolution Determinism Fix

Layer / File(s)	Summary
Resolver deterministic accumulation `installing/deps-resolver/src/resolveDependencies.ts`	`resolveDependencies` now gathers results from parallel `resolveDependenciesOfDependency` calls and then drains them in input order to populate `pkgAddresses` and postponed peer/resolution queues, removing completion-order timing effects that caused nondeterministic cyclic-peer suffix assignment.
Cyclic peer determinism regression test `installing/deps-installer/test/install/cyclicPeerDeterminism.ts`	Adds a Jest test that sets up a mock registry with mutually-peered transitive dependencies, enables `autoInstallPeers`, performs installs across 30 iterations, and asserts the lockfile snapshot key ordering is identical across runs. Includes teardown in `afterEach`.
Changeset documentation `.changeset/fix-cyclic-peer-determinism.md`	Patch changeset for `@pnpm/installing.deps-resolver` documenting the nondeterminism fix in `pnpm dedupe`/`pnpm install` caused by `resolveDependencies` timing effects and referencing issue `#8155`.

Sequence Diagram(s)

sequenceDiagram
  participant Client as Installer
  participant Resolver as resolveDependencies
  participant Child as resolveDependenciesOfDependency
  participant Accumulators as pkgAddresses/postponedQueues
  Client->>Resolver: request dependency resolution
  Resolver->>Child: spawn parallel resolveDependenciesOfDependency per dep
  Child-->>Resolver: return resolution result
  Resolver->>Accumulators: drain collected results in input order to update pkgAddresses/postponedQueues
  Resolver-->>Client: resolved dependency graph (deterministic)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested reviewers

zkochan

Poem

🐰 Hop, hop, the queues align,
Parallel work now drains in line.
Cyclic peers no longer race,
Lockfiles keep a steady face.
Snaps repeat — deterministic grace!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	Title clearly summarizes the main fix: deterministic ordering of cyclic peer suffixes in the resolver, directly addressing the core issue.
Linked Issues check	✅ Passed	Changes fully address issue `#8155` by fixing nondeterministic lockfile behavior through Promise.all refactoring and adding determinism validation test.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to fixing cyclic peer determinism: resolver logic fix, regression test, and changeset metadata—no out-of-scope modifications detected.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

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.}

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@installing/deps-installer/test/install/cyclicPeerDeterminism.ts`:
- Line 109: The test currently masks ordering by sorting snapshot keys—replace
the sorted retrieval with a raw key list to preserve insertion/completion order:
change the use of snapshotKeys (currently assigned via
Object.keys(lockfile.snapshots ?? {}).sort()) to use
Object.keys(lockfile.snapshots ?? {}) without .sort() so the determinism test
can detect completion-order regressions.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

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

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: ea86d746-ccff-44ca-9c79-5726b9e75bca

📥 Commits

Reviewing files that changed from the base of the PR and between f9a0abe and 7577d47.

📒 Files selected for processing (3)

.changeset/fix-cyclic-peer-determinism.md
installing/deps-installer/test/install/cyclicPeerDeterminism.ts
installing/deps-resolver/src/resolveDependencies.ts

📜 Review details

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)

GitHub Check: Compile & Lint
GitHub Check: Analyze (javascript)

🧰 Additional context used

📓 Path-based instructions (1)

**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ts,tsx}: Use trailing commas in TypeScript code
Prefer functions over classes in TypeScript code
Declare functions after they are used, relying on function hoisting in TypeScript
Functions should have no more than two or three arguments; use a single options object instead for multiple parameters in TypeScript
Follow import order in TypeScript: 1) standard libraries, 2) external dependencies (alphabetically sorted), 3) relative imports
Write self-explanatory code in TypeScript; avoid comments that restate what the code already says. Use comments only for non-obvious reasons, hidden invariants, or workarounds
Use JSDoc for function contracts (preconditions, postconditions, edge cases), not for re-narrating the function body in TypeScript

Files:

installing/deps-installer/test/install/cyclicPeerDeterminism.ts
installing/deps-resolver/src/resolveDependencies.ts

🧠 Learnings (1)

📚 Learning: 2026-05-14T09:04:00.133Z

Learnt from: zkochan
Repo: pnpm/pnpm PR: 11622
File: resolving/npm-resolver/test/publishedBy.test.ts:350-354
Timestamp: 2026-05-14T09:04:00.133Z
Learning: In the pnpm/pnpm repository, ESLint is the authoritative style linter. Do not raise review findings for missing trailing commas in multiline function calls (e.g., `fs.writeFileSync(...)`) when this repo’s ESLint configuration does not report them and lint passes. Prefer deferring to the ESLint results for this specific trailing-comma rule rather than enforcing it manually in code review.

Applied to files:

installing/deps-installer/test/install/cyclicPeerDeterminism.ts
installing/deps-resolver/src/resolveDependencies.ts

🪛 GitHub Check: CodeQL

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

[failure] 88-88: Incomplete string escaping or encoding
This replaces only the first occurrence of '/'.

🔇 Additional comments (3)

installing/deps-resolver/src/resolveDependencies.ts (1)

686-708: LGTM!

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

1-108: LGTM!

Also applies to: 110-120

.changeset/fix-cyclic-peer-determinism.md (1)

1-7: LGTM!

… path Addresses CodeQL incomplete-string-escaping finding: `replace('/', '%2F')` only swaps the first occurrence. Scoped names in this test only have one slash so the behavior is unchanged, but switching to `replaceAll` clears the warning and is more defensible. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

coderabbitai

♻️ Duplicate comments (1)

installing/deps-installer/test/install/cyclicPeerDeterminism.ts (1)
109-109: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Sorting snapshot keys weakens the determinism signal.

.sort() collapses any insertion-order differences into a single canonical sequence, so a regression that reintroduces completion-order leakage (without changing which keys exist) would still pass this test. Drop the .sort() to retain the ordering signal this test is designed to catch.
Proposed fix
-    const snapshotKeys = Object.keys(lockfile.snapshots ?? {}).sort()
+    const snapshotKeys = Object.keys(lockfile.snapshots ?? {})
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@installing/deps-installer/test/install/cyclicPeerDeterminism.ts` at line 109,
The test currently masks insertion-order differences by sorting snapshot keys;
locate where snapshotKeys is computed (the const snapshotKeys =
Object.keys(lockfile.snapshots ?? {}).sort() assignment) and remove the .sort()
call so snapshotKeys = Object.keys(lockfile.snapshots ?? {}), preserving
original ordering and allowing the test to detect completion-order leakage.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Duplicate comments:
In `@installing/deps-installer/test/install/cyclicPeerDeterminism.ts`:
- Line 109: The test currently masks insertion-order differences by sorting
snapshot keys; locate where snapshotKeys is computed (the const snapshotKeys =
Object.keys(lockfile.snapshots ?? {}).sort() assignment) and remove the .sort()
call so snapshotKeys = Object.keys(lockfile.snapshots ?? {}), preserving
original ordering and allowing the test to detect completion-order leakage.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 506481f6-0719-4588-96e7-da4f47065bc0

📥 Commits

Reviewing files that changed from the base of the PR and between 7577d47 and 266ad5e.

📒 Files selected for processing (1)

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

📜 Review details

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

GitHub Check: Compile & Lint

🧰 Additional context used

📓 Path-based instructions (1)

**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ts,tsx}: Use trailing commas in TypeScript code
Prefer functions over classes in TypeScript code
Declare functions after they are used, relying on function hoisting in TypeScript
Functions should have no more than two or three arguments; use a single options object instead for multiple parameters in TypeScript
Follow import order in TypeScript: 1) standard libraries, 2) external dependencies (alphabetically sorted), 3) relative imports
Write self-explanatory code in TypeScript; avoid comments that restate what the code already says. Use comments only for non-obvious reasons, hidden invariants, or workarounds
Use JSDoc for function contracts (preconditions, postconditions, edge cases), not for re-narrating the function body in TypeScript

Files:

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

🧠 Learnings (1)

📚 Learning: 2026-05-14T09:04:00.133Z

Learnt from: zkochan
Repo: pnpm/pnpm PR: 11622
File: resolving/npm-resolver/test/publishedBy.test.ts:350-354
Timestamp: 2026-05-14T09:04:00.133Z
Learning: In the pnpm/pnpm repository, ESLint is the authoritative style linter. Do not raise review findings for missing trailing commas in multiline function calls (e.g., `fs.writeFileSync(...)`) when this repo’s ESLint configuration does not report them and lint passes. Prefer deferring to the ESLint results for this specific trailing-comma rule rather than enforcing it manually in code review.

Applied to files:

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

Removed the .sort() applied to the lockfile snapshot keys in the cyclic peer determinism test so the comparison reflects the actual order emitted by the lockfile writer. The deterministic ordering guaranteed by 7577d47 makes the sorted view and the raw view identical today; dropping the sort lets the test fail on any future regression that keeps the key set stable but shuffles the order. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…ect literal MutatedProject does not carry a manifest field; it is conveyed via allProjects in MutateModulesOptions. Passing it inside the install project literal triggered TS2353 against the InstallDepsMutation shape. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Clearer name for the map keyed by package name, and avoids tripping cspell on the abbreviation "metas". Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Ubuntu Node.js 22.13.0 hit a transient 404 from Verdaccio's proxy-to-npm while resolving a transitive peer of @medusajs/medusa-js@6.1.7 in the pre-existing "install should not hang on circular peer dependencies" test (installing/deps-installer/test/install/misc.ts:1247). Ubuntu Node 24 and Node 26 ran the same code green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

welcome · 2026-05-22T00:20:04Z

Congrats on merging your first pull request! 🎉🎉🎉

Resolved two conflicts against upstream changes in `perf(pacquet): close the warm-cache resolve gap to pnpm CLI` (#11837) and `fix(installing.deps-resolver): deterministically order cyclic peer suffixes` (#11826): - `package-manager/src/install_with_fresh_lockfile.rs`: main dropped the `enable_global_virtual_store` conditional and now builds the lockfile unconditionally (renamed `layout_lockfile` → `built_lockfile`, added a tracing span). The clippy `if_then_some_else_none` rewrite this branch added to that block is no longer applicable — took main's version. - `resolving-deps-resolver/src/resolve_peers.rs` (two sites): main changed `NodeId` from a `Copy` newtype to an `enum { Counter(u64), Leaf(Arc<str>) }`, so `Some(node_id)` had to become `Some(node_id.clone())`. Combined with this branch's `if_then_some_else_none` rewrite of the sibling `alias` field. The new `node_id.clone()` does not trip `clippy::clone_on_ref_ptr` because `NodeId` is an enum, not an `Arc`/`Rc`/`Weak`. `engine-runtime-node-resolver/src/node_resolver.rs` and `resolving-npm-resolver/src/resolve_from_workspace.rs` auto-merged; the clippy fixes on those lines survived intact. Verified locally: `cargo clippy --locked --workspace --all-targets -- --deny warnings`, `cargo fmt --check`, `taplo format --check`.

* Upgrade pnpm to 11.3.0 to unblock dedupe --check in CI pnpm 9.1.1–11.2.x had a non-deterministic dedupe pass that failed ~50% of the time on identical input, so the workspace was carrying a commented-out `pnpm dedupe --check` step. 11.3.0 ships the fix (pnpm/pnpm#11826, pnpm/pnpm#8155), so this restores the check and brings every project onto a single version via corepack. Also performs the v11 config migration the release demands: settings other than auth/registry move out of .npmrc and into pnpm-workspace.yaml, and package.json#pnpm overrides move there too. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Upgrade deps * Upgrade deps * Restore eslint/prettier public-hoist after pnpm 10 default change pnpm 10 changed the default `publicHoistPattern` from `['*eslint*', '*prettier*']` to `[]`. That broke `apps/mcp-server/eslint.config.ts`, which imports `eslint-plugin-jsx-a11y`, `eslint-plugin-react`, and `eslint-plugin-react-hooks` directly from a sibling workspace package's transitive deps — it was relying on the old hoist. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Bump claude container memory to 8g The 4g cap was OOM-killing parallel `tsc` workers during `pnpm typecheck`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Format pnpm-workspace.yaml with prettier Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Enable injectWorkspacePackages for pnpm v10+ deploy pnpm v10+ refuses `pnpm deploy` from a non-injected workspace, breaking the flex-ui Docker builds. Enable injectWorkspacePackages so the deploy step in flex-ui/Dockerfile works without falling back to legacy mode. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Upgrade deps * Pin admin react-day-picker to v8 to match @flex/ui injectWorkspacePackages (enabled for pnpm deploy) installs @flex/ui as file: instead of link:, so its peer deps now resolve from the consumer. Admin declared react-day-picker@^9.14.0, which made @flex/ui/calendar.tsx type-check against v9 and fail on IconLeft/IconRight (renamed to Chevron in v9). Admin only uses the DateRange type, so downgrading to ^8.10.1 to match dashboard and @flex/ui is the smallest correct fix. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Revert injectWorkspacePackages; pass --legacy to pnpm deploy Enabling injectWorkspacePackages hard-copies internal workspace packages into node_modules. Every package under flex-ui/packages/ exports raw .ts(x) source, so once injected those .ts files live inside node_modules and the toolchain rejects them: - Node's native type stripping fails with ERR_UNSUPPORTED_NODE_MODULES_TYPE_STRIPPING (breaks vitest config loading and Playwright via pirates) - Turbopack errors with "Unknown module type" for every .ts(x) - tsc / next build OOM after traversing the injected sources Revert injectWorkspacePackages from pnpm-workspace.yaml and pass --legacy to pnpm deploy in the Dockerfile to preserve the v10+ deploy behavior the injection setting was meant to enable. Tracked in CON-701 to migrate workspace packages to compiled dist output and re-enable injection. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Raise Node heap on Check - Flex UI to fix dedupe OOM `pnpm dedupe --check` hits the V8 ~2 GB old-space ceiling while resolving the workspace on blacksmith-2vcpu runners. Set NODE_OPTIONS at the job level so typecheck and lint get the same headroom. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Disable pnpm 11 deps verify in flex-ui Docker builder `pnpm build` aborted with ERR_PNPM_ABORTED_REMOVE_MODULES_DIR_NO_TTY because pnpm 11's verify-deps-before-run saw the workspace-protocol package.json copied in after `pnpm deploy --legacy` and tried to purge an already-correct node_modules. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Disable pnpm 11 deps verify in flex-ui Docker test stage The Playwright test image copies only /opt/flex/flex-ui from the deps stage, so the `../flex-ai` entry in pnpm-workspace.yaml is unresolvable at container start. pnpm 11's verify-deps-before-run fires on `pnpm exec playwright test` and aborts with ERR_PNPM_WORKSPACE_PKG_NOT_FOUND for `@flex/ai@workspace:*`. Mirror the builder stage and skip the check. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Update Woo deps * Bump pnpm 11.3.0 -> 11.4.0 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Bump pnpm 11.4.0 -> 11.5.0 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Upgrade Woo deps * Run prettier on pnpm-workspace.yaml Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Fix wasm * Fix dedupe * Bump pnpm to 11.5.1 Picks up the v11.5.1 patch fixes — most notably preserving `integrity:` on remote-tarball lockfile entries when entries are rebuilt (the v11.4.0 hardening regression), normalized repository field for publishes, faster `pnpm audit`, and peer-resolution / workspace-state-cache fixes. Release notes: https://github.com/pnpm/pnpm/releases/tag/v11.5.1 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Dedupe * Upgrade deps --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

davidbarratt requested a review from zkochan as a code owner May 21, 2026 16:11

github-advanced-security AI found potential problems May 21, 2026

View reviewed changes

Comment thread installing/deps-installer/test/install/cyclicPeerDeterminism.ts Fixed

coderabbitai Bot requested changes May 21, 2026

View reviewed changes

Comment thread installing/deps-installer/test/install/cyclicPeerDeterminism.ts Outdated

coderabbitai Bot reviewed May 21, 2026

View reviewed changes

davidbarratt and others added 3 commits May 21, 2026 12:24

test(installing.deps-installer): rename metas to metaByName

85b78aa

Clearer name for the map keyed by package name, and avoids tripping cspell on the abbreviation "metas". Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

coderabbitai Bot approved these changes May 21, 2026

View reviewed changes

davidbarratt mentioned this pull request May 21, 2026

pnpm dedupe is behaving nondeterministically #8155

Closed

4 tasks

davidbarratt and others added 3 commits May 21, 2026 13:50

Merge branch 'pnpm:main' into fix/8155-cyclic-peer-determinism

c339374

Merge branch 'pnpm:main' into fix/8155-cyclic-peer-determinism

1c7b44c

zkochan approved these changes May 22, 2026

View reviewed changes

zkochan merged commit 3422cec into pnpm:main May 22, 2026
9 of 10 checks passed

coderabbitai Bot mentioned this pull request May 24, 2026

test(pacquet): port four peer-resolution cases from pnpm #11908

Closed

3 tasks

coderabbitai Bot mentioned this pull request May 31, 2026

fix(deps-resolver): prefer locked peer contexts during resolution by default #12083

Merged

This was referenced Jun 9, 2026

fix(resolver): stabilize transitivePeerDependencies in dependency cycles #12286

Merged

fix(deps-resolver): make shared children resolution deterministic #12362

Merged

coderabbitai Bot mentioned this pull request Jun 19, 2026

fix(resolver): make shared-children missing-peer reuse independent of resolution order #12514

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(installing.deps-resolver): deterministically order cyclic peer suffixes#11826

fix(installing.deps-resolver): deterministically order cyclic peer suffixes#11826
zkochan merged 8 commits into
pnpm:mainfrom
davidbarratt:fix/8155-cyclic-peer-determinism

davidbarratt commented May 21, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

welcome Bot commented May 21, 2026

Uh oh!

qodo-code-review Bot commented May 21, 2026

Uh oh!

coderabbitai Bot commented May 21, 2026 •

edited

Loading

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Suggested reviewers

Poem

❌ Failed checks (1 warning)

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

welcome Bot commented May 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Uh oh!

Conversation

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

Uh oh!

Summary

The bug

Root cause

The fix

Empirical validation

Regression test

Why pacquet does not have the same problem

Test plan

Summary by CodeRabbit

Uh oh!

welcome Bot commented May 21, 2026

Uh oh!

qodo-code-review Bot commented May 21, 2026

Qodo reviews are paused for this user.

Uh oh!

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

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Suggested reviewers

Poem

❌ Failed checks (1 warning)

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

welcome Bot commented May 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

davidbarratt commented May 21, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented May 21, 2026 •

edited

Loading