Skip to content

fix(ci): inject release version into PACQUET_VERSION constant#12058

Merged
zkochan merged 1 commit into
mainfrom
release-fix
May 29, 2026
Merged

fix(ci): inject release version into PACQUET_VERSION constant#12058
zkochan merged 1 commit into
mainfrom
release-fix

Conversation

@zkochan

@zkochan zkochan commented May 29, 2026

Copy link
Copy Markdown
Member

Why

The pacquet release run failed at the Inject version into pacquet/crates/cli/src/cli_args.rs step (exit 1), which — with fail-fast — cancelled every platform leg.

The step patched the clap version attribute, expecting a string literal:

#[clap(version = "0.2.2")]

Since #12047 that attribute references a constant:

#[clap(version = pacquet_config::PACQUET_VERSION)]

So the perl substitution matched nothing, the file was left unchanged, and the verifying grep "version = \"$VERSION\"" failed.

What

  • Workflow: patch the PACQUET_VERSION constant in pacquet/crates/config/src/defaults.rs instead of the clap attribute. That single constant feeds both pacquet --version and the default User-Agent, so both now report the published version (the old step only fixed --version).
  • User-Agent: tag the default UA as pnpm/pacquet-<version> npm/? node/? <platform> <arch> so registries can distinguish pacquet's traffic from the TypeScript pnpm CLI, while keeping the leading pnpm token so UA-keyed allow / rate-limit rules that pass pnpm also pass pacquet.
  • Updated the UA-format test and the two doc comments describing the old format.

Verification

  • Confirmed the new perl substitution + grep succeed under bash with a sample version (0.2.2-14).
  • cargo nextest run -p pacquet-config user_agent passes.
  • cargo clippy -p pacquet-config -p pacquet-network --all-targets -- --deny warnings clean.

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

Summary by CodeRabbit

  • Updates
    • User-agent identification in HTTP requests to package registries now uses an updated format for better clarity and distinction.

Review Change Stack

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

coderabbitai Bot commented May 29, 2026

Copy link
Copy Markdown

Caution

Review failed

Pull request was closed or merged during review

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: f57a86ec-8192-492c-8189-c06f0fdcaa3f

📥 Commits

Reviewing files that changed from the base of the PR and between 74dd8ba and 1cb2137.

📒 Files selected for processing (5)
  • .github/workflows/pacquet-release-to-npm.yml
  • pacquet/crates/config/src/defaults.rs
  • pacquet/crates/config/src/defaults/tests.rs
  • pacquet/crates/config/src/lib.rs
  • pacquet/crates/network/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). (2)
  • 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/network/src/lib.rs
  • pacquet/crates/config/src/defaults.rs
  • pacquet/crates/config/src/defaults/tests.rs
  • pacquet/crates/config/src/lib.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/network/src/lib.rs
  • pacquet/crates/config/src/defaults.rs
  • pacquet/crates/config/src/defaults/tests.rs
  • pacquet/crates/config/src/lib.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/network/src/lib.rs
  • pacquet/crates/config/src/defaults.rs
  • pacquet/crates/config/src/defaults/tests.rs
  • pacquet/crates/config/src/lib.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/network/src/lib.rs
  • pacquet/crates/config/src/defaults.rs
  • pacquet/crates/config/src/defaults/tests.rs
  • pacquet/crates/config/src/lib.rs
🔇 Additional comments (5)
.github/workflows/pacquet-release-to-npm.yml (1)

85-95: LGTM!

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

272-279: LGTM!

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

440-449: LGTM!

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

858-861: LGTM!

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

25-29: LGTM!


📝 Walkthrough

Walkthrough

The PR refactors the pacquet release process to inject the version into a dedicated constant in the config crate, then updates the user-agent format from pnpm/<version> to pnpm/pacquet-<version> consistently across workflow, implementation, tests, and documentation.

Changes

User-Agent Format and Version Injection

Layer / File(s) Summary
Workflow version injection refactoring
.github/workflows/pacquet-release-to-npm.yml
The release workflow now patches the PACQUET_VERSION constant in pacquet/crates/config/src/defaults.rs instead of modifying cli_args.rs.
User-Agent format and version integration
pacquet/crates/config/src/defaults.rs, pacquet/crates/config/src/defaults/tests.rs
The default_user_agent() function generates pnpm/pacquet-<version> prefix and the test expectations are updated to match.
Documentation consistency
pacquet/crates/config/src/lib.rs, pacquet/crates/network/src/lib.rs
Doc comments are updated to describe the new pnpm/pacquet-<version> user-agent format across config and network modules.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

  • pnpm/pnpm#11812: Related workflow refactoring that similarly updates version injection into the pacquet release build process.

Poem

🐰 A rabbit hops through version strings with care,
From pnpm/ to pacquet-—a clearer name to share!
The workflow learns a new injection way,
And all docs align in perfect harmony today.
hippity-hops 🌿✨

🚥 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 describes the main change: fixing the CI workflow to inject the release version into the PACQUET_VERSION constant instead of attempting to patch a clap attribute.
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 release-fix

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.

@github-actions

Copy link
Copy Markdown
Contributor

Micro-Benchmark Results

Linux

group                          main                                   pr
-----                          ----                                   --
tarball/download_dependency    1.00      8.0±0.19ms   542.2 KB/sec    1.01      8.1±0.20ms   537.9 KB/sec

@zkochan zkochan marked this pull request as ready for review May 29, 2026 10:03
@qodo-free-for-open-source-projects

Copy link
Copy Markdown

Review Summary by Qodo

Fix release workflow and update User-Agent format to include pacquet identifier

🐞 Bug fix ✨ Enhancement

Grey Divider

Walkthroughs

Description
• Fix release workflow by patching PACQUET_VERSION constant instead of clap attribute
• Update User-Agent format to pnpm/pacquet- for traffic differentiation
• Update documentation and tests to reflect new User-Agent format
• Ensure both --version and User-Agent report published version
Diagram
flowchart LR
  A["Release Workflow"] -->|"Patch PACQUET_VERSION"| B["defaults.rs constant"]
  B -->|"Feeds both"| C["pacquet --version"]
  B -->|"Feeds both"| D["User-Agent header"]
  D -->|"New format"| E["pnpm/pacquet-VERSION"]

Loading

Grey Divider

File Changes

1. pacquet/crates/config/src/defaults.rs ✨ Enhancement +6/-5

Update User-Agent format with pacquet identifier

• Updated default_user_agent() format from pnpm/{PACQUET_VERSION} to
 pnpm/pacquet-{PACQUET_VERSION}
• Updated doc comment to explain the new User-Agent format identifies pacquet traffic separately
• Clarified that the constant feeds both --version output and User-Agent header

pacquet/crates/config/src/defaults.rs


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

Update User-Agent format tests

• Updated test expectations to match new User-Agent format with pnpm/pacquet- prefix
• Updated test documentation comments to reflect the new format
• Maintained assertion logic for platform and arch tokens

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


3. pacquet/crates/config/src/lib.rs 📝 Documentation +1/-1

Update User-Agent documentation

• Updated doc comment for user_agent field to reflect new format pnpm/pacquet-
• Clarified that default format is built by default_user_agent function

pacquet/crates/config/src/lib.rs


View more (2)
4. pacquet/crates/network/src/lib.rs 📝 Documentation +1/-1

Update fallback User-Agent documentation

• Updated fallback User-Agent documentation to reflect new format pnpm/pacquet-
• Clarified that pnpm token is preserved for UA-keyed allow/rate-limit rules

pacquet/crates/network/src/lib.rs


5. .github/workflows/pacquet-release-to-npm.yml 🐞 Bug fix +7/-7

Fix release workflow version injection

• Changed target file from pacquet/crates/cli/src/cli_args.rs to
 pacquet/crates/config/src/defaults.rs
• Updated perl substitution pattern to patch PACQUET_VERSION constant instead of clap attribute
• Updated grep verification to match new constant format
• Updated step name and description to reflect the new approach

.github/workflows/pacquet-release-to-npm.yml


Grey Divider

Qodo Logo

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.12%. Comparing base (74dd8ba) to head (1cb2137).

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #12058      +/-   ##
==========================================
- Coverage   89.13%   89.12%   -0.02%     
==========================================
  Files         235      235              
  Lines       31550    31550              
==========================================
- Hits        28123    28118       -5     
- Misses       3427     3432       +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 98d4c60 into main May 29, 2026
27 of 28 checks passed
@zkochan zkochan deleted the release-fix branch May 29, 2026 10:06
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