Skip to content

feat(config): make network settings configurable in pacquet#12047

Merged
zkochan merged 6 commits into
mainfrom
network-settings
May 29, 2026
Merged

feat(config): make network settings configurable in pacquet#12047
zkochan merged 6 commits into
mainfrom
network-settings

Conversation

@zkochan

@zkochan zkochan commented May 28, 2026

Copy link
Copy Markdown
Member

What

Ports the Network settings group from #12042 to pacquet — making the previously hardcoded network-client constants user-configurable, matching pnpm's config surface.

Setting Default yaml PNPM_CONFIG_* CLI
networkConcurrency min(64, max(cores*3, 16)) NETWORK_CONCURRENCY --network-concurrency
fetchTimeout 60 000 ms FETCH_TIMEOUT --fetch-timeout
userAgent pnpm/<version> npm/? node/? <platform> <arch> USER_AGENT --user-agent
npmrcAuthFile ~/.npmrc NPMRC_AUTH_FILE / USERCONFIG / npm_config_userconfig --npmrc-auth-file (alias --userconfig)

How

  • pacquet-network: new NetworkSettings struct (concurrency, timeout, UA) threaded into ThrottledClient::for_installs. fetch_timeout drives both the response and connect deadlines (mirroring pnpm's AbortSignal.timeout + connectTimeout = fetchTimeout + 1). DEFAULT_FETCH_TIMEOUT_MS (60s) and DEFAULT_USER_AGENT are the shared default sources; the test-only constructors (new_for_installs/from_client) keep using NetworkSettings::default().
  • pacquet-config: four new Config fields wired through defaults.rs, WorkspaceSettings/apply!, and the PNPM_CONFIG_* env overlay. npmrcAuthFile (+ the --userconfig alias and env fallbacks) is resolved in Config::current to redirect the user-level .npmrc read. Single PACQUET_VERSION const now feeds both clap's --version and the UA builder.
  • Reuse: relocated the Rust→Node host_platform/host_arch mappers into the low-level pacquet-detect-libc crate (re-exported from graph-hasher so its public API is unchanged) and reuse them to build pnpm's UA string.
  • pacquet-cli: dedicated flags + client construction from config in state.rs.

Intentional behaviour changes (for parity)

  • Default request timeout: 300 s → pnpm's 60 s (retries still apply).
  • Default User-Agent: literal pnpm → pnpm's full UA format. The pnpm/ token is preserved so UA-keyed WAF/rate-limit allow-lists still pass.

Out of scope (still tracked on #12042)

  • maxSockets, fetchingConcurrency — reqwest has no hard per-host connection cap (the semaphore already bounds concurrency), and fetchingConcurrency is declared in pnpm's Config type but never consumed.
  • fetchWarnTimeoutMs, fetchMinSpeedKiBps — warn-only; pacquet emits no such warnings yet (needs new warning-emission infrastructure first).

Testing

New unit tests cover the defaults (fetch_timeout = 60 000, network_concurrency formula, UA format), the yaml + PNPM_CONFIG_* overlays, the npmrcAuthFile override (auth reaches auth_headers), the relocated mappers, and for_installs honoring custom NetworkSettings (incl. unencodable-UA fallback). Full cargo nextest (2476 passed), clippy --deny warnings, cargo fmt --check, and cargo doc -D warnings all clean.

Pacquet-only change, so no changeset.

Refs #12042.


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

Summary by CodeRabbit

  • New Features

    • Choose a user .npmrc for auth via --npmrc-auth-file / --userconfig
    • Install flags: --network-concurrency, --fetch-timeout, --user-agent
    • CLI reports packaged version and emits a versioned User-Agent; platform/arch included
  • Behavior Change / Security

    • Network settings can be sourced from env/workspace and override per-invocation
    • Unscoped NPM credentials are now pinned to their originating file and merge/precedence are refined
  • Tests

    • Expanded tests for network defaults, UA, env/workspace parsing, and auth precedence

Review Change Stack

Port pnpm's `networkConcurrency`, `fetchTimeout`, `userAgent`, and
`npmrcAuthFile` to pacquet, replacing the hardcoded network-client
constants. Each is configurable via `pnpm-workspace.yaml`, the
`PNPM_CONFIG_*` env overlay, and a dedicated CLI flag, matching pnpm's
config surface.

- `pacquet-network`: add `NetworkSettings` (concurrency, timeout, UA)
  threaded into `ThrottledClient::for_installs`; `fetch_timeout` drives
  both the response and connect deadlines. `DEFAULT_FETCH_TIMEOUT_MS`
  (60s) and `DEFAULT_USER_AGENT` are the default sources.
- `pacquet-config`: add the four `Config` fields with cascade wiring;
  resolve `npmrcAuthFile` (and the `--userconfig` alias / env vars) to
  redirect the user-level `.npmrc` read in `Config::current`.
- Relocate the Rust->Node `host_platform`/`host_arch` mappers into
  `pacquet-detect-libc` (re-exported from `graph-hasher`) and reuse them
  to build pnpm's `pnpm/<version> npm/? node/? <platform> <arch>` UA.

Two intentional behaviour changes for parity: the default request
timeout moves from 300s to pnpm's 60s, and the default User-Agent moves
from the literal `pnpm` to pnpm's full UA format.

Refs #12042
@coderabbitai

coderabbitai Bot commented May 28, 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: c62ca08d-15d6-4e6c-858f-b9d0f6609b7f

📥 Commits

Reviewing files that changed from the base of the PR and between 1281ef4 and df98eda.

📒 Files selected for processing (1)
  • pacquet/crates/config/src/npmrc_auth.rs
🚧 Files skipped from review as they are similar to previous changes (1)
  • pacquet/crates/config/src/npmrc_auth.rs

📝 Walkthrough

Walkthrough

Adds network tuning (network_concurrency, fetch_timeout, user_agent) and an explicit npmrc auth-file override across config, workspace/env overlays, CLI, network client construction, npmrc auth rescoping/merging, detection/version defaults, and tests.

Changes

Network settings and npmrc auth override

Layer / File(s) Summary
Foundation: Platform detection & version defaults
pacquet/crates/config/Cargo.toml, pacquet/crates/config/src/defaults.rs, pacquet/crates/config/src/defaults/tests.rs, pacquet/crates/detect-libc/src/lib.rs, pacquet/crates/graph-hasher/src/engine_name.rs
Add PACQUET_VERSION, default_fetch_timeout(), default_user_agent(), and host_platform()/host_arch() helpers; add pacquet-detect-libc dependency and tests.
Config: fields, npmrc auth resolution, helpers, and tests
pacquet/crates/config/src/lib.rs
Add network_concurrency, fetch_timeout, user_agent, and npmrc_auth_file to Config; implement explicit user .npmrc resolution, read_npmrc_file, read_pnpm_env/read_npm_env; refactor auth merging order and expand unit tests for defaults and npmrc precedence.
Workspace & env overlays
pacquet/crates/config/src/workspace_yaml.rs, pacquet/crates/config/src/workspace_yaml/tests.rs, pacquet/crates/config/src/env_overlay.rs
Parse networkConcurrency, fetchTimeout, userAgent from YAML and PNPM_CONFIG_* env vars (JSON for numeric fields) and propagate to Config.
Network: NetworkSettings and client builder
pacquet/crates/network/src/lib.rs
Introduce NetworkSettings and DEFAULT_FETCH_TIMEOUT_MS; thread settings into ThrottledClient::for_installs and default_client_builder to apply concurrency, timeout, and User-Agent (fallback on invalid header); add ZeroNetworkConcurrency error.
Network tests
pacquet/crates/network/src/tests.rs
Update many for_installs call sites to pass &NetworkSettings::default(); add tests asserting concurrency propagates to semaphore permits, rejects zero concurrency, and that invalid user-agent falls back.
CLI: args, wiring, and state init
pacquet/crates/cli/src/cli_args.rs, pacquet/crates/cli/src/cli_args/install.rs, pacquet/crates/cli/src/state.rs
CliArgs adds global --npmrc-auth-file (alias userconfig) and uses PACQUET_VERSION for clap version; InstallArgs exposes network override flags; CliArgs::run seeds Config with npmrc_auth_file and conditionally applies CLI network overrides in the install branch; State::init builds ThrottledClient with NetworkSettings from Config.
npmrc auth: rescope and merge
pacquet/crates/config/src/npmrc_auth.rs, pacquet/crates/config/src/npmrc_auth/tests.rs
Add rescope_unscoped to pin unscoped credentials to their source registry (or DEFAULT_REGISTRY), add merge_under for layered merging semantics, normalize registry URLs, and update tests to validate rescoping into tls_by_uri and credential pinning.
Test helpers updated
pacquet/crates/package-manager/src/install_package_from_registry/tests.rs
Test create_config() initializes new network and npmrc_auth_file fields for test setups.

Sequence Diagram

sequenceDiagram
  participant CLI as pacquet CLI
  participant Config as pacquet_config::Config
  participant State as pacquet::State
  participant Network as pacquet_network::ThrottledClient
  CLI->>Config: seed npmrc_auth_file + load current (env/YAML applied)
  CLI->>Config: apply CLI network overrides for install
  State->>Network: construct for_installs with NetworkSettings from Config
  Network->>Network: build client (apply fetch_timeout, user_agent, concurrency)
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related issues

Possibly related PRs

Suggested reviewers

  • KSXGitHub

"🐰 I hopped through configs, calm and keen,
found auth files hidden, neat and seen,
tuned fetch and set the agent true,
counted permits — one, two, rendezvous,
a tidy hop — network's running green!"

🚥 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 PR title 'feat(config): make network settings configurable in pacquet' accurately reflects the main objective: adding configurability for network settings (network_concurrency, fetch_timeout, user_agent, npmrc_auth_file) across config, CLI, environment, and workspace YAML sources. The title is concise, clear, and specific to the primary change.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
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 network-settings

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.

@zkochan zkochan marked this pull request as ready for review May 28, 2026 23:50
@qodo-free-for-open-source-projects

Copy link
Copy Markdown

Review Summary by Qodo

Port pnpm's network settings to pacquet with configurable concurrency, timeout, and user-agent

✨ Enhancement

Grey Divider

Walkthroughs

Description
• Port pnpm's network settings (networkConcurrency, fetchTimeout, userAgent, npmrcAuthFile)
  to pacquet
• Add NetworkSettings struct threaded into ThrottledClient::for_installs with configurable
  concurrency, timeout, and user-agent
• Wire four new Config fields through yaml, PNPM_CONFIG_* env vars, and CLI flags
  (--network-concurrency, --fetch-timeout, --user-agent, --npmrc-auth-file)
• Relocate Rust→Node platform/arch mappers to pacquet-detect-libc for reuse in pnpm-compatible
  user-agent format
• Change default request timeout from 300s to pnpm's 60s and default user-agent from literal pnpm
  to full format pnpm/ npm/? node/?  
Diagram
flowchart LR
  CLI["CLI flags<br/>--network-concurrency<br/>--fetch-timeout<br/>--user-agent<br/>--npmrc-auth-file"]
  YAML["pnpm-workspace.yaml<br/>networkConcurrency<br/>fetchTimeout<br/>userAgent"]
  ENV["PNPM_CONFIG_*<br/>env vars"]
  CONFIG["Config struct<br/>network_concurrency<br/>fetch_timeout<br/>user_agent<br/>npmrc_auth_file"]
  NETWORK["NetworkSettings<br/>struct"]
  CLIENT["ThrottledClient<br/>for_installs"]
  
  CLI -->|highest priority| CONFIG
  YAML -->|medium priority| CONFIG
  ENV -->|medium priority| CONFIG
  CONFIG -->|resolved values| NETWORK
  NETWORK -->|concurrency/timeout/UA| CLIENT

Loading

Grey Divider

File Changes

1. pacquet/crates/cli/src/cli_args.rs ✨ Enhancement +27/-3

Add CLI flags for network settings and npmrc override

pacquet/crates/cli/src/cli_args.rs


2. pacquet/crates/cli/src/cli_args/install.rs ✨ Enhancement +26/-0

Add install command flags for network configuration

pacquet/crates/cli/src/cli_args/install.rs


3. pacquet/crates/cli/src/state.rs ✨ Enhancement +12/-3

Thread NetworkSettings into ThrottledClient initialization

pacquet/crates/cli/src/state.rs


View more (12)
4. pacquet/crates/config/src/defaults.rs ✨ Enhancement +25/-0

Add PACQUET_VERSION const and network setting defaults

pacquet/crates/config/src/defaults.rs


5. pacquet/crates/config/src/defaults/tests.rs 🧪 Tests +24/-2

Test fetch timeout and user-agent default format

pacquet/crates/config/src/defaults/tests.rs


6. pacquet/crates/config/src/env_overlay.rs ✨ Enhancement +25/-0

Parse network settings from PNPM_CONFIG_* env vars

pacquet/crates/config/src/env_overlay.rs


7. pacquet/crates/config/src/lib.rs ✨ Enhancement +116/-9

Add four Config fields and npmrcAuthFile resolution logic

pacquet/crates/config/src/lib.rs


8. pacquet/crates/config/src/workspace_yaml.rs ✨ Enhancement +4/-0

Add network settings fields to WorkspaceSettings struct

pacquet/crates/config/src/workspace_yaml.rs


9. pacquet/crates/config/src/workspace_yaml/tests.rs 🧪 Tests +22/-0

Test parsing network settings from pnpm-workspace.yaml

pacquet/crates/config/src/workspace_yaml/tests.rs


10. pacquet/crates/detect-libc/src/lib.rs ✨ Enhancement +58/-1

Add host_platform and host_arch mappers for Node naming

pacquet/crates/detect-libc/src/lib.rs


11. pacquet/crates/graph-hasher/src/engine_name.rs ✨ Enhancement +2/-36

Re-export platform/arch mappers from detect-libc

pacquet/crates/graph-hasher/src/engine_name.rs


12. pacquet/crates/network/src/lib.rs ✨ Enhancement +90/-38

Add NetworkSettings struct and thread into client builder

pacquet/crates/network/src/lib.rs


13. pacquet/crates/network/src/tests.rs 🧪 Tests +153/-39

Update tests to pass NetworkSettings to for_installs

pacquet/crates/network/src/tests.rs


14. pacquet/crates/package-manager/src/install_package_from_registry/tests.rs 🧪 Tests +4/-0

Initialize new Config fields in test helper

pacquet/crates/package-manager/src/install_package_from_registry/tests.rs


15. pacquet/crates/config/Cargo.toml Dependencies +1/-0

Add pacquet-detect-libc dependency

pacquet/crates/config/Cargo.toml


Grey Divider

Qodo Logo

@github-actions

github-actions Bot commented May 28, 2026

Copy link
Copy Markdown
Contributor

Micro-Benchmark Results

Linux

group                          main                                   pr
-----                          ----                                   --
tarball/download_dependency    1.00      7.6±0.06ms   568.8 KB/sec    1.11      8.4±0.19ms   513.6 KB/sec

@codecov-commenter

codecov-commenter commented May 28, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.11111% with 13 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.07%. Comparing base (a9011ad) to head (df98eda).
⚠️ Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
pacquet/crates/config/src/npmrc_auth.rs 94.68% 5 Missing ⚠️
pacquet/crates/detect-libc/src/lib.rs 83.33% 5 Missing ⚠️
pacquet/crates/cli/src/cli_args.rs 72.72% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #12047      +/-   ##
==========================================
+ Coverage   88.73%   89.07%   +0.33%     
==========================================
  Files         234      235       +1     
  Lines       30461    31354     +893     
==========================================
+ Hits        27031    27928     +897     
+ Misses       3430     3426       -4     

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

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

Actionable comments posted: 3

🤖 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 `@pacquet/crates/config/src/lib.rs`:
- Around line 1333-1336: The environment-variable fallback chain in lib.rs uses
Sys::var for PNPM_CONFIG_NPMRC_AUTH_FILE and PNPM_CONFIG_USERCONFIG but misses
the lowercase alias "pnpm_config_userconfig", causing pnpm_config_userconfig to
be ignored; update the Sys::var chain (the same expression that calls
Sys::var("PNPM_CONFIG_NPMRC_AUTH_FILE").or_else(...).or_else(...)) to also call
Sys::var("pnpm_config_userconfig") (in the same position as the other lowercase
alias) so both casing variants for PNPM_CONFIG_USERCONFIG are honored.
- Around line 1344-1349: The code currently uses the file path itself as the
base dir when parent() is None, which causes relative paths in the npmrc to
resolve incorrectly; change the unwrap_or_else branch to use the caller's
current working directory (e.g., std::env::current_dir() falling back to "." on
error) instead of cloning the file path so NpmrcAuth::from_ini receives the
containing directory; update the map closure that returns (text, dir) (the block
using parent().map(...).unwrap_or_else(...)) to compute dir as the parent
directory or cwd.

In `@pacquet/crates/network/src/lib.rs`:
- Around line 286-287: ThrottledClient::for_installs must guard against
settings.network_concurrency == 0 because Semaphore::new(0) causes
acquire().await to block forever; change the construction path that currently
calls Semaphore::new(settings.network_concurrency) to validate and fail fast or
clamp to at least 1 (e.g., return an Err with a clear message if
network_concurrency == 0, or use usize::max(1, settings.network_concurrency)) so
installs never deadlock; update the code around ThrottledClient::for_installs
where Semaphore::new is invoked and ensure the returned error message references
network_concurrency when failing.
🪄 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: a0f13ce8-0b3e-47f3-954b-87c6b81aedb8

📥 Commits

Reviewing files that changed from the base of the PR and between a593082 and 2a3b7e4.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (15)
  • pacquet/crates/cli/src/cli_args.rs
  • pacquet/crates/cli/src/cli_args/install.rs
  • pacquet/crates/cli/src/state.rs
  • pacquet/crates/config/Cargo.toml
  • pacquet/crates/config/src/defaults.rs
  • pacquet/crates/config/src/defaults/tests.rs
  • pacquet/crates/config/src/env_overlay.rs
  • pacquet/crates/config/src/lib.rs
  • pacquet/crates/config/src/workspace_yaml.rs
  • pacquet/crates/config/src/workspace_yaml/tests.rs
  • pacquet/crates/detect-libc/src/lib.rs
  • pacquet/crates/graph-hasher/src/engine_name.rs
  • pacquet/crates/network/src/lib.rs
  • pacquet/crates/network/src/tests.rs
  • pacquet/crates/package-manager/src/install_package_from_registry/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). (4)
  • GitHub Check: ubuntu-latest / Node.js 24 / Test
  • GitHub Check: Code Coverage
  • GitHub Check: Lint and Test (windows-latest)
  • GitHub Check: Run benchmark on ubuntu-latest
🧰 Additional context used
📓 Path-based instructions (1)
pacquet/**/*.rs

📄 CodeRabbit inference engine (pacquet/AGENTS.md)

pacquet/**/*.rs: Warnings are errors (--deny warnings in lint). Do not silence them with #[allow(...)] unless there is a specific, justified reason.
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: https://rust-lang.github.io/api-guidelines/naming.html
No star imports inside module bodies. Write use super::{Foo, bar} instead of use super::*;, and the same for any other glob whose target is a module you control. Two forms stay allowed: external-crate preludes such as use rayon::prelude::*; and root-of-module re-exports such as pub use submodule::*; in a lib.rs.
Doc comments (///, //!) document the contract: preconditions, postconditions, panics, the reason the function exists. They are not a re-narration of the body.
Do not restate at call sites what the callee's doc comment already says. If /// on the function says 'no-op when …', the caller should not repeat that. Update the doc once; let every call site benefit.
Tests are documentation. Do not duplicate them in prose. If a behavioral scenario, edge case, failure mode, or worked example is already captured by a test, do not also narrate it in the doc comment on the implementation. The same applies in reverse: a test's own doc comment should not re-explain what the asserts already say, only the why if it is not obvious.
Use // SAFETY:, // TODO:, and similar prefixes to signal hidden invariants or known follow-ups that a reader cannot recover from the code alone.
When editing existing code, do not break a method chain (including pipe-trait .pipe(...) chains) into intermediate let bindings unless you can justify the rewrite. Valid justifications include a chain that fails to compile after your...

Files:

  • pacquet/crates/config/src/env_overlay.rs
  • pacquet/crates/package-manager/src/install_package_from_registry/tests.rs
  • pacquet/crates/config/src/workspace_yaml.rs
  • pacquet/crates/config/src/workspace_yaml/tests.rs
  • pacquet/crates/config/src/defaults.rs
  • pacquet/crates/cli/src/state.rs
  • pacquet/crates/cli/src/cli_args/install.rs
  • pacquet/crates/detect-libc/src/lib.rs
  • pacquet/crates/config/src/defaults/tests.rs
  • pacquet/crates/network/src/lib.rs
  • pacquet/crates/cli/src/cli_args.rs
  • pacquet/crates/graph-hasher/src/engine_name.rs
  • pacquet/crates/config/src/lib.rs
  • pacquet/crates/network/src/tests.rs
🧠 Learnings (3)
📚 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/config/src/env_overlay.rs
  • pacquet/crates/package-manager/src/install_package_from_registry/tests.rs
  • pacquet/crates/config/src/workspace_yaml.rs
  • pacquet/crates/config/src/workspace_yaml/tests.rs
  • pacquet/crates/config/src/defaults.rs
  • pacquet/crates/cli/src/state.rs
  • pacquet/crates/cli/src/cli_args/install.rs
  • pacquet/crates/detect-libc/src/lib.rs
  • pacquet/crates/config/src/defaults/tests.rs
  • pacquet/crates/network/src/lib.rs
  • pacquet/crates/cli/src/cli_args.rs
  • pacquet/crates/graph-hasher/src/engine_name.rs
  • pacquet/crates/config/src/lib.rs
  • pacquet/crates/network/src/tests.rs
📚 Learning: 2026-05-22T00:08:44.646Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11837
File: pacquet/crates/resolving-npm-resolver/src/pick_package.rs:33-51
Timestamp: 2026-05-22T00:08:44.646Z
Learning: In the pnpm/pnpm repo’s pacquet Rust crates, do not flag Unicode ellipsis characters (U+2026, `…`) in Rust doc comments (`///` / `/** */`) as a lint violation. The pacquet crate’s `dylint.toml` only enables `perfectionist::derive_ordering`, and the Dylint `unicode-ellipsis` rule is not enabled for this project—so `…` in doc comments is an intentional, repo-consistent style.

Applied to files:

  • pacquet/crates/config/src/env_overlay.rs
  • pacquet/crates/package-manager/src/install_package_from_registry/tests.rs
  • pacquet/crates/config/src/workspace_yaml.rs
  • pacquet/crates/config/src/workspace_yaml/tests.rs
  • pacquet/crates/config/src/defaults.rs
  • pacquet/crates/cli/src/state.rs
  • pacquet/crates/cli/src/cli_args/install.rs
  • pacquet/crates/detect-libc/src/lib.rs
  • pacquet/crates/config/src/defaults/tests.rs
  • pacquet/crates/network/src/lib.rs
  • pacquet/crates/cli/src/cli_args.rs
  • pacquet/crates/graph-hasher/src/engine_name.rs
  • pacquet/crates/config/src/lib.rs
  • pacquet/crates/network/src/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/config/src/env_overlay.rs
  • pacquet/crates/package-manager/src/install_package_from_registry/tests.rs
  • pacquet/crates/config/src/workspace_yaml.rs
  • pacquet/crates/config/src/workspace_yaml/tests.rs
  • pacquet/crates/config/src/defaults.rs
  • pacquet/crates/cli/src/state.rs
  • pacquet/crates/cli/src/cli_args/install.rs
  • pacquet/crates/detect-libc/src/lib.rs
  • pacquet/crates/config/src/defaults/tests.rs
  • pacquet/crates/network/src/lib.rs
  • pacquet/crates/cli/src/cli_args.rs
  • pacquet/crates/graph-hasher/src/engine_name.rs
  • pacquet/crates/config/src/lib.rs
  • pacquet/crates/network/src/tests.rs
🔇 Additional comments (14)
pacquet/crates/config/Cargo.toml (1)

15-15: LGTM!

pacquet/crates/config/src/defaults.rs (1)

259-282: LGTM!

pacquet/crates/detect-libc/src/lib.rs (2)

63-97: LGTM!


136-155: LGTM!

pacquet/crates/config/src/defaults/tests.rs (1)

434-453: LGTM!

pacquet/crates/graph-hasher/src/engine_name.rs (1)

1-1: ⚡ Quick win

Re-check ENGINE_NAME cache-key mapper equivalence

The comparison can’t establish byte-identical output: the repo snapshot/history for pacquet/crates/graph-hasher/src/engine_name.rs doesn’t contain prior host_platform/host_arch implementations, and the attempted detect-libc path lookup fails (fd reports detect-libc isn’t a directory). Add/adjust a test or small fixture that asserts the exact emitted strings from the old local host_* functions match pacquet_detect_libc::{host_platform, host_arch} for the full OS/arch matrix, ensuring pnpm cache interop.

pacquet/crates/network/src/tests.rs (2)

17-20: LGTM!

Also applies to: 164-164, 176-182, 189-195, 211-217, 227-233, 265-271, 307-313, 348-354, 364-370, 382-388, 405-411, 422-428, 435-441, 453-458, 473-479, 501-507, 528-528, 563-563


631-665: LGTM!

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

71-74: LGTM!

pacquet/crates/network/src/lib.rs (2)

50-76: LGTM!


351-363: LGTM!

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

24-24: LGTM!

Also applies to: 34-40, 98-139, 175-187

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

197-219: LGTM!

Also applies to: 240-242

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

5-5: LGTM!

Also applies to: 73-85

Comment thread pacquet/crates/config/src/lib.rs Outdated
Comment thread pacquet/crates/config/src/lib.rs Outdated
Comment thread pacquet/crates/network/src/lib.rs
…ce tests

The user-level `.npmrc` path resolution applied the empty-string filter
once after the whole `or_else` chain, so an exported-but-empty
`PNPM_CONFIG_NPMRC_AUTH_FILE` short-circuited resolution instead of
falling through. Mirror pnpm's `readEnvVar` / `readNpmEnvVar` exactly:
filter `value !== ''` per variable, accept both `pnpm_config_*` /
`PNPM_CONFIG_*` cases, and add the `npm_config_userconfig` /
`NPM_CONFIG_USERCONFIG` compatibility fallback.

Port the translatable pnpm precedence tests from
`config/reader/test/index.ts` (resolution from the PNPM_CONFIG_* family,
lowercase variant, empty-falls-through, npmrc_auth_file outranks
userconfig, npm_config_userconfig compat fallback + pnpm-wins). pnpm's
credential-scoping tests that read the workspace .npmrc and the
userconfig simultaneously and re-scope per file don't translate yet:
pacquet uses a single-file (project-or-user) model, not pnpm's layered
merge. Global config.yaml sourcing of npmrcAuthFile remains deferred.

Refs #12042
@github-actions

github-actions Bot commented May 29, 2026

Copy link
Copy Markdown
Contributor

Integrated-Benchmark Report (Linux)

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

Command Mean [s] Min [s] Max [s] Relative
pacquet@HEAD 2.093 ± 0.089 2.028 2.335 1.00
pacquet@main 2.115 ± 0.057 2.054 2.214 1.01 ± 0.05
BENCHMARK_REPORT.json
{
  "results": [
    {
      "command": "pacquet@HEAD",
      "mean": 2.09277841818,
      "stddev": 0.08909714031331682,
      "median": 2.05952879378,
      "user": 2.6348738,
      "system": 3.41852404,
      "min": 2.02822905528,
      "max": 2.33451435828,
      "times": [
        2.11053487928,
        2.05889568528,
        2.05936758528,
        2.05316642528,
        2.03467390728,
        2.02822905528,
        2.33451435828,
        2.08393650628,
        2.10477577728,
        2.05969000228
      ]
    },
    {
      "command": "pacquet@main",
      "mean": 2.1150200452800005,
      "stddev": 0.05720151408609704,
      "median": 2.11143746778,
      "user": 2.6901137,
      "system": 3.42461864,
      "min": 2.05435649728,
      "max": 2.21387818728,
      "times": [
        2.17645618028,
        2.06613788228,
        2.13225553528,
        2.21387818728,
        2.05435649728,
        2.14754306328,
        2.06007709828,
        2.09061940028,
        2.05628032528,
        2.15259628328
      ]
    }
  ]
}

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

Command Mean [ms] Min [ms] Max [ms] Relative
pacquet@HEAD 648.4 ± 38.0 629.3 755.3 1.00
pacquet@main 678.8 ± 26.0 660.2 749.6 1.05 ± 0.07
BENCHMARK_REPORT.json
{
  "results": [
    {
      "command": "pacquet@HEAD",
      "mean": 0.6484092193000001,
      "stddev": 0.038017772063967906,
      "median": 0.6371570389000001,
      "user": 0.35674789999999995,
      "system": 1.3214605599999998,
      "min": 0.6292929839,
      "max": 0.7553392089000001,
      "times": [
        0.7553392089000001,
        0.6348944449,
        0.6393465739,
        0.6374337869000001,
        0.6303156079000001,
        0.6292929839,
        0.6310301669,
        0.6368802909,
        0.6406088449,
        0.6489502839
      ]
    },
    {
      "command": "pacquet@main",
      "mean": 0.6787909652999999,
      "stddev": 0.02602697648843731,
      "median": 0.6724947074000001,
      "user": 0.36534969999999994,
      "system": 1.36207946,
      "min": 0.6601970179000001,
      "max": 0.7495545779,
      "times": [
        0.7495545779,
        0.6800668959,
        0.6792241359,
        0.6801203349,
        0.6665650609,
        0.6773976529,
        0.6643775519,
        0.6601970179000001,
        0.6628146629,
        0.6675917619
      ]
    }
  ]
}

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

Command Mean [s] Min [s] Max [s] Relative
pacquet@HEAD 2.342 ± 0.027 2.300 2.391 1.00
pacquet@main 2.385 ± 0.031 2.331 2.439 1.02 ± 0.02
BENCHMARK_REPORT.json
{
  "results": [
    {
      "command": "pacquet@HEAD",
      "mean": 2.3420414909,
      "stddev": 0.026604181803345103,
      "median": 2.3444414655,
      "user": 3.7707546799999996,
      "system": 3.17916472,
      "min": 2.3001129464999996,
      "max": 2.3912308185,
      "times": [
        2.3456084155,
        2.3524930415,
        2.3617720765,
        2.3578520915,
        2.3120962105,
        2.3362923755,
        2.3001129464999996,
        2.3432745155,
        2.3196824174999997,
        2.3912308185
      ]
    },
    {
      "command": "pacquet@main",
      "mean": 2.3851991734999998,
      "stddev": 0.03098416002487624,
      "median": 2.3789911874999996,
      "user": 3.8636414799999996,
      "system": 3.20535002,
      "min": 2.3311027775,
      "max": 2.4387011435,
      "times": [
        2.3764394285,
        2.3779619985,
        2.3935099295,
        2.3855048695,
        2.4387011435,
        2.3792299535,
        2.3311027775,
        2.3609858514999997,
        2.4298033615,
        2.3787524214999998
      ]
    }
  ]
}

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

Command Mean [s] Min [s] Max [s] Relative
pacquet@HEAD 1.487 ± 0.021 1.461 1.535 1.00
pacquet@main 1.515 ± 0.049 1.469 1.640 1.02 ± 0.04
BENCHMARK_REPORT.json
{
  "results": [
    {
      "command": "pacquet@HEAD",
      "mean": 1.4871258807999999,
      "stddev": 0.020777996159418253,
      "median": 1.4800372808,
      "user": 1.6874484999999997,
      "system": 1.85727006,
      "min": 1.4605069613000001,
      "max": 1.5354491743,
      "times": [
        1.4742965053000001,
        1.4903802863,
        1.4961832473,
        1.4797176513,
        1.5021796043000002,
        1.4605069613000001,
        1.4734590163,
        1.4787294513,
        1.5354491743,
        1.4803569103
      ]
    },
    {
      "command": "pacquet@main",
      "mean": 1.5145992562,
      "stddev": 0.048504727053240625,
      "median": 1.4976636288,
      "user": 1.7111889999999998,
      "system": 1.8681599600000003,
      "min": 1.4690991863,
      "max": 1.6403619383,
      "times": [
        1.5306261283,
        1.5009408523,
        1.4936591683,
        1.4795500563,
        1.6403619383,
        1.4903087983,
        1.4943864053,
        1.5266460582999999,
        1.4690991863,
        1.5204139703
      ]
    }
  ]
}

@github-actions

github-actions Bot commented May 29, 2026

Copy link
Copy Markdown
Contributor

🐰 Bencher Report

Branchpr/12047
Testbedpacquet
Click to view all benchmark results
BenchmarkLatencyBenchmark Result
milliseconds (ms)
(Result Δ%)
Upper Boundary
milliseconds (ms)
(Limit %)
isolated-linker.fresh-install.cold-cache.cold-store📈 view plot
🚷 view threshold
2,342.04 ms
(-0.45%)Baseline: 2,352.69 ms
2,823.23 ms
(82.96%)
isolated-linker.fresh-install.hot-cache.hot-store📈 view plot
🚷 view threshold
1,487.13 ms
(+1.25%)Baseline: 1,468.80 ms
1,762.56 ms
(84.37%)
isolated-linker.fresh-restore.cold-cache.cold-store📈 view plot
🚷 view threshold
2,092.78 ms
(-0.31%)Baseline: 2,099.30 ms
2,519.16 ms
(83.07%)
isolated-linker.fresh-restore.hot-cache.hot-store📈 view plot
🚷 view threshold
648.41 ms
(-5.09%)Baseline: 683.20 ms
819.84 ms
(79.09%)
🐰 View full continuous benchmarking report in Bencher

zkochan added 3 commits May 29, 2026 02:30
…scoping

Port pnpm's multi-file `.npmrc` layering and the credential-isolation
security boundary it provides. `Config::current` now reads the
user-level file (npmrcAuthFile / userconfig / ~/.npmrc), the global
`auth.ini`, and the project `.npmrc` together and merges them
(`user < auth.ini < workspace`) instead of reading just the first one
found.

Each source is rescoped before the merge (`NpmrcAuth::rescope_unscoped`,
ported from pnpm's `rescopeUnscopedCreds`): a file's *unscoped*
`_authToken` / `_auth` / `username` / `_password` / `cert` / `key` are
pinned to that file's own `registry=` (or the npmjs default when it
declares none), nerf-darted into a per-URI key. Once pinned, a
credential can never be pulled to a different registry that a
higher-priority file — or a later `pnpm-workspace.yaml` registry
override — sets. A deprecation warning is queued for each rescoped key.

`npmrcAuthFile` is now also sourced from the global `config.yaml`
(between `PNPM_CONFIG_USERCONFIG` and `npm_config_userconfig`),
completing pnpm's resolution order.

Behaviour changes for parity, both covered by ported tests: an unscoped
top-level `cert`/`key` becomes per-registry client identity (pinned to
its file's registry) rather than a global identity sent to every host;
and a user-file credential no longer leaks to a workspace registry
override. Ports the credential-scoping suite from
`config/reader/test/index.ts`.

Refs #12042
…ir safely

Address review feedback on #12042:

- `ThrottledClient::for_installs` now returns
  `ForInstallsError::ZeroNetworkConcurrency` when `network_concurrency`
  is 0 instead of building a zero-permit semaphore that would hang every
  fetch. Matches pnpm, which rejects the value (p-queue requires a
  concurrency >= 1).
- When `npmrcAuthFile` is a bare filename with no parent, resolve its
  relative `cafile`/`certfile` entries against the process cwd (empty
  base path) rather than treating the file itself as the base directory.
Satisfies dylint's `perfectionist::macro-trailing-comma`, which the
pre-push hook enforces but `just ready` / clippy don't surface.
Satisfies the Spell Check (crate-ci/typos) CI job.
@zkochan zkochan merged commit 74dd8ba into main May 29, 2026
28 checks passed
@zkochan zkochan deleted the network-settings branch May 29, 2026 09:39
zkochan added a commit that referenced this pull request May 29, 2026
The pacquet release workflow patched the clap `version` attribute in
cli_args.rs, expecting a string literal. Since #12047 that attribute
references the `pacquet_config::PACQUET_VERSION` constant, so the perl
substitution matched nothing and the verifying grep failed, aborting the
whole release.

Patch the `PACQUET_VERSION` constant in defaults.rs instead. That single
constant feeds both `pacquet --version` and the default User-Agent, so
both report the published version.

Also tag the default User-Agent as `pnpm/pacquet-<version>` so registries
can tell pacquet's traffic apart from the TypeScript pnpm CLI, while
keeping the `pnpm` token for UA-keyed allow / rate-limit rules.
zkochan added a commit that referenced this pull request May 29, 2026
* feat(config): make network settings configurable in pacquet

Port pnpm's `networkConcurrency`, `fetchTimeout`, `userAgent`, and
`npmrcAuthFile` to pacquet, replacing the hardcoded network-client
constants. Each is configurable via `pnpm-workspace.yaml`, the
`PNPM_CONFIG_*` env overlay, and a dedicated CLI flag, matching pnpm's
config surface.

- `pacquet-network`: add `NetworkSettings` (concurrency, timeout, UA)
  threaded into `ThrottledClient::for_installs`; `fetch_timeout` drives
  both the response and connect deadlines. `DEFAULT_FETCH_TIMEOUT_MS`
  (60s) and `DEFAULT_USER_AGENT` are the default sources.
- `pacquet-config`: add the four `Config` fields with cascade wiring;
  resolve `npmrcAuthFile` (and the `--userconfig` alias / env vars) to
  redirect the user-level `.npmrc` read in `Config::current`.
- Relocate the Rust->Node `host_platform`/`host_arch` mappers into
  `pacquet-detect-libc` (re-exported from `graph-hasher`) and reuse them
  to build pnpm's `pnpm/<version> npm/? node/? <platform> <arch>` UA.

Two intentional behaviour changes for parity: the default request
timeout moves from 300s to pnpm's 60s, and the default User-Agent moves
from the literal `pnpm` to pnpm's full UA format.

Refs #12042

* fix(config): correct npmrcAuthFile env resolution; port pnpm precedence tests

The user-level `.npmrc` path resolution applied the empty-string filter
once after the whole `or_else` chain, so an exported-but-empty
`PNPM_CONFIG_NPMRC_AUTH_FILE` short-circuited resolution instead of
falling through. Mirror pnpm's `readEnvVar` / `readNpmEnvVar` exactly:
filter `value !== ''` per variable, accept both `pnpm_config_*` /
`PNPM_CONFIG_*` cases, and add the `npm_config_userconfig` /
`NPM_CONFIG_USERCONFIG` compatibility fallback.

Port the translatable pnpm precedence tests from
`config/reader/test/index.ts` (resolution from the PNPM_CONFIG_* family,
lowercase variant, empty-falls-through, npmrc_auth_file outranks
userconfig, npm_config_userconfig compat fallback + pnpm-wins). pnpm's
credential-scoping tests that read the workspace .npmrc and the
userconfig simultaneously and re-scope per file don't translate yet:
pacquet uses a single-file (project-or-user) model, not pnpm's layered
merge. Global config.yaml sourcing of npmrcAuthFile remains deferred.

Refs #12042

* feat(config): merge multiple .npmrc sources with per-file credential scoping

Port pnpm's multi-file `.npmrc` layering and the credential-isolation
security boundary it provides. `Config::current` now reads the
user-level file (npmrcAuthFile / userconfig / ~/.npmrc), the global
`auth.ini`, and the project `.npmrc` together and merges them
(`user < auth.ini < workspace`) instead of reading just the first one
found.

Each source is rescoped before the merge (`NpmrcAuth::rescope_unscoped`,
ported from pnpm's `rescopeUnscopedCreds`): a file's *unscoped*
`_authToken` / `_auth` / `username` / `_password` / `cert` / `key` are
pinned to that file's own `registry=` (or the npmjs default when it
declares none), nerf-darted into a per-URI key. Once pinned, a
credential can never be pulled to a different registry that a
higher-priority file — or a later `pnpm-workspace.yaml` registry
override — sets. A deprecation warning is queued for each rescoped key.

`npmrcAuthFile` is now also sourced from the global `config.yaml`
(between `PNPM_CONFIG_USERCONFIG` and `npm_config_userconfig`),
completing pnpm's resolution order.

Behaviour changes for parity, both covered by ported tests: an unscoped
top-level `cert`/`key` becomes per-registry client identity (pinned to
its file's registry) rather than a global identity sent to every host;
and a user-file credential no longer leaks to a workspace registry
override. Ports the credential-scoping suite from
`config/reader/test/index.ts`.

Refs #12042

* fix(network): reject networkConcurrency=0; resolve user .npmrc base dir safely

Address review feedback on #12042:

- `ThrottledClient::for_installs` now returns
  `ForInstallsError::ZeroNetworkConcurrency` when `network_concurrency`
  is 0 instead of building a zero-permit semaphore that would hang every
  fetch. Matches pnpm, which rejects the value (p-queue requires a
  concurrency >= 1).
- When `npmrcAuthFile` is a bare filename with no parent, resolve its
  relative `cafile`/`certfile` entries against the process cwd (empty
  base path) rather than treating the file itself as the base directory.

* style(config): add trailing comma to multi-line format! (dylint)

Satisfies dylint's `perfectionist::macro-trailing-comma`, which the
pre-push hook enforces but `just ready` / clippy don't surface.

* style(config): fix "Unparseable" -> "Unparsable" typo in comment

Satisfies the Spell Check (crate-ci/typos) CI job.
zkochan added a commit that referenced this pull request May 29, 2026
The pacquet release workflow patched the clap `version` attribute in
cli_args.rs, expecting a string literal. Since #12047 that attribute
references the `pacquet_config::PACQUET_VERSION` constant, so the perl
substitution matched nothing and the verifying grep failed, aborting the
whole release.

Patch the `PACQUET_VERSION` constant in defaults.rs instead. That single
constant feeds both `pacquet --version` and the default User-Agent, so
both report the published version.

Also tag the default User-Agent as `pnpm/pacquet-<version>` so registries
can tell pacquet's traffic apart from the TypeScript pnpm CLI, while
keeping the `pnpm` token for UA-keyed allow / rate-limit rules.
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