Skip to content

fix(pacquet): read moved pnpm sources from their pnpm11/ locations#12540

Merged
zkochan merged 2 commits into
mainfrom
fix/pacquet-pnpm11-cli-path
Jun 20, 2026
Merged

fix(pacquet): read moved pnpm sources from their pnpm11/ locations#12540
zkochan merged 2 commits into
mainfrom
fix/pacquet-pnpm11-cli-path

Conversation

@zkochan

@zkochan zkochan commented Jun 20, 2026

Copy link
Copy Markdown
Member

Summary

#12537 moved the TypeScript pnpm CLI into a pnpm11/ directory, but two pacquet code paths still read the old top-level locations, so they fail on main:

  • pacquet-cli's pnpm_version_from read pnpm/package.json (now pnpm11/pnpm/package.json), so current_source_pnpm_version() returned None and package_manager_to_sync_preserves_dev_engine_specifier failed.
  • pacquet-config's pnpm_default_parity contract tests read config/reader/src/index.ts (now pnpm11/config/reader/src/index.ts), so both parity tests failed with a missing-file panic.

Point both at their new pnpm11/ locations.

Squash Commit Body

pnpm/pnpm#12537 moved the TypeScript pnpm CLI under `pnpm11/`. Two pacquet
paths into the TS tree weren't updated:

- pacquet-cli's `pnpm_version_from` read `pnpm/package.json`, so
  `current_source_pnpm_version` returned `None` and
  `package_manager_to_sync` (and its test) failed.
- pacquet-config's `pnpm_default_parity` contract tests read
  `config/reader/src/index.ts`, panicking on the missing file.

Read both from `pnpm11/...` instead.

Checklist

  • The change is implemented in both the TypeScript CLI and the Rust
    pacquet/ port, or the description notes what still needs porting.
  • Added or updated tests.

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

Summary by CodeRabbit

  • Improvements
    • Updated pnpm version detection to reference the correct package location within the workspace, ensuring the extracted version matches the expected pnpm installation.
  • Tests
    • Refreshed the contract test setup so it reads pnpm’s default options from the correct TypeScript source path.

#12537 moved the TypeScript pnpm CLI under `pnpm11/`, so
`pnpm_version_from` could no longer find `pnpm/package.json`;
`current_source_pnpm_version` then returned `None` and
`package_manager_to_sync` (and its test) failed.

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

coderabbitai Bot commented Jun 20, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 6737b37c-220b-409f-9c18-ac752c12e3f8

📥 Commits

Reviewing files that changed from the base of the PR and between 21d0c4f and 35c4eb0.

📒 Files selected for processing (1)
  • pacquet/crates/config/src/pnpm_default_parity.rs

📝 Walkthrough

Walkthrough

Two single-line changes align paths to pnpm11: pnpm_version_from reads the pnpm version from <root>/pnpm11/pnpm/package.json instead of <root>/pnpm/package.json, and the config contract test reads pnpm's defaultOptions from .../pnpm11/config/reader/src/index.ts instead of .../config/reader/src/index.ts.

Changes

pnpm11 path alignment

Layer / File(s) Summary
pnpm version manifest path update
pacquet/crates/cli/src/cli_args.rs
pnpm_version_from now reads the pnpm version from pnpm11/pnpm/package.json under the workspace root instead of pnpm/package.json.
pnpm default options contract test path update
pacquet/crates/config/src/pnpm_default_parity.rs
read_pnpm_default_options() test helper now constructs paths pointing to .../pnpm11/config/reader/src/index.ts instead of .../config/reader/src/index.ts when reading pnpm's default options source at runtime.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Possibly related PRs

  • pnpm/pnpm#12128: Aligns pacquet's pnpm version and config contract test paths to read from pnpm11 locations for version reading and default options validation.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/pacquet-pnpm11-cli-path

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

Copy link
Copy Markdown

PR Summary by Qodo

Fix pnpm CLI version lookup for pnpm11 directory layout
🐞 Bug fix 🕐 Less than 5 minutes

Grey Divider

Description

• Fix pnpm source version detection after pnpm CLI moved under pnpm11/.
• Unblocks package_manager_to_sync from returning None due to missing manifest.
• Restores CLI test expectations that depend on detecting the bundled pnpm version.
Diagram

graph TD
  A["package_manager_to_sync"] --> B["current_source_pnpm_version"] --> C["pnpm_version_from"] --> D[/"pnpm11/pnpm/package.json"/]
  D --> E["read_manifest_json"] --> F["Extract version"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Dual-path lookup (pnpm11 first, then legacy pnpm/)
  • ➕ Backward-compatible with older repo layouts and forks that still use pnpm/package.json.
  • ➕ Reduces risk if tooling is used against mixed pnpm versions/branches.
  • ➖ Slightly more code/branching in version discovery.
  • ➖ May mask repository layout regressions by silently falling back.
2. Centralize pnpm manifest path as a constant/config
  • ➕ Avoids future breakage from directory moves by changing one definition.
  • ➕ Makes intent explicit and easier to test.
  • ➖ More indirection for a simple lookup.
  • ➖ Doesn’t help if the path changes without updating the constant.

Recommendation: Current fix is appropriate for main if the repository now standardizes on pnpm11/pnpm/package.json. If pacquet is expected to work across older branches or downstream forks, consider a dual-path lookup (pnpm11 → legacy pnpm) to maintain compatibility while still preferring the new layout.

Files changed (1) +1 / -1

Bug fix (1) +1 / -1
cli_args.rsUpdate pnpm manifest path to 'pnpm11/pnpm/package.json' +1/-1

Update pnpm manifest path to 'pnpm11/pnpm/package.json'

• Adjusts 'pnpm_version_from' to read the pnpm CLI manifest from the new 'pnpm11/' directory layout. This restores correct 'current_source_pnpm_version()' detection and prevents downstream version-dependent logic from returning 'None'.

pacquet/crates/cli/src/cli_args.rs

@github-actions github-actions Bot added the reviewed: coderabbit CodeRabbit submitted an approving review label Jun 20, 2026
@qodo-free-for-open-source-projects

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

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (0) 📜 Skill insights (0)

Grey Divider


Remediation recommended

1. Missing legacy pnpm path 🐞 Bug ≡ Correctness
Description
pnpm_version_from() now only reads pnpm11/pnpm/package.json, so it fails to detect the source pnpm
version in older pnpm checkouts where the CLI manifest is pnpm/package.json. In that case,
package_manager_to_sync() can fall back to returning None (not syncing) for range specifiers,
changing install behavior and potentially breaking sync logic.
Code

pacquet/crates/cli/src/cli_args.rs[R646-648]

fn pnpm_version_from(root_dir: &Path) -> Option<String> {
-    let path = root_dir.join("pnpm").join("package.json");
+    let path = root_dir.join("pnpm11").join("pnpm").join("package.json");
 let value = read_manifest_json(&path).ok()??;
Evidence
The changed function hard-codes the pnpm11 manifest path; when that file doesn’t exist,
pnpm_version_from() returns None. package_manager_to_sync() relies on pnpm_version_from(root_dir) as
the runtime fallback when current_source_pnpm_version() is absent, and its final fallback requires
an exact version (ranges like ">=0.0.0" yield None), so failure to find the source manifest can
change behavior to ‘no sync’. Repo tooling still recognizes the historical pnpm/package.json layout,
indicating older checkouts exist and are expected.

pacquet/crates/cli/src/cli_args.rs[528-553]
pacquet/crates/cli/src/cli_args.rs[641-650]
pacquet/tasks/integrated-benchmark/src/verify.rs[46-61]

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

## Issue description
`pnpm_version_from()` is used as a runtime fallback to detect the pnpm source version, but it now only checks `pnpm11/pnpm/package.json`. This breaks compatibility with older pnpm source checkouts (and any tooling that still uses the historical `pnpm/package.json` location), causing version detection to fail and `package_manager_to_sync()` to skip syncing when the manifest’s desired pnpm version is a range.
## Issue Context
- `package_manager_to_sync()` first tries `current_source_pnpm_version()` and then falls back to `pnpm_version_from(root_dir)`.
- Older pnpm monorepo layouts used `pnpm/package.json` as the CLI manifest path.
## Fix Focus Areas
- pacquet/crates/cli/src/cli_args.rs[641-650]
### Suggested change
In `pnpm_version_from(root_dir)`, attempt to read `pnpm11/pnpm/package.json` first, and if not found (or returns `Ok(None)`), fall back to `pnpm/package.json`. Return the first successfully-parsed `version` string.

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



Informational

2. Stale path in panics 🐞 Bug ⚙ Maintainability
Description
read_pnpm_default_options() now reads pnpm11/config/reader/src/index.ts but its panic strings
still mention config/reader/src/index.ts, which will misdirect debugging when this contract test
fails. This inconsistency was introduced by the path change in this PR.
Code

pacquet/crates/config/src/pnpm_default_parity.rs[R232-235]

+    let path = concat!(env!("CARGO_MANIFEST_DIR"), "/../../../pnpm11/config/reader/src/index.ts");
   let src = std::fs::read_to_string(path).unwrap_or_else(|err| {
       panic!(
           "read pnpm config-reader source at {path}: {err}. \
Evidence
The code now reads the TypeScript source from .../pnpm11/config/reader/src/index.ts, but the panic
messages in the same function still explicitly reference config/reader/src/index.ts, which is no
longer the path being read.

pacquet/crates/config/src/pnpm_default_parity.rs[228-250]

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

## Issue description
`read_pnpm_default_options()` was updated to read from `pnpm11/config/reader/src/index.ts`, but the panic/error strings still refer to the old `config/reader/src/index.ts` path.
### Issue Context
This is a contract test helper; when it fails, the current messages point engineers at the wrong location.
### Fix Focus Areas
- pacquet/crates/config/src/pnpm_default_parity.rs[232-250]
### Suggested fix
- Update the panic strings to reference `pnpm11/config/reader/src/index.ts`.
- Prefer using the already-available `{path}` variable in error messages (so future moves only require updating the `path` constant).

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


Grey Divider

Previous review results

Review updated until commit 35c4eb0

Results up to commit 35c4eb0


🐞 Bugs (2) 📘 Rule violations (0) 📜 Skill insights (0)


Remediation recommended
1. Missing legacy pnpm path 🐞 Bug ≡ Correctness
Description
pnpm_version_from() now only reads pnpm11/pnpm/package.json, so it fails to detect the source pnpm
version in older pnpm checkouts where the CLI manifest is pnpm/package.json. In that case,
package_manager_to_sync() can fall back to returning None (not syncing) for range specifiers,
changing install behavior and potentially breaking sync logic.
Code

pacquet/crates/cli/src/cli_args.rs[R646-648]

fn pnpm_version_from(root_dir: &Path) -> Option<String> {
-    let path = root_dir.join("pnpm").join("package.json");
+    let path = root_dir.join("pnpm11").join("pnpm").join("package.json");
  let value = read_manifest_json(&path).ok()??;
Evidence
The changed function hard-codes the pnpm11 manifest path; when that file doesn’t exist,
pnpm_version_from() returns None. package_manager_to_sync() relies on pnpm_version_from(root_dir) as
the runtime fallback when current_source_pnpm_version() is absent, and its final fallback requires
an exact version (ranges like ">=0.0.0" yield None), so failure to find the source manifest can
change behavior to ‘no sync’. Repo tooling still recognizes the historical pnpm/package.json layout,
indicating older checkouts exist and are expected.

pacquet/crates/cli/src/cli_args.rs[528-553]
pacquet/crates/cli/src/cli_args.rs[641-650]
pacquet/tasks/integrated-benchmark/src/verify.rs[46-61]

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

## Issue description
`pnpm_version_from()` is used as a runtime fallback to detect the pnpm source version, but it now only checks `pnpm11/pnpm/package.json`. This breaks compatibility with older pnpm source checkouts (and any tooling that still uses the historical `pnpm/package.json` location), causing version detection to fail and `package_manager_to_sync()` to skip syncing when the manifest’s desired pnpm version is a range.
## Issue Context
- `package_manager_to_sync()` first tries `current_source_pnpm_version()` and then falls back to `pnpm_version_from(root_dir)`.
- Older pnpm monorepo layouts used `pnpm/package.json` as the CLI manifest path.
## Fix Focus Areas
- pacquet/crates/cli/src/cli_args.rs[641-650]
### Suggested change
In `pnpm_version_from(root_dir)`, attempt to read `pnpm11/pnpm/package.json` first, and if not found (or returns `Ok(None)`), fall back to `pnpm/package.json`. Return the first successfully-parsed `version` string.

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



Informational
2. Stale path in panics 🐞 Bug ⚙ Maintainability ⭐ New
Description
read_pnpm_default_options() now reads pnpm11/config/reader/src/index.ts but its panic strings
still mention config/reader/src/index.ts, which will misdirect debugging when this contract test
fails. This inconsistency was introduced by the path change in this PR.
Code

pacquet/crates/config/src/pnpm_default_parity.rs[R232-235]

+    let path = concat!(env!("CARGO_MANIFEST_DIR"), "/../../../pnpm11/config/reader/src/index.ts");
    let src = std::fs::read_to_string(path).unwrap_or_else(|err| {
        panic!(
            "read pnpm config-reader source at {path}: {err}. \
Evidence
The code now reads the TypeScript source from .../pnpm11/config/reader/src/index.ts, but the panic
messages in the same function still explicitly reference config/reader/src/index.ts, which is no
longer the path being read.

pacquet/crates/config/src/pnpm_default_parity.rs[228-250]

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

### Issue description
`read_pnpm_default_options()` was updated to read from `pnpm11/config/reader/src/index.ts`, but the panic/error strings still refer to the old `config/reader/src/index.ts` path.

### Issue Context
This is a contract test helper; when it fails, the current messages point engineers at the wrong location.

### Fix Focus Areas
- pacquet/crates/config/src/pnpm_default_parity.rs[232-250]

### Suggested fix
- Update the panic strings to reference `pnpm11/config/reader/src/index.ts`.
- Prefer using the already-available `{path}` variable in error messages (so future moves only require updating the `path` constant).

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


Results up to commit 21d0c4f


🐞 Bugs (1) 📘 Rule violations (0) 📜 Skill insights (0)


Remediation recommended
1. Missing legacy pnpm path 🐞 Bug ≡ Correctness
Description
pnpm_version_from() now only reads pnpm11/pnpm/package.json, so it fails to detect the source pnpm
version in older pnpm checkouts where the CLI manifest is pnpm/package.json. In that case,
package_manager_to_sync() can fall back to returning None (not syncing) for range specifiers,
changing install behavior and potentially breaking sync logic.
Code

pacquet/crates/cli/src/cli_args.rs[R646-648]

fn pnpm_version_from(root_dir: &Path) -> Option<String> {
-    let path = root_dir.join("pnpm").join("package.json");
+    let path = root_dir.join("pnpm11").join("pnpm").join("package.json");
   let value = read_manifest_json(&path).ok()??;
Evidence
The changed function hard-codes the pnpm11 manifest path; when that file doesn’t exist,
pnpm_version_from() returns None. package_manager_to_sync() relies on pnpm_version_from(root_dir) as
the runtime fallback when current_source_pnpm_version() is absent, and its final fallback requires
an exact version (ranges like ">=0.0.0" yield None), so failure to find the source manifest can
change behavior to ‘no sync’. Repo tooling still recognizes the historical pnpm/package.json layout,
indicating older checkouts exist and are expected.

pacquet/crates/cli/src/cli_args.rs[528-553]
pacquet/crates/cli/src/cli_args.rs[641-650]
pacquet/tasks/integrated-benchmark/src/verify.rs[46-61]

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

## Issue description
`pnpm_version_from()` is used as a runtime fallback to detect the pnpm source version, but it now only checks `pnpm11/pnpm/package.json`. This breaks compatibility with older pnpm source checkouts (and any tooling that still uses the historical `pnpm/package.json` location), causing version detection to fail and `package_manager_to_sync()` to skip syncing when the manifest’s desired pnpm version is a range.
## Issue Context
- `package_manager_to_sync()` first tries `current_source_pnpm_version()` and then falls back to `pnpm_version_from(root_dir)`.
- Older pnpm monorepo layouts used `pnpm/package.json` as the CLI manifest path.
## Fix Focus Areas
- pacquet/crates/cli/src/cli_args.rs[641-650]
### Suggested change
In `pnpm_version_from(root_dir)`, attempt to read `pnpm11/pnpm/package.json` first, and if not found (or returns `Ok(None)`), fall back to `pnpm/package.json`. Return the first successfully-parsed `version` string.

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


Results up to commit 21d0c4f


🐞 Bugs (1) 📘 Rule violations (0) 📜 Skill insights (0)


Remediation recommended
1. Missing legacy pnpm path 🐞 Bug ≡ Correctness
Description
pnpm_version_from() now only reads pnpm11/pnpm/package.json, so it fails to detect the source pnpm
version in older pnpm checkouts where the CLI manifest is pnpm/package.json. In that case,
package_manager_to_sync() can fall back to returning None (not syncing) for range specifiers,
changing install behavior and potentially breaking sync logic.
Code

pacquet/crates/cli/src/cli_args.rs[R646-648]

fn pnpm_version_from(root_dir: &Path) -> Option<String> {
-    let path = root_dir.join("pnpm").join("package.json");
+    let path = root_dir.join("pnpm11").join("pnpm").join("package.json");
    let value = read_manifest_json(&path).ok()??;
Evidence
The changed function hard-codes the pnpm11 manifest path; when that file doesn’t exist,
pnpm_version_from() returns None. package_manager_to_sync() relies on pnpm_version_from(root_dir) as
the runtime fallback when current_source_pnpm_version() is absent, and its final fallback requires
an exact version (ranges like ">=0.0.0" yield None), so failure to find the source manifest can
change behavior to ‘no sync’. Repo tooling still recognizes the historical pnpm/package.json layout,
indicating older checkouts exist and are expected.

pacquet/crates/cli/src/cli_args.rs[528-553]
pacquet/crates/cli/src/cli_args.rs[641-650]
pacquet/tasks/integrated-benchmark/src/verify.rs[46-61]

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

## Issue description
`pnpm_version_from()` is used as a runtime fallback to detect the pnpm source version, but it now only checks `pnpm11/pnpm/package.json`. This breaks compatibility with older pnpm source checkouts (and any tooling that still uses the historical `pnpm/package.json` location), causing version detection to fail and `package_manager_to_sync()` to skip syncing when the manifest’s desired pnpm version is a range.

## Issue Context
- `package_manager_to_sync()` first tries `current_source_pnpm_version()` and then falls back to `pnpm_version_from(root_dir)`.
- Older pnpm monorepo layouts used `pnpm/package.json` as the CLI manifest path.

## Fix Focus Areas
- pacquet/crates/cli/src/cli_args.rs[641-650]

### Suggested change
In `pnpm_version_from(root_dir)`, attempt to read `pnpm11/pnpm/package.json` first, and if not found (or returns `Ok(None)`), fall back to `pnpm/package.json`. Return the first successfully-parsed `version` string.

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


Qodo Logo

Comment thread pacquet/crates/cli/src/cli_args.rs
@qodo-free-for-open-source-projects

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 21d0c4f

@github-actions

github-actions Bot commented Jun 20, 2026

Copy link
Copy Markdown
Contributor

Micro-Benchmark Results

Linux

group                          main                                   pr
-----                          ----                                   --
tarball/download_dependency    1.00      7.8±0.11ms   554.4 KB/sec    1.00      7.8±0.32ms   556.0 KB/sec

The `pnpm_default_parity` contract tests read pnpm's `defaultOptions` from
`config/reader/src/index.ts` in the TypeScript tree, which moved under
`pnpm11/` in #12537. Point the path at the new location so the
tests stop failing with a missing-file panic.

Written by an agent (Claude Code, claude-opus-4-8).
@zkochan zkochan changed the title fix(pacquet): read the pnpm CLI manifest from its pnpm11/ location fix(pacquet): read moved pnpm sources from their pnpm11/ locations Jun 20, 2026
@zkochan zkochan merged commit 8c3a372 into main Jun 20, 2026
6 of 7 checks passed
@zkochan zkochan deleted the fix/pacquet-pnpm11-cli-path branch June 20, 2026 13:27
@zkochan zkochan mentioned this pull request Jun 20, 2026
@qodo-free-for-open-source-projects

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 35c4eb0

1 similar comment
@qodo-free-for-open-source-projects

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 35c4eb0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

product: pacquet reviewed: coderabbit CodeRabbit submitted an approving review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant