feat(pacquet): honor updateConfig.ignoreDependencies in update#12104
Conversation
When `pacquet update` is run with no package selectors, the names in `updateConfig.ignoreDependencies` (from pnpm-workspace.yaml) are turned into negation selectors (`!name`) so the update covers everything except them. Explicit CLI selectors override the ignore list, matching pnpm. Ports pnpm's `installDeps` gate + `makeIgnorePatterns`, and the "ignore packages in updateConfig.ignoreDependencies" and "do not update anything if all dependencies are ignored" tests from `installing/commands/test/update/update.ts`. Follow-up to #12101. Written by an agent (Claude Code, claude-opus-4-8).
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughApply updateConfig.ignoreDependencies when no package selectors are provided, omit ignored names from update seeds and manifest rewrites, thread a shared Arc into per-importer ResolveOptions, and add test helper plus CLI tests for ignore-dependency scenarios. ChangesIgnore Dependencies Feature
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
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 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 |
Micro-Benchmark ResultsLinux |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #12104 +/- ##
==========================================
+ Coverage 87.04% 87.08% +0.04%
==========================================
Files 252 252
Lines 28146 28236 +90
==========================================
+ Hits 24499 24590 +91
+ Misses 3647 3646 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Review Summary by Qodo(Agentic_describe updated until commit 0970bc9)Honor updateConfig.ignoreDependencies in update and fix preferred-versions seeding
WalkthroughsDescription• Honor updateConfig.ignoreDependencies in pacquet update command - Excludes listed packages from no-selector updates to keep lockfile pins - Explicit CLI selectors override ignore list, matching pnpm behavior - Respects group narrowing (--prod/--dev/--no-optional) • Fix preferred-versions seed not fed to version picker in fresh-resolve path - Prevents unintended bumps to highest in-range when lockfile pins still satisfy ranges - Ensures non-targeted pins remain unchanged during selective updates • Add comprehensive test coverage for ignore dependencies across update scenarios - Tests --latest, compatible updates, --prod scoping, and edge cases Diagramflowchart LR
A["Update Command"] --> B["Parse Selectors"]
B --> C["No Selectors?"]
C -->|Yes| D["Load ignoreDependencies"]
D --> E["Filter Direct Deps"]
E --> F["Apply Update Policy"]
C -->|No| G["Use CLI Selectors"]
G --> F
F --> H["Seed Preferred Versions"]
H --> I["Resolve & Update"]
I --> J["Updated Lockfile"]
File Changes1. pacquet/crates/cli/tests/update.rs
|
Integrated-Benchmark Report (Linux)Scenario: Isolated linker: fresh restore, cold cache + cold store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 2.20793809598,
"stddev": 0.14056544490355977,
"median": 2.18168624058,
"user": 2.61112624,
"system": 3.40196644,
"min": 2.02341590158,
"max": 2.51740541058,
"times": [
2.25096181858,
2.33617247158,
2.17069235458,
2.08755155958,
2.02341590158,
2.16831078858,
2.51740541058,
2.19268012658,
2.2313995755800002,
2.10079095258
]
},
{
"command": "pacquet@main",
"mean": 2.17675386238,
"stddev": 0.06869159761787877,
"median": 2.18391546258,
"user": 2.6422152399999996,
"system": 3.40386184,
"min": 2.09093817258,
"max": 2.25490408958,
"times": [
2.10606190558,
2.11584820858,
2.24736213858,
2.11083909358,
2.22876025058,
2.25490408958,
2.14211859158,
2.24499383958,
2.09093817258,
2.22571233358
]
}
]
}Scenario: Isolated linker: fresh restore, hot cache + hot store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 0.6962684671,
"stddev": 0.02727742336233797,
"median": 0.6899286740999999,
"user": 0.35883721999999996,
"system": 1.3662147599999999,
"min": 0.6708838906,
"max": 0.7567368526,
"times": [
0.7567368526,
0.6944895656,
0.7294802316,
0.6747878646000001,
0.6754930126,
0.6708838906,
0.7003426506,
0.6949371536,
0.6853677826,
0.6801656666
]
},
{
"command": "pacquet@main",
"mean": 0.6837659945000001,
"stddev": 0.05673548683449874,
"median": 0.6570359841,
"user": 0.35777222,
"system": 1.33593116,
"min": 0.6379142926,
"max": 0.7927341676,
"times": [
0.7802201586,
0.6614127766,
0.6479694716,
0.6379142926,
0.6456833066000001,
0.6964184816,
0.6740528186,
0.6526591916,
0.6485952796,
0.7927341676
]
}
]
}Scenario: Isolated linker: fresh install, cold cache + cold store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 2.4553501924399996,
"stddev": 0.031204870697229065,
"median": 2.45900995294,
"user": 3.81604916,
"system": 3.18116192,
"min": 2.40805769394,
"max": 2.51693597294,
"times": [
2.45493101894,
2.4208308069399997,
2.47804887494,
2.43351578894,
2.46315722094,
2.4719724139399997,
2.51693597294,
2.4429632459399997,
2.46308888694,
2.40805769394
]
},
{
"command": "pacquet@main",
"mean": 2.34929880074,
"stddev": 0.04327887544912064,
"median": 2.33883450594,
"user": 3.7691124599999997,
"system": 3.1710580199999994,
"min": 2.28594337994,
"max": 2.4190161729399997,
"times": [
2.4059981379399997,
2.38971025494,
2.4190161729399997,
2.32517644394,
2.3558843179399998,
2.30738175694,
2.33945440694,
2.28594337994,
2.33821460494,
2.32620853094
]
}
]
}Scenario: Isolated linker: fresh install, hot cache + hot store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 1.55327809278,
"stddev": 0.03648556550745488,
"median": 1.55128099028,
"user": 1.7247077799999997,
"system": 1.8885631399999998,
"min": 1.5092218327799998,
"max": 1.61840721978,
"times": [
1.55063290478,
1.5131903007799998,
1.5201942607799999,
1.54648356978,
1.55428457878,
1.5092218327799998,
1.61840721978,
1.55192907578,
1.6079442717799999,
1.56049291278
]
},
{
"command": "pacquet@main",
"mean": 1.58399474798,
"stddev": 0.04830852604083104,
"median": 1.58620449328,
"user": 1.7393181799999997,
"system": 1.90901014,
"min": 1.51874161778,
"max": 1.69281808878,
"times": [
1.5390088467799998,
1.51874161778,
1.69281808878,
1.5992032647799999,
1.54309427478,
1.58228778978,
1.56905894378,
1.60143794078,
1.59012119678,
1.60417551578
]
}
]
} |
|
Bencher Report| Branch | pr/12104 |
| 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,455.35 ms(+2.39%)Baseline: 2,397.95 ms | 2,877.54 ms (85.33%) |
| isolated-linker.fresh-install.hot-cache.hot-store | 📈 view plot 🚷 view threshold | 1,553.28 ms(+1.95%)Baseline: 1,523.62 ms | 1,828.35 ms (84.96%) |
| isolated-linker.fresh-restore.cold-cache.cold-store | 📈 view plot 🚷 view threshold | 2,207.94 ms(+3.81%)Baseline: 2,126.83 ms | 2,552.20 ms (86.51%) |
| isolated-linker.fresh-restore.hot-cache.hot-store | 📈 view plot 🚷 view threshold | 696.27 ms(+2.04%)Baseline: 682.35 ms | 818.82 ms (85.03%) |
There was a problem hiding this comment.
Pull request overview
This PR updates pacquet’s update command to honor updateConfig.ignoreDependencies (from pnpm-workspace.yaml) when the user runs pacquet update with no explicit package selectors, mirroring pnpm’s “ignore list only applies when params are empty” behavior.
Changes:
- In
Update::run, synthesize negation selectors (!name) fromconfig.update_config.ignore_dependencieswhenpackages.is_empty(). - Parse selectors and apply the
--latest“LATEST_WITH_SPEC” guard against the synthesized selector inputs. - Add e2e tests covering
update --latestwith an ignore list (including the “all ignored = no-op” case).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| pacquet/crates/package-manager/src/update.rs | Applies updateConfig.ignoreDependencies by converting it to negation selector inputs when no CLI selectors are provided. |
| pacquet/crates/cli/tests/update.rs | Adds a helper to append updateConfig.ignoreDependencies to pnpm-workspace.yaml and adds two new --latest regression tests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
… picker The fresh-resolve path built `get_preferred_versions_from_lockfile_and_manifests` but only ever used it for peer hoisting — it was never written into `ResolveOptions.preferred_versions`, so the npm picker ignored it and always took the highest version in range. That meant a fresh re-resolve (stale lockfile / `preferFrozenLockfile: false`) bumped *every* dependency to its highest in-range version instead of keeping the lockfile pins for the ones that still satisfy their range — diverging from pnpm's `update: false` behavior, and making `pacquet update <selector>` / `updateConfig.ignoreDependencies` unable to keep non-targeted pins. Seed `base_opts.preferred_versions` from the same map so the picker biases toward the locked versions, matching pnpm. Fresh installs with no lockfile are unaffected (the manifest-only seed carries no concrete version pin); all 2693 workspace tests still pass. Written by an agent (Claude Code, claude-opus-4-8).
…ed groups Apply `ignoreDependencies` as an exclusion filter over the included direct deps inside the no-selector branch, instead of injecting global `!name` negation selectors. The injection route took the name-matcher path and scanned every lockfile snapshot name, which dropped pins for transitive deps too and ignored `--prod`/`--dev`/`--no-optional` group narrowing (Copilot review on #12104). Now group narrowing still scopes the update while ignored names keep their pins. Adds regression tests: compatible (non-`--latest`) update honoring the ignore list, and `--prod` scoping with an ignored prod dep + an untouched devDependency. Guards the test helper against duplicate `updateConfig:` YAML keys. Written by an agent (Claude Code, claude-opus-4-8).
`ResolveOptions.preferred_versions` was a `PreferredVersions` map cloned into `direct_opts`/`subdep_opts` per depth tier by the tree walker and per importer by the install layer. Now that it carries the lockfile + manifest seed, those deep clones can be sizeable. Hold it behind `Arc` and build the seed once, so each clone is a refcount bump. The two read sites (`npm_resolver`, `named_registry_resolver`) reach it through `Arc` deref unchanged. Addresses a review comment on #12104. Written by an agent (Claude Code, claude-opus-4-8).
`create_matcher` was built unconditionally in the no-selector update branch even when `ignoreDependencies` is empty. Wrap it in an `Option` so the common (no ignore list) path skips the compile. Addresses a review comment on #12104. Written by an agent (Claude Code, claude-opus-4-8).
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pacquet/crates/package-manager/src/update.rs (1)
252-264:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAvoid re-expanding to the full lockfile when every included direct dep was ignored.
If
ignoreDependenciesfilters out all included direct deps,drop_namesis empty after the direct scan, but this branch still inserts every non-ignored snapshot name. That turns a “nothing selected” update into a transitive-only re-resolve, so an all-ignoredpacquet updateis no longer a no-op.Based on learnings, pacquet should “only match pnpm’s behavior exactly.”Suggested guard
- } else if updates_all_groups { + } else if updates_all_groups && !drop_names.is_empty() { // Whole-graph update minus the ignored names: drop every // locked name that isn't ignored so the ignored ones keep // their pins. if let Some(snapshots) = lockfile.and_then(|lf| lf.snapshots.as_ref()) { for key in snapshots.keys() { @@ } UpdateSeedPolicy::DropOnly(drop_names) } else {🤖 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/update.rs` around lines 252 - 264, The current updates_all_groups branch repopulates drop_names from lockfile.snapshots even when the direct-scan produced an empty drop_names (e.g., all included direct deps were ignored), causing a full transitive re-resolve; change the logic in the updates_all_groups branch so that you only iterate lockfile.snapshots and insert snapshot keys into drop_names when drop_names is already non-empty after the direct dependency scan (i.e., only expand to non-ignored snapshots if there is at least one direct package selected to drop). Update the block around updates_all_groups / lockfile.and_then(|lf| lf.snapshots.as_ref()) to check drop_names.is_empty() and skip the snapshot loop when empty before returning UpdateSeedPolicy::DropOnly(drop_names), preserving the no-op behavior when all included direct deps are ignored.
🤖 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.
Outside diff comments:
In `@pacquet/crates/package-manager/src/update.rs`:
- Around line 252-264: The current updates_all_groups branch repopulates
drop_names from lockfile.snapshots even when the direct-scan produced an empty
drop_names (e.g., all included direct deps were ignored), causing a full
transitive re-resolve; change the logic in the updates_all_groups branch so that
you only iterate lockfile.snapshots and insert snapshot keys into drop_names
when drop_names is already non-empty after the direct dependency scan (i.e.,
only expand to non-ignored snapshots if there is at least one direct package
selected to drop). Update the block around updates_all_groups /
lockfile.and_then(|lf| lf.snapshots.as_ref()) to check drop_names.is_empty() and
skip the snapshot loop when empty before returning
UpdateSeedPolicy::DropOnly(drop_names), preserving the no-op behavior when all
included direct deps are ignored.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 75c7058b-2c2a-400a-9669-903d844f360c
📒 Files selected for processing (3)
pacquet/crates/package-manager/src/install_with_fresh_lockfile.rspacquet/crates/package-manager/src/update.rspacquet/crates/resolving-resolver-base/src/resolve.rs
🚧 Files skipped from review as they are similar to previous changes (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). (10)
- GitHub Check: Doc
- GitHub Check: Lint and Test (windows-latest)
- GitHub Check: Code Coverage
- GitHub Check: Lint and Test (macos-latest)
- GitHub Check: Lint and Test (ubuntu-latest)
- GitHub Check: Dylint
- GitHub Check: Analyze (javascript)
- GitHub Check: Run benchmark on ubuntu-latest
- GitHub Check: Run benchmark on ubuntu-latest
- GitHub Check: Compile & Lint
🧰 Additional context used
📓 Path-based instructions (1)
pacquet/**/*.rs
📄 CodeRabbit inference engine (pacquet/AGENTS.md)
pacquet/**/*.rs: 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/resolving-resolver-base/src/resolve.rspacquet/crates/package-manager/src/update.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/resolving-resolver-base/src/resolve.rspacquet/crates/package-manager/src/update.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/resolving-resolver-base/src/resolve.rspacquet/crates/package-manager/src/update.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/resolving-resolver-base/src/resolve.rspacquet/crates/package-manager/src/update.rs
🔇 Additional comments (2)
pacquet/crates/package-manager/src/update.rs (1)
230-235: LGTM!pacquet/crates/resolving-resolver-base/src/resolve.rs (1)
197-203: LGTM!
…ignored When a no-selector update's included direct deps are all excluded by `updateConfig.ignoreDependencies`, `--latest` must not re-resolve the non-ignored indirect deps — pnpm returns early there (`if (opts.latest) return`). The whole-graph expansion now skips when `latest && drop_names.is_empty()`. A non-`--latest` update keeps expanding in that case: pnpm explicitly "updates indirect dependencies only" when no direct dep matches, so the indirect deps still re-resolve. Two regression tests pin both halves (the second guards against narrowing the condition to an unconditional `drop_names.is_empty()`). Written by an agent (Claude Code, claude-opus-4-8).
There was a problem hiding this comment.
🧹 Nitpick comments (1)
pacquet/crates/cli/tests/update.rs (1)
299-306: ⚡ Quick winAdd diagnostic logging before non-
assert_eq!assertions in these tests.These
assert!checks currently fail without enough context to debug quickly in CI. Please add a short log (ordbg!) right before each non-assert_eq!assertion group.Suggested adjustment
+ eprintln!("after update --latest, checking virtual store for indirect dep versions"); assert!(virtual_store_has(&workspace, "`@pnpm.e2e`+dep-of-pkg-with-1-dep@100.0.0")); assert!(!virtual_store_has(&workspace, "`@pnpm.e2e`+dep-of-pkg-with-1-dep@100.1.0")); ... + eprintln!("after compatible update, checking indirect dep bumped within range"); assert!(virtual_store_has(&workspace, "`@pnpm.e2e`+dep-of-pkg-with-1-dep@100.1.0"));As per coding guidelines: “Follow test-logging guidance — log before non-
assert_eq!assertions,dbg!complex structures, skip logging for simple scalarassert_eq!”.Also applies to: 327-328
🤖 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/cli/tests/update.rs` around lines 299 - 306, Before each non-assert_eq! assertion in this test add a short diagnostic log or dbg! of the relevant state so failing CI runs show context: log the workspace manifest and the return values from virtual_store_has before each assert!(virtual_store_has(...)) call (the two checks after pacquet(... ["update","--latest"]) and the other pair referenced at 327-328), and also log any inputs to write_manifest or pacquet that affect the check; locate the assertions by the symbol virtual_store_has and the test-local helpers write_manifest and pacquet and insert concise dbg!/eprintln! statements immediately above each assert! line.
🤖 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/cli/tests/update.rs`:
- Around line 299-306: Before each non-assert_eq! assertion in this test add a
short diagnostic log or dbg! of the relevant state so failing CI runs show
context: log the workspace manifest and the return values from virtual_store_has
before each assert!(virtual_store_has(...)) call (the two checks after
pacquet(... ["update","--latest"]) and the other pair referenced at 327-328),
and also log any inputs to write_manifest or pacquet that affect the check;
locate the assertions by the symbol virtual_store_has and the test-local helpers
write_manifest and pacquet and insert concise dbg!/eprintln! statements
immediately above each assert! line.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: b1b5e9cf-ce80-4f79-805c-9041daa9989c
📒 Files selected for processing (2)
pacquet/crates/cli/tests/update.rspacquet/crates/package-manager/src/update.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- pacquet/crates/package-manager/src/update.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: 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 (2)
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/cli/tests/update.rs
pacquet/**/tests/**/*.rs
📄 CodeRabbit inference engine (pacquet/AGENTS.md)
pacquet/**/tests/**/*.rs: Prefer real fixtures over dependency-injection seams — usetempfile::TempDir, the mocked registry, or integration tests spawning the actual binary for happy paths and most error paths; use the DI seam only for filesystem error kinds, deterministic time, shared process-global state, or external-service happy paths
Follow test-logging guidance — log before non-assert_eq!assertions,dbg!complex structures, skip logging for simple scalarassert_eq!
Tests must not be tolerant of missing build/runtime environment by silently returning early when a tool isn't found — let the test panic when a required tool is absent
Prefer#[cfg_attr(target_os = "windows", ignore = "...")](or matching#[cfg(unix)]gates) over runtime probe-and-skip helpers for platform-locked tools
Port relevant pnpm tests to Rust tests whenever they translate when porting behavior from pnpm
Use snapshot tests withinstaand carefully review diffs when intentional changes alter snapshots; accept withcargo insta reviewonly after careful review
Tests that need the mocked registry should startpnprthroughpacquet-testing-utils;cargo test/cargo nextest runshould not require a separatejust registry-mock launchstep
Files:
pacquet/crates/cli/tests/update.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/cli/tests/update.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/cli/tests/update.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/tests/update.rs
🔇 Additional comments (1)
pacquet/crates/cli/tests/update.rs (1)
11-13: LGTM!
Per pacquet's test-logging convention (log before non-`assert_eq!` assertions), add a `list_virtual_store` helper and `eprintln!` the `node_modules/.pnpm` contents before each `virtual_store_has` assertion cluster, so a failing CI run shows what was actually materialized. Addresses a review nit on #12104. Written by an agent (Claude Code, claude-opus-4-8).
`Arc::new(all_preferred_versions.clone())` deep-cloned the map once just to build the shared seed. Move it into the `Arc` instead and clone the map out of the seed for the per-importer hoist copy (which mutates its own copy). Saves the one-time clone. Addresses a review comment on #12104. Written by an agent (Claude Code, claude-opus-4-8).
Follow-up to #12102, addressing one item from #12101.
When
pacquet updateruns with no package selectors, the names listed inupdateConfig.ignoreDependencies(frompnpm-workspace.yaml) are excluded from the update so they keep their pins. Explicit CLI selectors take precedence over the ignore list — matching pnpm, which only appliesignoreDependencieswhenparams.length === 0. The exclusion is applied over the included direct deps, so--prod/--dev/--no-optionalgroup narrowing still scopes the update.Ports pnpm's
installDepsgate +makeIgnorePatterns.Also: a resolver fix this surfaced
Writing the compatible-update ignore test revealed that the fresh-resolve path never fed the preferred-versions seed to the version picker —
get_preferred_versions_from_lockfile_and_manifestswas built but only used for peer hoisting, never written intoResolveOptions.preferred_versions. So a fresh re-resolve bumped every dep to its highest in-range version instead of keeping the lockfile pins that still satisfy their range, diverging from pnpm'supdate: falsebehavior (and makingupdate <selector>/ignoreDependenciesunable to keep non-targeted pins). Seedingbase_opts.preferred_versionsfixes it; fresh installs with no lockfile are unaffected, and all 2693 workspace tests still pass.Tests
Ports four cases from
installing/commands/test/update/update.ts(adapted to pacquet's static fixtures, wherelatest = max(version)):update --latestignores the listed dep and bumps the rest.update --latestwith every dependency ignored is a no-op.--latest) update keeps the ignored dep's resolved version while the rest bump.--prodscoping: an ignored prod dep keeps its range and a devDependency is left untouched.All
updatee2e tests + full workspace suite (2693) +clippy -D warnings/ dylint /fmt --check/typospass.No changeset (pacquet-only change).
Written by an agent (Claude Code, claude-opus-4-8).
Summary by CodeRabbit
New Features
Bug Fixes
update(including--latestand scoped--prod) now correctly honors ignore settings; when all direct deps are ignored,update --latestis a no-op and indirect deps behave appropriately.Tests
Refactor