Skip to content

feat(pacquet/cli): accept --config.<key>=<value> overrides#11806

Merged
zkochan merged 1 commit into
mainfrom
pacquet-accept-config-flags
May 21, 2026
Merged

feat(pacquet/cli): accept --config.<key>=<value> overrides#11806
zkochan merged 1 commit into
mainfrom
pacquet-accept-config-flags

Conversation

@zkochan

@zkochan zkochan commented May 21, 2026

Copy link
Copy Markdown
Member

Summary

  • Strips pnpm's --config.<key>=<value> tokens out of argv before clap parses, and layers the values onto Config after .npmrc/pnpm-workspace.yaml have been applied — matching pnpm 11's CLI > yaml > .npmrc > defaults precedence.
  • Only registry is wired in for now; unknown keys are accepted silently (forward-compat with future pnpm keys); malformed tokens (--config.foo, --config.=value) are dropped so clap never sees them.
  • New ConfigOverrides lives in pacquet/crates/cli/src/config_overrides.rs with 5 unit tests covering the happy path, unknown-key tolerance, malformed-token dropping, last-value-wins, and the no-op case.

Context

When pnpm delegates install to pacquet (via configDependencies) it forwards the user's pnpm install flags verbatim, including pnpm's --config.<key>=<value> universal syntax (npm-conf upstream). Pacquet's clap parser previously rejected those with error: unexpected argument '--config.registry' found, which broke the e2e pnpm install --config.registry=… path on the JS side (the failing case being added in #11774pnpm/test/install/pacquet.ts).

A companion change on the JS side (filed separately) tightens runPacquet.ts's command-token detection so that a leading --config.* doesn't cause install to be re-forwarded as a positional. Both changes are needed for the JS test to flip green end-to-end; this PR is the pacquet half.

Test plan

  • cargo nextest run -p pacquet-cli (92 tests pass, including 5 new config_overrides tests)
  • cargo clippy -p pacquet-cli --all-targets --locked -- --deny warnings (clean)
  • cargo fmt --check (clean)
  • Verify the matching JS-side change lands so the pnpm install --config.registry=… e2e test in pnpm/test/install/pacquet.ts goes green end-to-end.

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

Summary by CodeRabbit

  • New Features
    • Added support for CLI configuration overrides via --config.<key>=<value> arguments, enabling users to customize configuration values directly from the command line without modifying config files.

Review Change Stack

When pnpm delegates `install` to the pacquet binary it forwards the
user's `pnpm install` flags verbatim, including pnpm's
`--config.<key>=<value>` universal syntax (handled by npm-conf upstream).
Pacquet's clap parser previously rejected those tokens with `unexpected
argument '--config.registry'`, causing the delegated install to fail.

Strip every `--config.*` token out of argv before clap parses it and
layer the values onto `Config` after `.npmrc`/yaml have been applied,
mirroring pnpm 11's `CLI > yaml > .npmrc > defaults` precedence. Only
`registry` is wired in for now (matches the immediate test gap); unknown
keys are accepted silently so a pnpm-side key pacquet hasn't ported yet
doesn't break the delegation. Malformed tokens (`--config.foo`,
`--config.=value`) are dropped on the same path so clap never sees them
either.
@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: 7d970715-b5ce-4a07-887e-0c470d5c33da

📥 Commits

Reviewing files that changed from the base of the PR and between 69f8ea8 and dcb1009.

📒 Files selected for processing (3)
  • pacquet/crates/cli/src/cli_args.rs
  • pacquet/crates/cli/src/config_overrides.rs
  • pacquet/crates/cli/src/lib.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: Run benchmark on ubuntu-latest
  • GitHub Check: Run benchmark on ubuntu-latest
  • GitHub Check: Lint and Test (macos-latest)
  • GitHub Check: Code Coverage
  • GitHub Check: Lint and Test (windows-latest)
  • GitHub Check: Lint and Test (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/cli/src/lib.rs
  • pacquet/crates/cli/src/cli_args.rs
  • pacquet/crates/cli/src/config_overrides.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/cli/src/lib.rs
  • pacquet/crates/cli/src/cli_args.rs
  • pacquet/crates/cli/src/config_overrides.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/lib.rs
  • pacquet/crates/cli/src/cli_args.rs
  • pacquet/crates/cli/src/config_overrides.rs
🔇 Additional comments (3)
pacquet/crates/cli/src/config_overrides.rs (1)

1-151: LGTM!

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

7-7: LGTM!

Also applies to: 75-80, 108-111

pacquet/crates/cli/src/lib.rs (1)

2-2: LGTM!

Also applies to: 7-7, 15-20, 26-26, 28-28


📝 Walkthrough

Walkthrough

This PR introduces a CLI-level configuration override layer that allows users to pass --config.<key>=<value> arguments to override configuration values. The new ConfigOverrides mechanism extracts these tokens from argv before clap parsing, applies them to the loaded config, and integrates with the existing CliArgs::run flow without changing the main entrypoint signature.

Changes

CLI Config Override Layer

Layer / File(s) Summary
ConfigOverrides struct and token parsing
pacquet/crates/cli/src/config_overrides.rs
Introduces ConfigOverrides struct with extract to parse --config.<key>=<value> tokens from argv and return remaining args, and apply to layer recognized overrides (currently registry) onto a Config. Internal token classification distinguishes valid, malformed, and unrelated tokens. Unit tests verify argv separation, unknown key silent handling, malformed token dropping, last-value-wins precedence, and no-op apply behavior.
CliArgs::run integration
pacquet/crates/cli/src/cli_args.rs
CliArgs::run signature updated to accept config_overrides: &ConfigOverrides and applies overrides to the loaded Config before leaking for execution. Inline documentation describes how CLI-stripped config tokens layer on top of file and default sources. Import of ConfigOverrides added.
Main entrypoint config extraction
pacquet/crates/cli/src/lib.rs
CLI main pre-processes raw process arguments via ConfigOverrides::extract, then passes filtered argv to CliArgs::parse_from instead of direct std::env::args parsing. Module imports updated to include config_overrides. Rayon pool configuration unchanged.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Suggested labels

area: config dependencies

🐰 A config override hop so neat,
CLI tokens and clap args meet,
Registry values take their place,
Last-win wins the parsing race! 🎯

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately and concisely summarizes the primary change: adding support for --config.<key>=<value> CLI overrides in the pacquet CLI.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ 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 pacquet-accept-config-flags

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 coderabbitai Bot added the area: config dependencies Changes related to configDependencies. label May 21, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Micro-Benchmark Results

Linux

group                          main                                   pr
-----                          ----                                   --
tarball/download_dependency    1.01      7.7±0.25ms   560.2 KB/sec    1.00      7.6±0.28ms   568.2 KB/sec

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.43%. Comparing base (69f8ea8) to head (dcb1009).

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #11806      +/-   ##
==========================================
+ Coverage   87.39%   87.43%   +0.04%     
==========================================
  Files         200      201       +1     
  Lines       23514    23598      +84     
==========================================
+ Hits        20550    20634      +84     
  Misses       2964     2964              

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

@github-actions

Copy link
Copy Markdown
Contributor

Integrated-Benchmark Report (Linux)

Scenario: Frozen Lockfile

Command Mean [s] Min [s] Max [s] Relative
pacquet@HEAD 2.400 ± 0.105 2.279 2.604 1.01 ± 0.05
pacquet@main 2.377 ± 0.067 2.310 2.494 1.00
pnpm 4.638 ± 0.025 4.608 4.697 1.95 ± 0.06
BENCHMARK_REPORT.json
{
  "results": [
    {
      "command": "pacquet@HEAD",
      "mean": 2.40036829344,
      "stddev": 0.1050268168315364,
      "median": 2.36897585904,
      "user": 2.8754877799999994,
      "system": 3.6314742800000004,
      "min": 2.27853064104,
      "max": 2.60403851004,
      "times": [
        2.33981549504,
        2.60403851004,
        2.41556580704,
        2.56647584304,
        2.32734825104,
        2.34096298004,
        2.38591875204,
        2.39299368904,
        2.35203296604,
        2.27853064104
      ]
    },
    {
      "command": "pacquet@main",
      "mean": 2.37670238114,
      "stddev": 0.06689583633200591,
      "median": 2.35052987454,
      "user": 2.8304281799999993,
      "system": 3.6187403800000006,
      "min": 2.31026659704,
      "max": 2.49392200704,
      "times": [
        2.33250612504,
        2.49392200704,
        2.32566927004,
        2.43570805604,
        2.34426598004,
        2.36820337204,
        2.32404801004,
        2.47564062504,
        2.31026659704,
        2.35679376904
      ]
    },
    {
      "command": "pnpm",
      "mean": 4.638177689040001,
      "stddev": 0.025402036915555325,
      "median": 4.6325438190399995,
      "user": 7.87704288,
      "system": 4.092197779999999,
      "min": 4.6076446000399995,
      "max": 4.6968055700399995,
      "times": [
        4.62686984504,
        4.62122116804,
        4.6968055700399995,
        4.64941399504,
        4.61853495804,
        4.65922256104,
        4.6076446000399995,
        4.6369765550399995,
        4.63367973804,
        4.631407900039999
      ]
    }
  ]
}

Scenario: Frozen Lockfile (Hot Cache)

Command Mean [ms] Min [ms] Max [ms] Relative
pacquet@HEAD 720.7 ± 55.0 689.9 875.7 1.00
pacquet@main 742.3 ± 55.3 708.9 868.8 1.03 ± 0.11
pnpm 2532.8 ± 123.8 2323.1 2756.8 3.51 ± 0.32
BENCHMARK_REPORT.json
{
  "results": [
    {
      "command": "pacquet@HEAD",
      "mean": 0.7207035657,
      "stddev": 0.05499984079967497,
      "median": 0.7045185462000001,
      "user": 0.40093378,
      "system": 1.5993881399999998,
      "min": 0.6899391507000001,
      "max": 0.8756594627000001,
      "times": [
        0.8756594627000001,
        0.7068872407000001,
        0.7148025217000001,
        0.7135381087000001,
        0.7050089567000001,
        0.6899391507000001,
        0.7039254707000001,
        0.7001156597000001,
        0.7040281357000001,
        0.6931309497000001
      ]
    },
    {
      "command": "pacquet@main",
      "mean": 0.7422501401000001,
      "stddev": 0.0552734612472116,
      "median": 0.7140919757000002,
      "user": 0.39151798000000004,
      "system": 1.60983604,
      "min": 0.7089095457000001,
      "max": 0.8687884647000002,
      "times": [
        0.7602359867000001,
        0.7098102817,
        0.7093322177000001,
        0.8101704147000001,
        0.7181213997000001,
        0.7089095457000001,
        0.7175089317000001,
        0.8687884647000002,
        0.7106750197000001,
        0.7089491387000001
      ]
    },
    {
      "command": "pnpm",
      "mean": 2.532829624,
      "stddev": 0.12378924192489676,
      "median": 2.5181609416999997,
      "user": 2.9862776799999997,
      "system": 2.20655244,
      "min": 2.3231411667,
      "max": 2.7568331787,
      "times": [
        2.5140825636999997,
        2.5176544837,
        2.5080767137,
        2.7568331787,
        2.5672360036999997,
        2.5632470597,
        2.3878281047,
        2.3231411667,
        2.6715295657,
        2.5186673997
      ]
    }
  ]
}

@zkochan zkochan merged commit 86bf5db into main May 21, 2026
28 checks passed
@zkochan zkochan deleted the pacquet-accept-config-flags branch May 21, 2026 08:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: config dependencies Changes related to configDependencies.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants