Conversation
Port pnpm's `update` (aliases `up`/`upgrade`) onto pacquet's always-fresh-resolve install path. - Compatible bump: withhold matched names' lockfile pins from the preferred-versions seed (new `UpdateSeedPolicy`) so they re-resolve to highest-in-range; `package.json` is left untouched. This is what distinguishes `update` from `install`. - `--latest`: fetch each matched direct dep's `latest` tag and rewrite the manifest range (`^v`, or exact under `--save-exact`), like `add`. - Selectors: bare-name/glob patterns (`depth>0`, no `--latest`) match every locked package name at any depth; versioned (`foo@2`) or `--latest` selectors match direct deps only; `--latest` + spec is rejected with `ERR_PNPM_LATEST_WITH_SPEC`. - CLI flags: `-L/--latest`, `-E/--save-exact`, `-P/-D/--no-optional` (faithful `makeIncludeDependenciesFromCLI`), `--depth`, `--lockfile-only`, `-i/--interactive` (dialoguer + inline outdated check). - `--global` and `--workspace` error out for now: the global-dir and workspace-version-linking subsystems are not ported yet. The resolver's `UpdateBehavior` tri-state and the npm resolver's `include_latest_tag` already existed; this change drives them from a CLI command. Written by an agent (Claude Code, claude-opus-4-8).
📝 WalkthroughWalkthroughAdds a full Changespacquet update implementation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
Contributor
Micro-Benchmark ResultsLinux |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #12102 +/- ##
==========================================
- Coverage 87.30% 87.04% -0.27%
==========================================
Files 249 252 +3
Lines 27769 28142 +373
==========================================
+ Hits 24245 24495 +250
- Misses 3524 3647 +123 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…sing Reviewing pnpm's `update` test suite surfaced gaps: - Fix `parse_update_param` to match pnpm's `parseUpdateParam`: search for the version `@` starting at index 2 for `!`-negated patterns (1 otherwise), so `!@scope/pkg-*` is no longer wrongly split into pattern `!` + version `scope/pkg-*`. Negation selectors now work. - Add `--no-save`: the range rewrites still drive resolution (lockfile updates) but `package.json` is not persisted. Mirrors pnpm's `updatePackageManifest: opts.save !== false`. - Add `ERR_PNPM_NO_PACKAGE_IN_DEPENDENCIES`: a selector that matches no direct dependency under `--depth 0` (without `--latest`) now errors, matching pnpm; `--latest` with an unmatched selector is a no-op. Ports the negation-pattern, `--no-save`, and no-package-in-deps tests from pnpm's `installing/commands/test/update/update.ts`. Written by an agent (Claude Code, claude-opus-4-8).
Review Summary by QodoImplement
WalkthroughsDescription• Implement pacquet update command with up/upgrade aliases • Support compatible bump (re-resolve to highest in-range) and --latest flag • Add interactive mode with outdated dependency selection via dialoguer • Support package selectors (bare names, globs, versioned) with depth filtering • Implement CLI flags: -L/--latest, -E/--save-exact, -P/-D/--no-optional, --depth, --lockfile-only, -i/--interactive Diagramflowchart LR
CLI["CLI: update/up/upgrade"]
UpdateArgs["UpdateArgs parsing"]
UpdateOp["Update operation"]
Selector["Selector parsing<br/>bare/glob/versioned"]
Latest["--latest flag<br/>fetch latest tag"]
Compatible["Compatible bump<br/>UpdateSeedPolicy"]
Install["Install with<br/>fresh resolve"]
Manifest["Rewrite package.json<br/>if --save"]
CLI --> UpdateArgs
UpdateArgs --> UpdateOp
UpdateOp --> Selector
Selector --> Latest
Selector --> Compatible
Latest --> Manifest
Compatible --> Install
Manifest --> Install
File Changes1. pacquet/crates/cli/src/cli_args.rs
|
Contributor
Integrated-Benchmark Report (Linux)Scenario: Isolated linker: fresh restore, cold cache + cold store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 2.4566408774200004,
"stddev": 0.2275829329168382,
"median": 2.5218089810200004,
"user": 2.10212818,
"system": 2.49490444,
"min": 2.09966515202,
"max": 2.82932251902,
"times": [
2.82932251902,
2.6000103180200003,
2.58340993902,
2.2678382690200003,
2.6067853430200003,
2.4602080230200003,
2.6084587000200004,
2.27673856402,
2.09966515202,
2.23397194702
]
},
{
"command": "pacquet@main",
"mean": 2.35753664512,
"stddev": 0.3489274948616896,
"median": 2.24000815802,
"user": 2.0984539799999995,
"system": 2.47609354,
"min": 2.03081571402,
"max": 3.1580120390200004,
"times": [
3.1580120390200004,
2.06085648102,
2.53113283802,
2.36768578102,
2.1082387280200003,
2.03081571402,
2.26750410602,
2.1633057350200002,
2.67530281902,
2.2125122100200003
]
}
]
}Scenario: Isolated linker: fresh restore, hot cache + hot store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 0.8062802473399999,
"stddev": 0.11413541015969599,
"median": 0.76622457294,
"user": 0.2857632,
"system": 1.0327461199999999,
"min": 0.72830877794,
"max": 1.10387462994,
"times": [
0.88408071394,
0.72830877794,
0.79174978994,
0.75116673394,
0.79639671294,
0.74545707994,
0.72931888894,
0.75263400294,
1.10387462994,
0.77981514294
]
},
{
"command": "pacquet@main",
"mean": 0.7505368386400002,
"stddev": 0.02367250598092555,
"median": 0.74256530594,
"user": 0.27311589999999997,
"system": 1.04466612,
"min": 0.73414326594,
"max": 0.80989705394,
"times": [
0.77321919394,
0.80989705394,
0.73414326594,
0.7429471359400001,
0.73629707694,
0.74751090494,
0.74218347594,
0.74454786494,
0.73490263794,
0.73971977594
]
}
]
}Scenario: Isolated linker: fresh install, cold cache + cold store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 2.4542280744600005,
"stddev": 0.16530176023290175,
"median": 2.39945773156,
"user": 3.03911338,
"system": 2.407408639999999,
"min": 2.26002281006,
"max": 2.7420432770600005,
"times": [
2.4629734930600002,
2.26002281006,
2.3246303470600003,
2.39689334606,
2.36583799406,
2.4020221170600005,
2.7420432770600005,
2.31543201806,
2.56849986906,
2.70392547306
]
},
{
"command": "pacquet@main",
"mean": 2.34975311766,
"stddev": 0.13221839937672794,
"median": 2.30795019006,
"user": 3.01900108,
"system": 2.3977972399999996,
"min": 2.24084593806,
"max": 2.59430741406,
"times": [
2.30549216706,
2.35287892906,
2.31040821306,
2.58943219906,
2.2793184400600004,
2.59430741406,
2.31912332506,
2.24899032806,
2.24084593806,
2.25673422306
]
}
]
}Scenario: Isolated linker: fresh install, hot cache + hot store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 1.4131425543199998,
"stddev": 0.05458893394674749,
"median": 1.41495722752,
"user": 1.3574363,
"system": 1.4672560000000001,
"min": 1.34638586602,
"max": 1.49416346802,
"times": [
1.34638586602,
1.4147625400200001,
1.44365742402,
1.45029734802,
1.38834800002,
1.49416346802,
1.41515191502,
1.34836915702,
1.3493344200200001,
1.48095540502
]
},
{
"command": "pacquet@main",
"mean": 1.4944150061200001,
"stddev": 0.10626653054988991,
"median": 1.48209901802,
"user": 1.3635203000000002,
"system": 1.4626891,
"min": 1.35512796502,
"max": 1.72371037702,
"times": [
1.37273677602,
1.5164135250200002,
1.42958888902,
1.35512796502,
1.72371037702,
1.56014902702,
1.49224517102,
1.47195286502,
1.55998059102,
1.46224487502
]
}
]
} |
Contributor
|
Bencher Report| Branch | pr/12102 |
| Testbed | pacquet |
Click to view all benchmark results
| Benchmark | Latency | Benchmark Result milliseconds (ms) (Result Δ%) | Upper Boundary milliseconds (ms) (Limit %) |
|---|---|---|---|
| isolated-linker.fresh-install.cold-cache.cold-store | 📈 view plot 🚷 view threshold | 2,454.23 ms(+2.62%)Baseline: 2,391.57 ms | 2,869.88 ms (85.52%) |
| isolated-linker.fresh-install.hot-cache.hot-store | 📈 view plot 🚷 view threshold | 1,413.14 ms(-7.02%)Baseline: 1,519.81 ms | 1,823.78 ms (77.48%) |
| isolated-linker.fresh-restore.cold-cache.cold-store | 📈 view plot 🚷 view threshold | 2,456.64 ms(+15.94%)Baseline: 2,118.98 ms | 2,542.77 ms (96.61%) |
| isolated-linker.fresh-restore.hot-cache.hot-store | 📈 view plot 🚷 view threshold | 806.28 ms(+19.30%)Baseline: 675.87 ms | 811.04 ms (99.41%) |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR ports pnpm’s update/up/upgrade behavior into pacquet by adding a dedicated Update operation and wiring it through the CLI, including support for compatible in-range bumps (via lockfile-seed withholding) and --latest manifest rewrites.
Changes:
- Add
pacquet updatecommand (withup/upgradealiases), selectors,--latest,--save-exact,--no-save,--depth,--lockfile-only, and interactive selection. - Introduce
UpdateSeedPolicyand thread it through the fresh-resolve install path to control which lockfile pins seed preferred versions. - Add unit + e2e coverage for selector parsing, CLI include-group behavior, and end-to-end update semantics.
Reviewed changes
Copilot reviewed 16 out of 17 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| pnpr/crates/pnpr/src/install_accelerator/resolve.rs | Passes update_seed_policy to installs initiated via pnpr. |
| pacquet/crates/package-manager/src/update.rs | Implements the Update operation (selector parsing, seed policy selection, manifest rewrites, install invocation). |
| pacquet/crates/package-manager/src/update/tests.rs | Unit tests for update selector parsing. |
| pacquet/crates/package-manager/src/lib.rs | Exposes the new update module publicly. |
| pacquet/crates/package-manager/src/install.rs | Threads UpdateSeedPolicy through install and bypasses optimistic repeat-install when updating. |
| pacquet/crates/package-manager/src/install_with_fresh_lockfile.rs | Defines UpdateSeedPolicy and applies it to lockfile preferred-version seeding. |
| pacquet/crates/package-manager/src/install/tests.rs | Updates install tests to provide update_seed_policy. |
| pacquet/crates/package-manager/src/add.rs | Ensures add uses UpdateSeedPolicy::KeepAll. |
| pacquet/crates/cli/tests/update.rs | Adds e2e tests for pacquet update behaviors and aliases. |
| pacquet/crates/cli/src/cli_args/update.rs | Adds CLI args for update and group-selection logic. |
| pacquet/crates/cli/src/cli_args/update/tests.rs | Unit tests for dependency-group selection logic. |
| pacquet/crates/cli/src/cli_args/update_interactive.rs | Implements --interactive selection using dialoguer. |
| pacquet/crates/cli/src/cli_args/install.rs | Ensures install CLI sets update_seed_policy: KeepAll. |
| pacquet/crates/cli/src/cli_args.rs | Registers the Update subcommand and its aliases. |
| pacquet/crates/cli/Cargo.toml | Adds dialoguer and node-semver dependencies for interactive update. |
| Cargo.toml | Adds workspace dependency on dialoguer. |
| Cargo.lock | Locks new Rust dependencies introduced by interactive update. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
… filter `UpdateSeedPolicy::DropOnly`'s snapshot filter allocated a `String` for every lockfile snapshot key. Parse the (small) update-target set to `PkgName` once and compare against `key.name` directly. Addresses a review comment on #12102. Written by an agent (Claude Code, claude-opus-4-8).
There was a problem hiding this comment.
🧹 Nitpick comments (1)
pacquet/crates/package-manager/src/install_with_fresh_lockfile.rs (1)
717-720: 💤 Low valueConsider logging when
PkgName::parsefails.Using
.ok()silently drops names that fail to parse, meaning those packages won't be filtered from the seed (opposite of intended behavior). While unlikely given upstream validation, a debug log would help diagnose issues if a selector unexpectedly doesn't trigger an update.💡 Optional: Add debug logging for parse failures
let drop: std::collections::HashSet<pacquet_lockfile::PkgName> = names .iter() - .filter_map(|name| pacquet_lockfile::PkgName::parse(name.as_str()).ok()) + .filter_map(|name| { + match pacquet_lockfile::PkgName::parse(name.as_str()) { + Ok(pkg_name) => Some(pkg_name), + Err(err) => { + tracing::debug!( + target: "pacquet::install", + name = %name, + ?err, + "update selector did not parse as PkgName; ignoring", + ); + None + } + } + }) .collect();🤖 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_with_fresh_lockfile.rs` around lines 717 - 720, The current collection into drop silently ignores parse errors from pacquet_lockfile::PkgName::parse (used in the names.iter().filter_map(...).collect()), so add a debug log when PkgName::parse fails: replace the filter_map usage with an explicit match/if let that on Err(e) emits a debug-level log (including the original name and error) and on Ok(name) includes it in the HashSet; keep the rest of the logic unchanged so only parse failures are recorded for diagnostics.
🤖 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_with_fresh_lockfile.rs`:
- Around line 717-720: The current collection into drop silently ignores parse
errors from pacquet_lockfile::PkgName::parse (used in the
names.iter().filter_map(...).collect()), so add a debug log when PkgName::parse
fails: replace the filter_map usage with an explicit match/if let that on Err(e)
emits a debug-level log (including the original name and error) and on Ok(name)
includes it in the HashSet; keep the rest of the logic unchanged so only parse
failures are recorded for diagnostics.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 7d411f4a-0797-48c2-8fd3-dbbb5db0f91e
📒 Files selected for processing (1)
pacquet/crates/package-manager/src/install_with_fresh_lockfile.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). (5)
- GitHub Check: ubuntu-latest / Node.js 24 / Test
- GitHub Check: Lint and Test (ubuntu-latest)
- GitHub Check: Lint and Test (macos-latest)
- 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: Log emissions are part of matching pnpm — when porting a function that firespnpm:<channel>events throughglobalLogger,logger.debug(...), orstreamParser.write(...), mirror the call site, payload, and ordering so@pnpm/cli.default-reporterparses pacquet's NDJSON the same way
Declare a newtype wrapper for branded string types instead of collapsing the brand into a plainStringor&strin Rust
If upstream TypeScript always validates before construction of a branded string, validate in the Rust wrapper too viaTryFrom<String>and/orFromStrand do not provide an infallible public constructor
If upstream TypeScript never validates a branded string, just brand for type-safety in Rust by exposing an infallibleFrom<String>constructor
If upstream TypeScript occasionally constructs a branded string without validation, exposefrom_str_uncheckedin Rust as an escape hatch alongside the validating constructor
Match upstream serde behavior for branded strings crossing JSON, YAML, or INI boundaries by using#[serde(try_from = "String")]for deserialization and#[serde(into = "String")]for serialization
Derive simple conversions for branded strings using#[derive(derive_more::From)]and#[derive(derive_more::Into)]instead of handwritingimplblocks; use manualimplonly when conversion needs custom logic
Model TypeScript string literal unions (like'auto' | 'always' | 'never') as Rustenums instead of newtype wrappers, since the set of valid values is closed
Treat TypeScript string template literal types (like`${string}@${string}`) the same as branded string types in Rust, using a newtype wrapper with validation
Follow the code style guide inCODE_STYLE_GUIDE.md— imports, modules, naming, ownership and borrowing, parameter type selection, trait bounds, pattern matching,pipe-trait, error handling, test layout, and cloning ofArcandRc
Choose owned vs. borrowed parameters to minimize copies; widen to t...
Files:
pacquet/crates/package-manager/src/install_with_fresh_lockfile.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/package-manager/src/install_with_fresh_lockfile.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/package-manager/src/install_with_fresh_lockfile.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/install_with_fresh_lockfile.rs
🔇 Additional comments (2)
pacquet/crates/package-manager/src/install_with_fresh_lockfile.rs (2)
188-200: LGTM!Also applies to: 171-171
721-726: LGTM!
This was referenced Jun 2, 2026
Merged
This was referenced Jun 15, 2026
This was referenced Jun 22, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ports pnpm's
update(aliasesup/upgrade) to pacquet, onto its always-fresh-resolve install path.Behavior
--latest): the matched names have their lockfile pins withheld from the preferred-versions seed (newUpdateSeedPolicy), so the resolver re-picks the highest version satisfying the manifest range.package.jsonis left untouched — this is exactly what distinguishesupdatefrominstall(which keeps the pin because it still satisfies the range). Mirrors pnpm'supdate: 'compatible'.--latest: each matched direct dependency'slatestdist-tag is fetched and written intopackage.json(^<version>, or exact under--save-exact), then the install resolves the new range. Mirrors pnpm'supdateToLatest+updateSpec.@scope/bar-*) withdepth > 0and no--latestmatch every locked package name at any depth (like pnpm'supdateMatching(infoFromLockfile.name, …)); versioned (foo@2) or--latestselectors match direct deps only;--latestcombined with a spec is rejected (ERR_PNPM_LATEST_WITH_SPEC).-L/--latest,-E/--save-exact,-P/-D/--no-optional(faithfulmakeIncludeDependenciesFromCLI, including the quirk that--no-optionalonly drops optionals alongside--prod/--dev),--depth,--lockfile-only,-i/--interactive.dialoguer(added to[workspace.dependencies]) plus an inline outdated check (current from the lockfile, target from one packument fetch).The resolver's
UpdateBehaviortri-state and the npm resolver'sinclude_latest_tagalready existed — this PR drives them from a new CLI command andpacquet_package_manager::Updateoperation.Deferred (error out rather than misbehave) → tracked in #12101
--global/-g(global-dir +@pnpm/global.commandsnot ported)--workspace(workspace-protocol version linking not ported)-r, sibling-importerpackage.jsonfiles aren't rewritten on--latest(only the root manifest is); resolution still spans all importers.Tests
New e2e suite
pacquet/crates/cli/tests/update.rs: compatible bump within range,--latestmanifest rewrite,--save-exact, scoped selector,upalias, andLATEST_WITH_SPECrejection. Full Rust suite green;cargo check/clippy -D warnings/cargo doc -D warnings/ dylint (perfectionist) /fmt --check/typosall clean.No changeset (pacquet-only change). This is the pnpm → pacquet porting direction, so no TypeScript-side mirror is needed.
Written by an agent (Claude Code, claude-opus-4-8).
Summary by CodeRabbit
New Features
pacquet update(aliases:up,upgrade) with flags:--interactive,--latest,--save-exact,--no-save,--prod,--dev,--no-optional,--depth--interactiveprompts to select outdated direct depsadd/installnow preserve existing lockfile pins by default during updatesTests