Conversation
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📜 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). (9)
🧰 Additional context used📓 Path-based instructions (1)pacquet/**/*.rs📄 CodeRabbit inference engine (pacquet/AGENTS.md)
Files:
🧠 Learnings (3)📚 Learning: 2026-05-20T19:40:55.051ZApplied to files:
📚 Learning: 2026-05-22T00:08:44.646ZApplied to files:
📚 Learning: 2026-05-20T23:07:58.444ZApplied to files:
🔇 Additional comments (2)
📝 WalkthroughWalkthroughAdds an optimistic repeat-install fast-path: new module checks cached workspace-state, settings, project list, node_modules presence, and manifest mtimes to decide UpToDate vs Skipped; integrates the check into Install::run (early workspace scan), exposes a Config flag, and adds unit + integration tests including frozen-lockfile polarity. ChangesOptimistic repeat-install fast-path
Sequence Diagram(s)sequenceDiagram
participant CLI
participant Install
participant OptimisticCheck
participant WorkspaceState
participant FS
CLI->>Install: run install
Install->>OptimisticCheck: provide workspace_root, config, node_linker, included, project_manifests
OptimisticCheck->>WorkspaceState: read .pnpm-workspace-state-v1.json
OptimisticCheck->>FS: stat package.json files & check node_modules dirs
OptimisticCheck->>Install: Decision::UpToDate or Decision::Skipped
Install->>CLI: continue full pipeline or return early
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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 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 |
Micro-Benchmark ResultsLinux |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/package-manager/src/install/tests.rs`:
- Line 5116: The test currently only drops the temporary directory, leaking the
mock instance; update the cleanup to drop both resources by including
mock_instance in the drop call (i.e., drop the tuple containing dir and
mock_instance) so that the mock_instance variable is dropped and its resources
are released (refer to the mock_instance variable and the existing drop(...)
call near the end of the test).
🪄 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: 58046ebd-b8ab-44d5-a0a8-04098ba04ecd
📒 Files selected for processing (2)
pacquet/crates/package-manager/src/install.rspacquet/crates/package-manager/src/install/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). (9)
- GitHub Check: Code Coverage
- GitHub Check: Lint and Test (ubuntu-latest)
- GitHub Check: Run benchmark on ubuntu-latest
- GitHub Check: Lint and Test (windows-latest)
- GitHub Check: Lint and Test (macos-latest)
- GitHub Check: Run benchmark on ubuntu-latest
- GitHub Check: Dylint
- GitHub Check: Analyze (javascript)
- GitHub Check: Compile & Lint
🧰 Additional context used
📓 Path-based instructions (1)
pacquet/**/*.rs
📄 CodeRabbit inference engine (pacquet/AGENTS.md)
pacquet/**/*.rs: When porting a function that firespnpm:<channel>events throughglobalLogger,logger.debug(), orstreamParser.write(), mirror the call site, payload, and ordering so the reporter parses pacquet's NDJSON the same way it parses pnpm's.
Declare a newtype wrapper for branded string types. Do not collapse the brand into a plainStringor&str.
If upstream always validates before construction, validate in pacquet's wrapper too. The wrapper must construct only viaTryFrom<String>and/orFromStr. Do not provide an infallible public constructor.
If upstream never validates, just brand for type-safety. Expose an infallibleFrom<String>(andFrom<&str>when convenient).
If upstream occasionally constructs without validation, exposefrom_str_uncheckedas an escape hatch alongside the validating constructor.
Match upstream serde behavior for branded types that cross JSON, YAML, or INI boundaries. Use#[serde(try_from = "String")]for deserialization and#[serde(into = "String")]for serialization.
Use#[derive(derive_more::From)]and#[derive(derive_more::Into)]for mechanical conversion impls. Fall back to manualimplonly when conversion needs custom logic.
String-literal unions should becomeenums, not newtype wrappers. Model closed sets of valid string values as enums.
Template literal types should be treated as branded strings with validation discipline from rules 2-5.
Choose owned vs. borrowed parameters to minimize copies. Widen to the most encompassing type (&Pathover&PathBuf,&strover&String) when it doesn't force extra copies.
PreferArc::clone(&x)/Rc::clone(&x)overx.clone()for reference-counted types, so the cost is visible at the call site.
Follow Rust API Guidelines for naming conventions.
Do not use star imports inside module bodies. Writeuse super::{Foo, bar}instead ofuse super::*;. Two forms stay allowed: external-crate preludes likeuse rayon::prelude::*;and root-of-module re-...
Files:
pacquet/crates/package-manager/src/install/tests.rspacquet/crates/package-manager/src/install.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/tests.rspacquet/crates/package-manager/src/install.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/tests.rspacquet/crates/package-manager/src/install.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/tests.rspacquet/crates/package-manager/src/install.rs
🔇 Additional comments (6)
pacquet/crates/package-manager/src/install.rs (5)
486-488: LGTM!
532-557: LGTM!
559-602: LGTM!
604-628: LGTM!
630-654: LGTM!pacquet/crates/package-manager/src/install/tests.rs (1)
4853-5001: LGTM!
Integrated-Benchmark Report (Linux)Scenario: Isolated linker: fresh restore, cold cache + cold store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 2.0197359986400003,
"stddev": 0.05507855701239871,
"median": 2.01287262274,
"user": 2.7662617800000002,
"system": 3.4183973200000004,
"min": 1.93236290824,
"max": 2.11626067924,
"times": [
2.11626067924,
1.9895135162400002,
2.01885594824,
2.04357312824,
1.93236290824,
2.08544413624,
2.00688929724,
1.9718464402400002,
2.04847722424,
1.98413670824
]
},
{
"command": "pacquet@main",
"mean": 1.9669094181399998,
"stddev": 0.051324750657947775,
"median": 1.95822459724,
"user": 2.7298167799999997,
"system": 3.38511972,
"min": 1.89289706924,
"max": 2.03455202624,
"times": [
2.03455202624,
1.96994477224,
2.0219993932399998,
2.02646337424,
1.90632408424,
1.99871449224,
1.89289706924,
1.93162206824,
1.94007247924,
1.94650442224
]
}
]
}Scenario: Isolated linker: fresh restore, hot cache + hot store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 0.6466068423800002,
"stddev": 0.03045047597187396,
"median": 0.6391868103800001,
"user": 0.37704804000000003,
"system": 1.33240598,
"min": 0.6236229923800001,
"max": 0.7301054443800001,
"times": [
0.7301054443800001,
0.6397499273800001,
0.6349967083800001,
0.6275085433800001,
0.6347112353800001,
0.6391726903800001,
0.6440578793800001,
0.6236229923800001,
0.6529420723800001,
0.6392009303800001
]
},
{
"command": "pacquet@main",
"mean": 0.6491464550800001,
"stddev": 0.021755212779682,
"median": 0.6458845203800001,
"user": 0.36691524,
"system": 1.35409268,
"min": 0.6275436593800001,
"max": 0.7054811403800001,
"times": [
0.7054811403800001,
0.6453695783800001,
0.6438886073800001,
0.6463994623800001,
0.6509147113800001,
0.6397594433800001,
0.6279365533800001,
0.6496888943800001,
0.6544825003800001,
0.6275436593800001
]
}
]
}Scenario: Isolated linker: fresh install, cold cache + cold store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 2.27755885608,
"stddev": 0.0308656786719531,
"median": 2.2730485962799998,
"user": 3.8674139399999996,
"system": 3.1194423,
"min": 2.22959084978,
"max": 2.32939877878,
"times": [
2.2569833197799998,
2.32624017178,
2.26577817778,
2.2714912207799998,
2.22959084978,
2.27546655978,
2.25624294378,
2.28979056678,
2.32939877878,
2.27460597178
]
},
{
"command": "pacquet@main",
"mean": 2.26797746638,
"stddev": 0.0184670474034304,
"median": 2.26116973278,
"user": 3.7839952400000008,
"system": 3.1403461,
"min": 2.25130019578,
"max": 2.31135980678,
"times": [
2.25760964178,
2.2715160867799997,
2.31135980678,
2.25908590278,
2.25509302678,
2.25130019578,
2.27907859878,
2.2632535627799997,
2.25180204778,
2.27967579378
]
}
]
}Scenario: Isolated linker: fresh install, hot cache + hot store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 1.4181379473,
"stddev": 0.027336790868007887,
"median": 1.4145712262,
"user": 1.7307876799999995,
"system": 1.8134590400000001,
"min": 1.3676959772,
"max": 1.4589453232,
"times": [
1.4589453232,
1.3676959772,
1.4172993762,
1.4444744082,
1.4045163752,
1.4437489342,
1.3951909172,
1.4118430762,
1.4332048802,
1.4044602052
]
},
{
"command": "pacquet@main",
"mean": 1.4240024723000002,
"stddev": 0.056785583408625134,
"median": 1.4101787787,
"user": 1.7067976799999998,
"system": 1.8061771400000002,
"min": 1.3722886762,
"max": 1.5726869302,
"times": [
1.4077024592,
1.4067775171999999,
1.3986508662,
1.4126550982,
1.5726869302,
1.4518793312,
1.4188155132,
1.3722886762,
1.4202623462,
1.3783059852
]
}
]
} |
|
Bencher Report| Branch | pr/11943 |
| 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,277.56 ms(-38.44%)Baseline: 3,699.74 ms | 4,439.69 ms (51.30%) |
| isolated-linker.fresh-install.hot-cache.hot-store | 📈 view plot 🚷 view threshold | 1,418.14 ms(-48.48%)Baseline: 2,752.60 ms | 3,303.11 ms (42.93%) |
| isolated-linker.fresh-restore.cold-cache.cold-store | 📈 view plot 🚷 view threshold | 2,019.74 ms(-7.78%)Baseline: 2,190.05 ms | 2,628.06 ms (76.85%) |
| isolated-linker.fresh-restore.hot-cache.hot-store | 📈 view plot 🚷 view threshold | 646.61 ms(-1.47%)Baseline: 656.25 ms | 787.50 ms (82.11%) |
Closes #11940. Port of upstream's `optimisticRepeatInstall` + `checkDepsStatus` dispatch from `installing/commands/src/installDeps.ts:179-194` and `deps/status/src/checkDepsStatus.ts`. `Install::run` now runs a workspace-state freshness check before any of the install setup — read `.pnpm-workspace-state-v1.json`, compare its `lastValidatedTimestamp` against each project's `package.json` mtime, and emit "Already up to date" + return when nothing has changed. That short-circuit beats the lockfile-verifier fan-out (no `<cache_dir>/lockfile-verified.jsonl` lookup), the lockfile load, and `getContext`. Cold-cache repeat installs now skip the same work pnpm skips in the bench's `lockfile+node_modules` cells. Scope: the mtime-vs-`lastValidatedTimestamp` exit only. Branches that detect a modified project and then re-verify the lockfile (`assertWantedLockfileUpToDate`, `patchesOrHooksAreModified`) aren't ported here — falling through to the regular install path retains the existing freshness guards (`check_lockfile_freshness`, the no-op short-circuit). The new `optimistic_repeat_install: bool` Config field defaults to `true`, matching `config/reader/src/index.ts:169`. Disabled under `--frozen-lockfile` so a headless install still fails loudly on missing / stale lockfiles. The settings construction inside `build_workspace_state` and the new `check_optimistic_repeat_install` is shared via `optimistic_repeat_install::current_settings` so writer and checker can't drift.
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/package-manager/src/optimistic_repeat_install/tests.rs`:
- Around line 15-58: Delete the duplicate current_settings function in the tests
file and replace all local calls to current_settings(...) with
super::current_settings(...); specifically remove the fn current_settings(...)
definition in optimistic_repeat_install/tests.rs and update test helpers like
setup_fresh_install and other test functions that call current_settings to call
super::current_settings instead so they reuse the crate-visible pub(crate)
function in the parent module; ensure imports/argument types match the parent's
signature and remove any now-unused local helper imports.
🪄 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: 507c8e3f-bff7-4c7b-bf26-5b336b2433e9
📒 Files selected for processing (8)
pacquet/crates/config/src/lib.rspacquet/crates/config/src/workspace_yaml.rspacquet/crates/package-manager/src/install.rspacquet/crates/package-manager/src/install/tests.rspacquet/crates/package-manager/src/install_package_from_registry/tests.rspacquet/crates/package-manager/src/lib.rspacquet/crates/package-manager/src/optimistic_repeat_install.rspacquet/crates/package-manager/src/optimistic_repeat_install/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). (7)
- GitHub Check: Run benchmark on ubuntu-latest
- GitHub Check: Lint and Test (ubuntu-latest)
- GitHub Check: Lint and Test (windows-latest)
- GitHub Check: Lint and Test (macos-latest)
- GitHub Check: Run benchmark on ubuntu-latest
- GitHub Check: Code Coverage
- GitHub Check: Compile & Lint
🧰 Additional context used
📓 Path-based instructions (1)
pacquet/**/*.rs
📄 CodeRabbit inference engine (pacquet/AGENTS.md)
pacquet/**/*.rs: When porting a function that firespnpm:<channel>events throughglobalLogger,logger.debug(), orstreamParser.write(), mirror the call site, payload, and ordering so the reporter parses pacquet's NDJSON the same way it parses pnpm's.
Declare a newtype wrapper for branded string types. Do not collapse the brand into a plainStringor&str.
If upstream always validates before construction, validate in pacquet's wrapper too. The wrapper must construct only viaTryFrom<String>and/orFromStr. Do not provide an infallible public constructor.
If upstream never validates, just brand for type-safety. Expose an infallibleFrom<String>(andFrom<&str>when convenient).
If upstream occasionally constructs without validation, exposefrom_str_uncheckedas an escape hatch alongside the validating constructor.
Match upstream serde behavior for branded types that cross JSON, YAML, or INI boundaries. Use#[serde(try_from = "String")]for deserialization and#[serde(into = "String")]for serialization.
Use#[derive(derive_more::From)]and#[derive(derive_more::Into)]for mechanical conversion impls. Fall back to manualimplonly when conversion needs custom logic.
String-literal unions should becomeenums, not newtype wrappers. Model closed sets of valid string values as enums.
Template literal types should be treated as branded strings with validation discipline from rules 2-5.
Choose owned vs. borrowed parameters to minimize copies. Widen to the most encompassing type (&Pathover&PathBuf,&strover&String) when it doesn't force extra copies.
PreferArc::clone(&x)/Rc::clone(&x)overx.clone()for reference-counted types, so the cost is visible at the call site.
Follow Rust API Guidelines for naming conventions.
Do not use star imports inside module bodies. Writeuse super::{Foo, bar}instead ofuse super::*;. Two forms stay allowed: external-crate preludes likeuse rayon::prelude::*;and root-of-module re-...
Files:
pacquet/crates/package-manager/src/lib.rspacquet/crates/package-manager/src/install_package_from_registry/tests.rspacquet/crates/package-manager/src/install/tests.rspacquet/crates/package-manager/src/optimistic_repeat_install.rspacquet/crates/config/src/workspace_yaml.rspacquet/crates/package-manager/src/optimistic_repeat_install/tests.rspacquet/crates/config/src/lib.rspacquet/crates/package-manager/src/install.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/lib.rspacquet/crates/package-manager/src/install_package_from_registry/tests.rspacquet/crates/package-manager/src/install/tests.rspacquet/crates/package-manager/src/optimistic_repeat_install.rspacquet/crates/config/src/workspace_yaml.rspacquet/crates/package-manager/src/optimistic_repeat_install/tests.rspacquet/crates/config/src/lib.rspacquet/crates/package-manager/src/install.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/lib.rspacquet/crates/package-manager/src/install_package_from_registry/tests.rspacquet/crates/package-manager/src/install/tests.rspacquet/crates/package-manager/src/optimistic_repeat_install.rspacquet/crates/config/src/workspace_yaml.rspacquet/crates/package-manager/src/optimistic_repeat_install/tests.rspacquet/crates/config/src/lib.rspacquet/crates/package-manager/src/install.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/lib.rspacquet/crates/package-manager/src/install_package_from_registry/tests.rspacquet/crates/package-manager/src/install/tests.rspacquet/crates/package-manager/src/optimistic_repeat_install.rspacquet/crates/config/src/workspace_yaml.rspacquet/crates/package-manager/src/optimistic_repeat_install/tests.rspacquet/crates/config/src/lib.rspacquet/crates/package-manager/src/install.rs
🧬 Code graph analysis (6)
pacquet/crates/package-manager/src/lib.rs (1)
pacquet/crates/package-manager/src/optimistic_repeat_install.rs (6)
modules_dirs_present(228-247)manifests_unchanged_since(277-297)Decision(55-66)check_optimistic_repeat_install(84-137)settings_match(144-152)project_structure_matches(210-226)
pacquet/crates/package-manager/src/install/tests.rs (1)
pacquet/crates/config/src/lib.rs (2)
new(941-943)NodeLinker(43-56)
pacquet/crates/package-manager/src/optimistic_repeat_install.rs (4)
pacquet/crates/config/src/lib.rs (1)
NodeLinker(43-56)pacquet/crates/package-manager/src/install.rs (3)
Decision(7-7)check_optimistic_repeat_install(6-6)check_optimistic_repeat_install(391-397)pacquet/crates/package-manager/src/optimistic_repeat_install/tests.rs (1)
check_optimistic_repeat_install(132-138)pacquet/crates/package-manager/src/lib.rs (1)
optimistic_repeat_install(59-59)
pacquet/crates/package-manager/src/optimistic_repeat_install/tests.rs (2)
pacquet/crates/config/src/lib.rs (3)
NodeLinker(43-56)new(941-943)leak(1393-1395)pacquet/crates/package-manager/src/optimistic_repeat_install.rs (2)
check_optimistic_repeat_install(84-137)Decision(55-66)
pacquet/crates/config/src/lib.rs (3)
pacquet/crates/package-manager/src/install/tests.rs (2)
optimistic_repeat_install_skips_entire_pipeline_when_state_is_fresh(4862-5004)frozen_lockfile_disables_optimistic_short_circuit(5013-5140)pacquet/crates/package-manager/src/install_package_from_registry/tests.rs (1)
create_config(22-89)pacquet/crates/config/src/workspace_yaml.rs (1)
apply_to(475-639)
pacquet/crates/package-manager/src/install.rs (1)
pacquet/crates/package-manager/src/optimistic_repeat_install.rs (2)
check_optimistic_repeat_install(84-137)Decision(55-66)
🔇 Additional comments (26)
pacquet/crates/package-manager/src/install.rs (1)
6-7: LGTM!Also applies to: 15-15, 35-35, 360-407, 757-762, 1242-1266, 1389-1395
pacquet/crates/package-manager/src/install/tests.rs (1)
4853-5004: LGTM!Also applies to: 5006-5140
pacquet/crates/config/src/lib.rs (1)
411-427: LGTM!pacquet/crates/config/src/workspace_yaml.rs (2)
135-139: LGTM!
491-491: LGTM!pacquet/crates/package-manager/src/lib.rs (1)
25-25: LGTM!Also applies to: 59-59
pacquet/crates/package-manager/src/install_package_from_registry/tests.rs (1)
40-40: LGTM!pacquet/crates/package-manager/src/optimistic_repeat_install.rs (11)
1-38: LGTM!
39-51: LGTM!
53-66: LGTM!
68-137: LGTM!
139-188: LGTM!
190-204: LGTM!
206-226: LGTM!
228-257: LGTM!
259-269: LGTM!
271-297: LGTM!
299-300: LGTM!pacquet/crates/package-manager/src/optimistic_repeat_install/tests.rs (8)
1-13: LGTM!
60-75: LGTM!
77-123: LGTM!
125-140: LGTM!
142-167: LGTM!
169-216: LGTM!
218-234: LGTM!
236-320: LGTM!
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #11943 +/- ##
==========================================
+ Coverage 87.83% 87.91% +0.07%
==========================================
Files 224 225 +1
Lines 27434 27583 +149
==========================================
+ Hits 24098 24249 +151
+ Misses 3336 3334 -2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Adds the four settings-drift tests from pnpm's `deps/status/test/checkDepsStatus.test.ts` for fields pacquet already tracks: `overrides`, `ignoredOptionalDependencies`, `patchedDependencies`, `allowBuilds`. Each test seeds `.pnpm-workspace-state-v1.json` with a stale value and asserts the short-circuit falls through to the regular install path. Adds one extra test (`returns_skipped_when_unported_pnpm_settings_present`) covering upstream's `packageExtensions` and `peersSuffixMaxLength` drift cases together. Pacquet doesn't yet read either yaml field into `Config`, but the field-by-field `WorkspaceStateSettings::PartialEq` still catches a previous install (or pnpm-written state) that recorded a value. Split into per-field tests once the yaml is read. Documents the four upstream checkDepsStatus cases that can't be ported yet (pnpmfile, patches, lockfile conflicts, ignoredWorkspaceStateSettings) in `pacquet/plans/TEST_PORTING.md`, with the underlying feature each blocks on.
…settings Fixes the dylint (perfectionist) failures CI surfaced on the previous commits and applies the duplicate-helper feedback from the PR review: - 2 instances of unicode `…` in `// ` comments → ASCII `...`. - 3 multi-line macro invocations missing trailing commas. - Remove the test-local `current_settings` helper that duplicated the parent module's `pub(crate) fn current_settings`; tests now call the shared one via `super::current_settings`.
…eatInstall fast path (#11945) * fix(pacquet): require pnpm-lock.yaml for single-project optimisticRepeatInstall fast path The port of pnpm's `optimisticRepeatInstall` short-circuit in #11943 applied the workspace branch's mtime-only exit (`checkDepsStatus.ts:263-271`) to every install, including single-project ones. Pnpm's single-project branch (`checkDepsStatus.ts:387-462`) additionally throws `RUN_CHECK_DEPS_LOCKFILE_NOT_FOUND` when `pnpm-lock.yaml` is absent, which the outer `try` converts into `upToDate: false`. Without that gate, pacquet treated a single-project install with `node_modules` present but no lockfile as "Already up to date" — the pnpm.io `node_modules`-only and `cache+node_modules` benchmark cells finished in ~35 ms instead of running the install (pnpm ~5–7 s on the same fixtures). Add an `is_workspace_install: bool` parameter; in single-project mode, require `<workspace_root>/pnpm-lock.yaml` to exist before declaring the install up to date. Workspace installs continue to skip the lockfile probe — pnpm's workspace branch's only lockfile check (`findConflictedLockfileDir`) silently `continue`s on ENOENT (`checkDepsStatus.ts:593-596`). Tests: - `returns_skipped_when_lockfile_missing_in_single_project_mode` - `returns_up_to_date_in_workspace_mode_without_lockfile` - `optimistic_repeat_install_does_not_short_circuit_when_lockfile_missing` (install-level integration test) - Existing happy-path tests now seed `pnpm-lock.yaml` via a new `write_empty_lockfile` helper in `setup_fresh_install`. * test(pacquet): prove single-project optimisticRepeatInstall round-trips end-to-end Add `optimistic_repeat_install_round_trips_on_single_project_install`: two real `Install::run` calls back-to-back on a non-workspace project (no `pnpm-workspace.yaml`). The first install resolves through the registry mock and writes `pnpm-lock.yaml` + `.pnpm-workspace-state-v1.json` to disk. The second install must hit the optimistic fast path — emit `Already up to date` and skip every install-setup event. Pairs with the negative `..._does_not_short_circuit_when_lockfile_missing` test so the gate's polarity is pinned in both directions. * style(pacquet): cargo fmt
Closes #11940.
Summary
Ports upstream pnpm's
optimisticRepeatInstall+checkDepsStatusdispatch (installing/commands/src/installDeps.ts:179-194). When nothing has changed since the previous successful install,Install::runnow logsAlready up to dateand returns before:verify_lockfile_resolutionsfan-out (and the<cache_dir>/lockfile-verified.jsonllookup it performs internally),getContext, project registration,validateModules,That's the missing earlier shortcut from the original investigation. It's what lets pnpm finish the vlt
lockfile+node_modulescells in ~580 ms regardless of~/.cache/pnpmstate — the verifier-cache file the bench wipes is irrelevant because pnpm never reaches the verifier on a repeat install.How it works
The fast path keys off
<workspace_root>/node_modules/.pnpm-workspace-state-v1.json'slastValidatedTimestampagainst each project'spackage.jsonmtime, plus a settings-drift check and a workspace-project structure check. Wire shape and field-by-field comparison match upstream'sWorkspaceStateSettingsso a previous-install state file written by pnpm is honored by pacquet and vice versa.Settings construction is shared between
build_workspace_state(writer) andcheck_optimistic_repeat_install(reader) viaoptimistic_repeat_install::current_settings, so the two can't drift on a new field.Scope
This PR ports the mtime-vs-
lastValidatedTimestampexit only — upstream'smodifiedProjects.length === 0branch atcheckDepsStatus.ts:263-271. Branches that detect a modified project and then re-verify the lockfile (assertWantedLockfileUpToDate,patchesOrHooksAreModified) aren't ported here — when any manifest is newer than the last validation, this function returnsSkippedand the install falls through to the regular path, which still has its own freshness guards (check_lockfile_freshness, the existing no-op short-circuit).Disabled under
--frozen-lockfileso a headless install still fails loudly on missing / stale lockfiles, matching upstream not callingcheckDepsStatusin that mode.Config
New
optimistic_repeat_install: boolfield onpacquet_config::Config, defaulttrue— matchesconfig/reader/src/index.ts:169. Wired throughpnpm-workspace.yamlviaWorkspaceSettings.optimistic_repeat_install: Option<bool>. YamloptimisticRepeatInstall: falseopts out per-project; the value also lives in the global config file's allowlist so users can opt out at the user level.Tests
optimistic_repeat_installmodule (7 unit tests) — happy path; config disabled; missing state file; manifest newer than validation; settings drift; workspace project list changed; siblingnode_modulesmissing for a project with deps.install::testsend-to-end —optimistic_repeat_install_skips_entire_pipeline_when_state_is_freshwalksInstall::runwith a seeded fresh state and asserts theAlready up to datelog fires AND noContext/Stage/LockfileVerificationevents get emitted (the entire install setup is skipped).frozen_lockfile_disables_optimistic_short_circuitis the polarity test — identical setup withfrozen_lockfile: trueproves theAlready up to datelog doesn't fire.Test plan
cargo nextest run -p pacquet-config -p pacquet-package-manager -p pacquet-workspace-state— 556/556 passcargo nextest run -p pacquet-package-manager 'install::tests::' optimistic_repeat_install— 67/67 passcargo nextest run -p pacquet-cli --test install— 11/11 passcargo nextest run -p pacquet-cli --test lockfile_verification— 3/3 passjust lint,just check,just fmt— cleanWritten by an agent (Claude Code, claude-opus-4-7).
Summary by CodeRabbit
New Features
Tests
Documentation