Skip to content

feat(pacquet): add --ignore-manifest-check to skip frozen-lockfile manifest gate#11811

Merged
zkochan merged 1 commit into
mainfrom
fix/11797
May 21, 2026
Merged

feat(pacquet): add --ignore-manifest-check to skip frozen-lockfile manifest gate#11811
zkochan merged 1 commit into
mainfrom
fix/11797

Conversation

@zkochan

@zkochan zkochan commented May 21, 2026

Copy link
Copy Markdown
Member

Summary

  • Adds --ignore-manifest-check to pacquet install. The flag gates only satisfies_package_manifest; settings-drift checks (overrides, ignoredOptionalDependencies, …) still fire. The broader --ignore-package-manifest name is reserved for a later port of pnpm's pnpm fetch semantics (which would skip linking / hoisting / pruning too).
  • Intended for the pnpm CLI's configDependencies delegation path: pnpm resolves and writes the lockfile, then hands materialization to pacquet but hasn't yet written the post-mutation package.json. With the flag, the freshness gate skips the per-importer manifest check that would otherwise reject every pnpm up / add / remove with ERR_PNPM_OUTDATED_LOCKFILE.

The matching pnpm-side change (forwarding the flag from installing/commands/src/runPacquet.ts) lands separately on the forward-install-flags-to-pacquet branch.

Refs #11797.

Test plan

  • cargo nextest run -p pacquet-cli -p pacquet-package-manager — 367 tests pass, including the new ignore_manifest_check_bypasses_manifest_freshness_gate (uses the existing drift fixture; asserts the install moves past the freshness gate instead of returning OutdatedLockfile) and ignore_manifest_check_flag_parses (CLI surface coverage).
  • cargo clippy -p pacquet-cli -p pacquet-package-manager --all-targets -- --deny warnings clean.
  • cargo fmt --all -- --check clean.

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

Summary by CodeRabbit

Release Notes

  • New Features

    • Added --ignore-manifest-check CLI flag for the install command, allowing you to skip the package manifest freshness validation check during installation.
  • Tests

    • Added test coverage for the new flag behavior and parsing.

Review Change Stack

@qodo-code-review

Copy link
Copy Markdown

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

coderabbitai Bot commented May 21, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 220703a8-f8d6-402e-9b2b-487d30a8cf6e

📥 Commits

Reviewing files that changed from the base of the PR and between 250c580 and 0249f6e.

📒 Files selected for processing (5)
  • pacquet/crates/cli/src/cli_args/install.rs
  • pacquet/crates/cli/src/cli_args/install/tests.rs
  • pacquet/crates/package-manager/src/add.rs
  • pacquet/crates/package-manager/src/install.rs
  • pacquet/crates/package-manager/src/install/tests.rs
📜 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). (7)
  • GitHub Check: Lint and Test (windows-latest)
  • GitHub Check: Lint and Test (ubuntu-latest)
  • GitHub Check: Lint and Test (macos-latest)
  • GitHub Check: Code Coverage
  • GitHub Check: Run benchmark on ubuntu-latest
  • GitHub Check: Run benchmark on ubuntu-latest
  • GitHub Check: Compile & Lint
🧰 Additional context used
📓 Path-based instructions (1)
pacquet/**/*.rs

📄 CodeRabbit inference engine (pacquet/AGENTS.md)

pacquet/**/*.rs: When porting a function that fires pnpm:<channel> events through globalLogger, logger.debug(), or streamParser.write(), mirror the call site, payload, and ordering so the reporter parses pacquet's NDJSON the same way it parses pnpm's.
Declare a newtype wrapper for branded string types. Do not collapse the brand into a plain String or &str.
If upstream always validates before construction, validate in pacquet's wrapper too. The wrapper must construct only via TryFrom<String> and/or FromStr. Do not provide an infallible public constructor.
If upstream never validates, just brand for type-safety. Expose an infallible From<String> (and From<&str> when convenient).
If upstream occasionally constructs without validation, expose from_str_unchecked as an escape hatch alongside the validating constructor.
Match upstream serde behavior for branded types that cross JSON, YAML, or INI boundaries. Use #[serde(try_from = "String")] for deserialization and #[serde(into = "String")] for serialization.
Use #[derive(derive_more::From)] and #[derive(derive_more::Into)] for mechanical conversion impls. Fall back to manual impl only when conversion needs custom logic.
String-literal unions should become enums, not newtype wrappers. Model closed sets of valid string values as enums.
Template literal types should be treated as branded strings with validation discipline from rules 2-5.
Choose owned vs. borrowed parameters to minimize copies. Widen to the most encompassing type (&Path over &PathBuf, &str over &String) when it doesn't force extra copies.
Prefer Arc::clone(&x) / Rc::clone(&x) over x.clone() for reference-counted types, so the cost is visible at the call site.
Follow Rust API Guidelines for naming conventions.
Do not use star imports inside module bodies. Write use super::{Foo, bar} instead of use super::*;. Two forms stay allowed: external-crate preludes like use rayon::prelude::*; and root-of-module re-...

Files:

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

Applied to files:

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

Applied to files:

  • pacquet/crates/package-manager/src/add.rs
  • pacquet/crates/cli/src/cli_args/install/tests.rs
  • pacquet/crates/package-manager/src/install.rs
  • pacquet/crates/cli/src/cli_args/install.rs
  • pacquet/crates/package-manager/src/install/tests.rs
🔇 Additional comments (5)
pacquet/crates/cli/src/cli_args/install.rs (1)

83-101: LGTM!

Also applies to: 163-163, 207-207

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

112-123: LGTM!

pacquet/crates/package-manager/src/install.rs (1)

67-78: LGTM!

Also applies to: 252-252, 526-544

pacquet/crates/package-manager/src/add.rs (1)

97-97: LGTM!

pacquet/crates/package-manager/src/install/tests.rs (1)

61-61: LGTM!

Also applies to: 119-119, 160-160, 230-230, 293-293, 373-373, 438-438, 523-523, 665-665, 763-763, 878-878, 998-998, 1093-1093, 1196-1196, 1243-1243, 1332-1332, 1423-1423, 1441-1500, 1546-1546, 1634-1634, 1692-1692, 1777-1777, 1852-1852, 1933-1933, 2132-2132, 2253-2253, 2350-2350, 2453-2453, 2541-2541, 2632-2632, 2720-2720, 2805-2805, 2896-2896, 2985-2985, 3080-3080, 3177-3177


📝 Walkthrough

Walkthrough

A new CLI flag --ignore-manifest-check allows pacquet to bypass manifest-lockfile freshness validation in frozen-lockfile mode. The flag is threaded through the install pipeline and conditionally skips the satisfies_package_manifest check when enabled, deferring manifest drift failures to downstream logic.

Changes

Manifest check bypass feature

Layer / File(s) Summary
Pacquet CLI flag definition and parsing
pacquet/crates/cli/src/cli_args/install.rs, pacquet/crates/cli/src/cli_args/install/tests.rs
InstallArgs gains a boolean ignore_manifest_check field with clap documentation. The field is extracted during InstallArgs::run and wired into the Install struct. A new test verifies the flag parses correctly to true when present and false when absent.
Install logic and downstream wiring
pacquet/crates/package-manager/src/install.rs, pacquet/crates/package-manager/src/add.rs
The Install struct gains the ignore_manifest_check field. During Install::run, the manifest freshness check is conditionally executed: when the flag is true, the satisfies_package_manifest check is skipped, bypassing OutdatedLockfile errors; when false, the previous behavior is preserved. The Add command explicitly sets the flag to false when invoking install post-mutation.
Test coverage for manifest check behavior
pacquet/crates/package-manager/src/install/tests.rs
All existing install tests are updated with explicit ignore_manifest_check: false to maintain default frozen-lockfile freshness validation. A new async test ignore_manifest_check_bypasses_manifest_freshness_gate verifies that setting the flag to true allows manifest/lockfile drift to bypass the freshness gate and fail with a downstream error instead of OutdatedLockfile.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related issues

  • #11797: This PR resolves the underlying issue where pnpm up fails when pacquet is declared as a configDependency and the manifest drifts (e.g., version mismatch between lockfile and package.json) before pacquet's frozen-lockfile install. The new flag allows pnpm to invoke pacquet with manifest drift tolerance during update operations.

Poem

🐰 A flag hops in to calm the storm,
When manifests and lockfiles drift and swarm,
Ignore the check, let downstream sort the fray,
Pacquet now flexes—pnpm's update way! 🚀

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: adding a --ignore-manifest-check CLI flag to skip the frozen-lockfile manifest freshness gate in pacquet.
Linked Issues check ✅ Passed The pull request implements the required feature to resolve #11797: adds --ignore-manifest-check flag to bypass the manifest freshness check, includes comprehensive tests, and maintains other integrity checks.
Out of Scope Changes check ✅ Passed All changes are directly related to implementing the --ignore-manifest-check flag feature across CLI args, package manager install logic, and corresponding tests as required by the linked issue.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

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

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/11797

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.

…nifest gate

Surfaces a narrow CLI flag on `pacquet install` that gates only
`satisfies_package_manifest`. Settings-drift checks (`overrides`,
`ignoredOptionalDependencies`, …) still fire, and the broader
`--ignore-package-manifest` name is reserved for a future port of
pnpm's `pnpm fetch` semantics (which skip linking / hoisting /
pruning too).

Intended for the pnpm CLI's `configDependencies` delegation path
(issue #11797): pnpm resolves and writes the lockfile, then hands
materialization to pacquet but hasn't yet written the post-mutation
`package.json`. With the flag set, the freshness gate skips the
per-importer manifest check that would otherwise reject every
`pnpm up` / `add` / `remove` with `ERR_PNPM_OUTDATED_LOCKFILE`. The
matching pnpm-side change to forward the flag lands separately.

Refs #11797.

---
Written by an agent (Claude Code, claude-opus-4-7).
@zkochan zkochan changed the title fix: pacquet --ignore-manifest-check skips frozen-lockfile manifest gate feat(pacquet): add --ignore-manifest-check to skip frozen-lockfile manifest gate May 21, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
pacquet/crates/package-manager/src/install/tests.rs (1)

1493-1497: ⚡ Quick win

Tighten the bypass assertion to prove the install reached downstream frozen-path work.

!matches!(..., OutdatedLockfile) is too broad; unrelated early errors would also pass this test. Assert a downstream frozen-lockfile error shape instead.

Proposed test assertion tightening
     let err = result.expect_err("bogus tarball URL must still surface a downstream error");
-    assert!(
-        !matches!(err, InstallError::OutdatedLockfile { .. }),
-        "ignore_manifest_check should bypass the freshness gate, got OutdatedLockfile: {err:?}",
-    );
+    assert!(
+        matches!(err, InstallError::FrozenLockfile(_)),
+        "expected downstream frozen-lockfile failure after bypass, got {err:?}",
+    );
🤖 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 `@pacquet/crates/package-manager/src/install/tests.rs` around lines 1493 -
1497, Replace the broad negative check that err is not
InstallError::OutdatedLockfile with a positive assertion that the failure came
from the downstream frozen-lockfile path; specifically assert that err matches
the frozen-lockfile install error variant (e.g. assert!(matches!(err,
InstallError::FrozenLockfile { .. }))) so the test proves we reached the
frozen-path logic when ignore_manifest_check is used.
🤖 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.

Nitpick comments:
In `@pacquet/crates/package-manager/src/install/tests.rs`:
- Around line 1493-1497: Replace the broad negative check that err is not
InstallError::OutdatedLockfile with a positive assertion that the failure came
from the downstream frozen-lockfile path; specifically assert that err matches
the frozen-lockfile install error variant (e.g. assert!(matches!(err,
InstallError::FrozenLockfile { .. }))) so the test proves we reached the
frozen-path logic when ignore_manifest_check is used.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 35caace8-c07e-4853-a0bb-bc1bfd21963d

📥 Commits

Reviewing files that changed from the base of the PR and between 3a63928 and 250c580.

📒 Files selected for processing (7)
  • .changeset/fix-pacquet-outdated-lockfile-on-update.md
  • installing/commands/src/runPacquet.ts
  • pacquet/crates/cli/src/cli_args/install.rs
  • pacquet/crates/cli/src/cli_args/install/tests.rs
  • pacquet/crates/package-manager/src/add.rs
  • pacquet/crates/package-manager/src/install.rs
  • pacquet/crates/package-manager/src/install/tests.rs
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
  • GitHub Check: Run benchmark on ubuntu-latest
  • GitHub Check: Run benchmark on ubuntu-latest
  • GitHub Check: Lint and Test (windows-latest)
  • GitHub Check: Lint and Test (ubuntu-latest)
  • GitHub Check: Analyze (javascript)
  • GitHub Check: Lint and Test (macos-latest)
  • GitHub Check: Dylint
  • GitHub Check: Code Coverage
  • GitHub Check: Compile & Lint
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{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/commands/src/runPacquet.ts
pacquet/**/*.rs

📄 CodeRabbit inference engine (pacquet/AGENTS.md)

pacquet/**/*.rs: When porting a function that fires pnpm:<channel> events through globalLogger, logger.debug(), or streamParser.write(), mirror the call site, payload, and ordering so the reporter parses pacquet's NDJSON the same way it parses pnpm's.
Declare a newtype wrapper for branded string types. Do not collapse the brand into a plain String or &str.
If upstream always validates before construction, validate in pacquet's wrapper too. The wrapper must construct only via TryFrom<String> and/or FromStr. Do not provide an infallible public constructor.
If upstream never validates, just brand for type-safety. Expose an infallible From<String> (and From<&str> when convenient).
If upstream occasionally constructs without validation, expose from_str_unchecked as an escape hatch alongside the validating constructor.
Match upstream serde behavior for branded types that cross JSON, YAML, or INI boundaries. Use #[serde(try_from = "String")] for deserialization and #[serde(into = "String")] for serialization.
Use #[derive(derive_more::From)] and #[derive(derive_more::Into)] for mechanical conversion impls. Fall back to manual impl only when conversion needs custom logic.
String-literal unions should become enums, not newtype wrappers. Model closed sets of valid string values as enums.
Template literal types should be treated as branded strings with validation discipline from rules 2-5.
Choose owned vs. borrowed parameters to minimize copies. Widen to the most encompassing type (&Path over &PathBuf, &str over &String) when it doesn't force extra copies.
Prefer Arc::clone(&x) / Rc::clone(&x) over x.clone() for reference-counted types, so the cost is visible at the call site.
Follow Rust API Guidelines for naming conventions.
Do not use star imports inside module bodies. Write use super::{Foo, bar} instead of use super::*;. Two forms stay allowed: external-crate preludes like use rayon::prelude::*; and root-of-module re-...

Files:

  • pacquet/crates/cli/src/cli_args/install/tests.rs
  • pacquet/crates/package-manager/src/add.rs
  • pacquet/crates/package-manager/src/install.rs
  • pacquet/crates/cli/src/cli_args/install.rs
  • pacquet/crates/package-manager/src/install/tests.rs
🧠 Learnings (3)
📚 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/commands/src/runPacquet.ts
📚 Learning: 2026-05-20T19:40:55.051Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11774
File: pacquet/crates/resolving-deps-resolver/src/resolve_peers.rs:0-0
Timestamp: 2026-05-20T19:40:55.051Z
Learning: In the pacquet Rust code, ensure the semver implementation uses the `node-semver` crate (not `nodejs-semver`). `node-semver`’s public API does not include a `satisfies_with_prerelease`-style method; prerelease-tolerant matching should be implemented inline by first calling `Range::satisfies`, and when it rejects a prerelease version, retry matching against a stripped `MAJOR.MINOR.PATCH` base of the prerelease version.

Applied to files:

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

Applied to files:

  • pacquet/crates/cli/src/cli_args/install/tests.rs
  • pacquet/crates/package-manager/src/add.rs
  • pacquet/crates/package-manager/src/install.rs
  • pacquet/crates/cli/src/cli_args/install.rs
  • pacquet/crates/package-manager/src/install/tests.rs
🔇 Additional comments (7)
.changeset/fix-pacquet-outdated-lockfile-on-update.md (1)

5-5: LGTM!

installing/commands/src/runPacquet.ts (1)

77-92: LGTM!

pacquet/crates/cli/src/cli_args/install.rs (1)

83-101: LGTM!

Also applies to: 163-163, 207-207

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

112-123: LGTM!

pacquet/crates/package-manager/src/install.rs (1)

67-78: LGTM!

Also applies to: 252-253, 526-544

pacquet/crates/package-manager/src/add.rs (1)

97-97: LGTM!

pacquet/crates/package-manager/src/install/tests.rs (1)

61-61: LGTM!

Also applies to: 119-119, 160-160, 230-230, 293-293, 373-373, 438-438, 523-523, 665-665, 763-763, 878-878, 998-998, 1093-1093, 1196-1196, 1243-1243, 1332-1332, 1423-1423, 1441-1492, 1499-1500, 1546-1546, 1634-1634, 1692-1692, 1777-1777, 1852-1852, 1933-1933, 2132-2132, 2253-2253, 2350-2350, 2453-2453, 2541-2541, 2632-2632, 2720-2720, 2805-2805, 2896-2896, 2985-2985, 3080-3080, 3177-3177

zkochan added a commit that referenced this pull request May 21, 2026
`pnpm up` / `add` / `remove` were aborting with
`pacquet_package_manager::outdated_lockfile` whenever pacquet was
declared in `configDependencies`. After resolving and writing the
updated lockfile, pnpm hands materialization off to pacquet but
hasn't yet written the post-mutation `package.json` — that write
happens after `mutateModules` returns. Pacquet's frozen-lockfile
freshness gate then saw the new lockfile paired with the
pre-mutation manifest and refused to install.

Pass pacquet's new `--ignore-manifest-check` flag (pacquet PR #11811)
on every delegation. The flag is narrow: it only skips
`satisfies_package_manifest`. Settings drift like `overrides` is
still enforced, and pnpm already re-validated the lockfile before
delegating, so re-checking the manifest here was redundant work that
only ever fired false positives on the mutate-then-materialize path.

Requires a pacquet release that ships the flag; bump
`PACQUET_VERSION` in `pnpm/test/install/pacquet.ts` once it does, or
the existing e2e tests will fail against pacquet 0.2.2-9 (which
doesn't recognize the flag and clap would reject).

Closes #11797.

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

Copy link
Copy Markdown
Contributor

Micro-Benchmark Results

Linux

group                          main                                   pr
-----                          ----                                   --
tarball/download_dependency    1.04      5.2±0.38ms   828.5 KB/sec    1.00      5.0±0.35ms   861.9 KB/sec

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.47%. Comparing base (69f8ea8) to head (0249f6e).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #11811      +/-   ##
==========================================
+ Coverage   87.39%   87.47%   +0.07%     
==========================================
  Files         200      202       +2     
  Lines       23514    23704     +190     
==========================================
+ Hits        20550    20735     +185     
- Misses       2964     2969       +5     

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

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

@zkochan zkochan merged commit 0dd1ec4 into main May 21, 2026
28 checks passed
@zkochan zkochan deleted the fix/11797 branch May 21, 2026 09:17
zkochan added a commit that referenced this pull request May 21, 2026
`pnpm up` / `add` / `remove` were aborting with
`pacquet_package_manager::outdated_lockfile` whenever pacquet was
declared in `configDependencies`. After resolving and writing the
updated lockfile, pnpm hands materialization off to pacquet but
hasn't yet written the post-mutation `package.json` — that write
happens after `mutateModules` returns. Pacquet's frozen-lockfile
freshness gate then saw the new lockfile paired with the
pre-mutation manifest and refused to install.

Pass pacquet's new `--ignore-manifest-check` flag (pacquet PR #11811)
on every delegation. The flag is narrow: it only skips
`satisfies_package_manifest`. Settings drift like `overrides` is
still enforced, and pnpm already re-validated the lockfile before
delegating, so re-checking the manifest here was redundant work that
only ever fired false positives on the mutate-then-materialize path.

Requires a pacquet release that ships the flag; bump
`PACQUET_VERSION` in `pnpm/test/install/pacquet.ts` once it does, or
the existing e2e tests will fail against pacquet 0.2.2-9 (which
doesn't recognize the flag and clap would reject).

Closes #11797.

---
Written by an agent (Claude Code, claude-opus-4-7).
zkochan added a commit that referenced this pull request May 21, 2026
…1781)

* fix(installing.commands): forward `pnpm install` flags to pacquet

When the install engine is delegated to pacquet via configDependencies,
pnpm hard-coded the args to `install --frozen-lockfile --reporter=ndjson`
and silently dropped the user's other CLI flags. `pnpm install --no-runtime`
therefore still installed the workspace's runtime devDependency, clobbering
the Node version the surrounding tooling had set up — visible as the
`Verify Node version` failure on PR #11765 where setup-pnpm provisions
Node 24.0.0 but pacquet then materializes node 24.6.0.

Pacquet's `install` subcommand already mirrors pnpm's surface for the
common flags (`--no-runtime`, `--prod`, `--dev`, `--no-optional`,
`--node-linker`, `--offline`, `--prefer-offline`, `--cpu`/`--os`/`--libc`).
Forward the user's argv verbatim when the command is `install`/`i`;
`add`/`update`/`dedupe` still don't forward — their flag surfaces don't
line up with pacquet's `install`.

* fix(installing.commands): pass --ignore-manifest-check to pacquet

`pnpm up` / `add` / `remove` were aborting with
`pacquet_package_manager::outdated_lockfile` whenever pacquet was
declared in `configDependencies`. After resolving and writing the
updated lockfile, pnpm hands materialization off to pacquet but
hasn't yet written the post-mutation `package.json` — that write
happens after `mutateModules` returns. Pacquet's frozen-lockfile
freshness gate then saw the new lockfile paired with the
pre-mutation manifest and refused to install.

Pass pacquet's new `--ignore-manifest-check` flag (pacquet PR #11811)
on every delegation. The flag is narrow: it only skips
`satisfies_package_manifest`. Settings drift like `overrides` is
still enforced, and pnpm already re-validated the lockfile before
delegating, so re-checking the manifest here was redundant work that
only ever fired false positives on the mutate-then-materialize path.

Requires a pacquet release that ships the flag; bump
`PACQUET_VERSION` in `pnpm/test/install/pacquet.ts` once it does, or
the existing e2e tests will fail against pacquet 0.2.2-9 (which
doesn't recognize the flag and clap would reject).

Closes #11797.

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

* fix: update pacquet in tests

* fix(installing.commands): strip positionals + always-injected flags when forwarding to pacquet

`collectForwardedFlags` checked `argv[0] === 'install'` to find the
command token to strip. Any global flag the user typed before `install`
(e.g. `--config.registry=...` in the e2e test) shifted the token out
of position, so the function returned the full argv and pacquet saw
`install` twice — `error: unexpected argument 'install' found`.

Use the parsed argv that `@pnpm/cli.parse-cli-args` already produced:
`remain` lists positionals (the `install`/`i` token and nothing else
on this code path, since `isInstallCommand` is only true when no
package params are present), and `original` preserves the user's
exact tokens. Drop positionals + the flags we always inject
(`--reporter=ndjson`, `--frozen-lockfile`, `--ignore-manifest-check`)
so clap doesn't reject duplicates either.

`original` over `cooked` deliberately: nopt's `cooked` splits
`--key=value` into two tokens, which would break pacquet's
`--config.<key>=<value>` parser (it requires the `=` form).

* fix(installing.commands): make argv.cooked/remain optional on InstallCommandOptions

Widening these to required broke test fixtures elsewhere (publish/pack/
deprecate/dist-tag/deploy) that construct minimal `argv: { original }`
options for code paths that never reach pacquet. Only the pacquet
delegation actually reads `remain`, so make the two new fields optional
on the shared options type and supply a default at the runPacquet call
site. The runtime path through main.ts already populates all three.

* fix(installing.commands): strip any user-supplied --reporter when forwarding to pacquet

Pacquet's `--reporter` is a clap value option with last-value-wins
semantics, so `pnpm install --reporter=silent` (or
`--reporter silent` two-token form) reached pacquet and overrode
the `--reporter=ndjson` pnpm injects, breaking the NDJSON-to-
streamParser plumbing the default reporter depends on. The previous
filter only matched the exact `--reporter=ndjson` token.

Walk argv with a lookahead so both `--reporter=<value>` and
`--reporter <value>` are dropped without consuming an adjacent flag.

* fix(installing.commands): drop negated/value forms of always-injected flags

`collectForwardedFlags` only matched the exact positive tokens
`--frozen-lockfile` and `--ignore-manifest-check`, so a user typing
`pnpm install --no-frozen-lockfile` (or `--frozen-lockfile=false`)
forwarded the negation to pacquet, which then saw both our injected
`--frozen-lockfile` and the user's `--no-frozen-lockfile` and crashed
clap with "unexpected argument".

Match every shape the user can write the same flag in: positive,
`--no-` negated, and any `=value` form. Can't blindly strip `--no-`
either way — pacquet has flags whose literal name starts with `no-`
(`--no-runtime`, `--no-optional`); those must still forward.

The user's `--no-frozen-lockfile` intent is honored upstream — pnpm
did a fresh resolve before delegating; pacquet's role here is just
lockfile-driven materialization, which is always frozen.

* fix(installing.commands): match positionals by index, hide reporter from dropped-flags warning

`collectForwardedFlags` matched positionals via `new Set(argv.remain)`,
which strips by value: a flag value that happened to equal a
positional token (e.g. `pnpm install --node-linker install`) was
wrongly dropped from the forwarded list, costing pacquet the value
of `--node-linker`. Walk `argv.original` with a subsequence pointer
into `argv.remain` so only the actual positional indexes get skipped.

`collectDroppedFlags` still surfaced `--reporter foo` / `--reporter=foo`
in the "may not be honored" warning on `add`/`update`/`dedupe`, but
pnpm honors reporter selection itself before delegation — so the
warning was misleading. Route both helpers through the same
`isAlwaysInjected` check and consume `--reporter` and its value the
same way `collectForwardedFlags` already does.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants