feat(pacquet): port workspace: protocol resolution and publish-time rewrite#11789
Conversation
|
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 (1)
📜 Recent review details🧰 Additional context used📓 Path-based instructions (1)pacquet/**/*.rs📄 CodeRabbit inference engine (pacquet/AGENTS.md)
Files:
🧠 Learnings (2)📚 Learning: 2026-05-20T19:40:55.051ZApplied to files:
📚 Learning: 2026-05-20T23:07:58.444ZApplied to files:
🔇 Additional comments (2)
📝 WalkthroughWalkthroughAdds workspace-aware resolution: parser, range resolver, npm-spec translation, workspace package discovery/threading in installer, npm-resolver routing, and exportable-manifest rewriting with tests. ChangesWorkspace-aware package resolution support
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related issues
Possibly related PRs
Suggested labels
🚥 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 |
a11fdc6 to
3fbaa2f
Compare
Micro-Benchmark ResultsLinux |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #11789 +/- ##
==========================================
+ Coverage 87.28% 87.40% +0.12%
==========================================
Files 193 198 +5
Lines 22636 23081 +445
==========================================
+ Hits 19758 20175 +417
- Misses 2878 2906 +28 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Integrated-Benchmark Report (Linux)Scenario: Frozen Lockfile
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 2.51886002834,
"stddev": 0.1556238036241517,
"median": 2.49044698404,
"user": 2.7406007399999996,
"system": 3.7608488600000003,
"min": 2.36683714054,
"max": 2.88482543754,
"times": [
2.55560384654,
2.62554774354,
2.50029451154,
2.48059945654,
2.42082650054,
2.88482543754,
2.57426850154,
2.36683714054,
2.37500624254,
2.40479090254
]
},
{
"command": "pacquet@main",
"mean": 2.39289520274,
"stddev": 0.11214822566849063,
"median": 2.3599300635400002,
"user": 2.7348570399999996,
"system": 3.7094885599999996,
"min": 2.29473715654,
"max": 2.66014637554,
"times": [
2.66014637554,
2.29473715654,
2.39895794054,
2.34831070454,
2.31575574554,
2.48295022754,
2.43577366154,
2.30196070454,
2.3715494225400002,
2.31881008854
]
},
{
"command": "pnpm",
"mean": 4.87888943544,
"stddev": 0.06126696847072278,
"median": 4.852037747039999,
"user": 8.28613694,
"system": 4.24362246,
"min": 4.804222760539999,
"max": 5.01297025254,
"times": [
5.01297025254,
4.89171849454,
4.85170232254,
4.8402685955399996,
4.84242907154,
4.84963965454,
4.852373171539999,
4.804222760539999,
4.9469506685399995,
4.89661936254
]
}
]
}Scenario: Frozen Lockfile (Hot Cache)
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 0.6904891942200001,
"stddev": 0.054420168738003545,
"median": 0.67062545372,
"user": 0.37535214,
"system": 1.5518191799999999,
"min": 0.66243640922,
"max": 0.83920885522,
"times": [
0.71508589722,
0.66243640922,
0.66694043322,
0.66494102522,
0.83920885522,
0.66879996522,
0.66470506722,
0.67245094222,
0.67332353222,
0.67699981522
]
},
{
"command": "pacquet@main",
"mean": 0.6663354495200001,
"stddev": 0.010781570229783302,
"median": 0.66257218422,
"user": 0.37177444,
"system": 1.5639573800000002,
"min": 0.65603410522,
"max": 0.69423814122,
"times": [
0.69423814122,
0.66241512622,
0.6658063012200001,
0.6599710332200001,
0.67233531022,
0.65603410522,
0.66748124722,
0.66024754022,
0.66272924222,
0.66209644822
]
},
{
"command": "pnpm",
"mean": 2.6018090934199996,
"stddev": 0.109390032669039,
"median": 2.5667775812199998,
"user": 3.236820839999999,
"system": 2.24324098,
"min": 2.5074585732199997,
"max": 2.84184187322,
"times": [
2.6485536982199998,
2.57185048722,
2.56696370222,
2.84184187322,
2.54495669822,
2.5074585732199997,
2.52335294222,
2.51071545122,
2.73580604822,
2.5665914602199997
]
}
]
} |
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? |
…ewrite Ports the `workspace:` family of bare specifiers end-to-end: - `pacquet-workspace-spec` (new) ports `workspace/spec-parser`'s `WorkspaceSpec` parser + `toString`. - `pacquet-workspace-range-resolver` (new) ports `workspace/range-resolver`'s `resolveWorkspaceRange` — `*`/`^`/`~`/`""` pick the highest version with prereleases included; other inputs follow standard semver range rules. - `pacquet-resolving-npm-resolver` grows two helpers from the upstream npm-resolver: `workspace_pref_to_npm` (port of `workspacePrefToNpm.ts`) and `try_resolve_from_workspace` (port of `tryResolveFromWorkspace` + `tryResolveFromWorkspacePackages` + `pickMatchingLocalVersionOrNull` + `resolveFromLocalPackage`). `NpmResolver::resolve_impl` now intercepts `workspace:` specs before the npm pick, deferring `workspace:./` / `workspace:../` to the local resolver. Emits `link:` / `file:` (injected) lockfile resolutions, with the matching `WORKSPACE_PKG_NOT_FOUND` / `NO_MATCHING_VERSION_INSIDE_WORKSPACE` / `CANNOT_RESOLVE_WORKSPACE_PROTOCOL` error codes preserved. - `pacquet-exportable-manifest` (new) ports the publish-time `replaceWorkspaceProtocol` and `replaceWorkspaceProtocolPeerDependency` helpers from `releasing/exportable-manifest`. The full `createExportableManifest` (catalog rewrite, jsr rewrite, pre-pack hooks, publishConfig overrides) lands as pacquet ports the surrounding commands. - `Install::run` builds a workspace-packages map via `find_workspace_projects` when a `pnpm-workspace.yaml` is present and threads it through `ResolveOptions::workspace_packages` so the resolver chain can satisfy `workspace:` specs from local projects in the no-lockfile install path. Test ports: - `workspace/spec-parser/test/workspace-spec.test.ts` - `workspace/range-resolver/test/index.test.ts` - `resolving/npm-resolver/test/workspacePrefToNpm.test.ts` - `releasing/exportable-manifest/test/index.test.ts` (workspace cases) - plus new unit tests for `try_resolve_from_workspace` covering `WORKSPACE_PKG_NOT_FOUND`, `NO_MATCHING_VERSION_INSIDE_WORKSPACE`, the inject branch, and the `publishConfig.directory` / `linkDirectory` handling. Frozen-lockfile installs already record `link:` entries directly; the new resolution path matters for the no-lockfile install path. --- Written by an agent (Claude Code, claude-opus-4-7).
- Rename single-letter params (perfectionist::single-letter-*). - Add trailing commas in multi-line macro invocations. - Avoid the ambiguous `crate::parse_bare_specifier` doc link and the cross-crate `pacquet_workspace_spec::WorkspaceSpec` / `pacquet_workspace_range_resolver::resolve_workspace_range` doc links (the crates don't have a Cargo dependency on each other, so rustdoc can't resolve them). --- Written by an agent (Claude Code, claude-opus-4-7).
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pacquet/crates/exportable-manifest/src/replace.rs`:
- Around line 136-142: The current peer rewrite uses
dep_spec.replace("workspace:", "") which removes all occurrences and breaks pnpm
parity for multi-segment specs; change both occurrences (the early return and
the version-empty branch) to use a single-occurrence replacement instead of
String::replace — e.g., use the str::replacen variant with count=1 when
transforming dep_spec — keeping the rest of the logic around
find_workspace_peer_segment and matched intact.
In `@pacquet/crates/package-manager/src/install.rs`:
- Around line 766-769: The current match on
pacquet_workspace::read_workspace_manifest(workspace_root) collapses Err(_) into
Ok(None) which hides parse/read errors; update the handling so that only
Ok(None) maps to Ok(None) (missing workspace file) and any Err is propagated as
an error (do not convert to Ok). Locate the call to
pacquet_workspace::read_workspace_manifest in install.rs and either use the ?
operator or explicitly return the Err (preserving the original error) instead of
returning Ok(None); ensure the surrounding function signature supports returning
the propagated error so that malformed/unreadable pnpm-workspace.yaml errors
surface instead of downstream resolver failures when resolving
workspace_packages.
In `@pacquet/crates/resolving-npm-resolver/src/npm_resolver.rs`:
- Around line 126-133: Update the module-level documentation to reflect that
non-path workspace: specifiers are now handled here instead of always returning
Ok(None); change the “Out of scope” bullet to state that only certain workspace:
forms (e.g., path-relative like workspace:./foo or workspace:../bar) are
delegated to the local resolver while non-path workspace: specifiers are routed
through try_resolve_from_workspace/handled by the npm resolver (see the if let
Some(bare) = wanted_dependency.bare_specifier.as_deref() branch), and ensure doc
comments (//! or ///) describe this contract change consistently with resolveNpm
behavior.
🪄 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: e9f86eaf-85a0-43fc-9645-6855fd296638
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (21)
Cargo.tomlpacquet/crates/exportable-manifest/Cargo.tomlpacquet/crates/exportable-manifest/src/lib.rspacquet/crates/exportable-manifest/src/replace.rspacquet/crates/exportable-manifest/src/tests.rspacquet/crates/package-manager/src/install.rspacquet/crates/package-manager/src/install_without_lockfile.rspacquet/crates/resolving-npm-resolver/Cargo.tomlpacquet/crates/resolving-npm-resolver/src/lib.rspacquet/crates/resolving-npm-resolver/src/npm_resolver.rspacquet/crates/resolving-npm-resolver/src/npm_resolver/tests.rspacquet/crates/resolving-npm-resolver/src/resolve_from_workspace.rspacquet/crates/resolving-npm-resolver/src/resolve_from_workspace/tests.rspacquet/crates/resolving-npm-resolver/src/workspace_pref_to_npm.rspacquet/crates/resolving-npm-resolver/src/workspace_pref_to_npm/tests.rspacquet/crates/workspace-range-resolver/Cargo.tomlpacquet/crates/workspace-range-resolver/src/lib.rspacquet/crates/workspace-range-resolver/tests/resolve.rspacquet/crates/workspace-spec/Cargo.tomlpacquet/crates/workspace-spec/src/lib.rspacquet/crates/workspace-spec/src/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). (3)
- GitHub Check: windows-latest / Node.js 22.13.0 / Test
- GitHub Check: Lint and Test (windows-latest)
- GitHub Check: Run benchmark on ubuntu-latest
🧰 Additional context used
📓 Path-based instructions (2)
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/exportable-manifest/src/lib.rspacquet/crates/resolving-npm-resolver/src/workspace_pref_to_npm.rspacquet/crates/resolving-npm-resolver/src/npm_resolver/tests.rspacquet/crates/workspace-range-resolver/tests/resolve.rspacquet/crates/exportable-manifest/src/tests.rspacquet/crates/resolving-npm-resolver/src/resolve_from_workspace/tests.rspacquet/crates/workspace-range-resolver/src/lib.rspacquet/crates/package-manager/src/install.rspacquet/crates/resolving-npm-resolver/src/lib.rspacquet/crates/workspace-spec/src/lib.rspacquet/crates/package-manager/src/install_without_lockfile.rspacquet/crates/exportable-manifest/src/replace.rspacquet/crates/resolving-npm-resolver/src/workspace_pref_to_npm/tests.rspacquet/crates/resolving-npm-resolver/src/resolve_from_workspace.rspacquet/crates/resolving-npm-resolver/src/npm_resolver.rspacquet/crates/workspace-spec/src/tests.rs
pacquet/**/{tests,test}/**/*.rs
📄 CodeRabbit inference engine (pacquet/AGENTS.md)
pacquet/**/{tests,test}/**/*.rs: Prefer real fixtures; reach for the dependency-injection seam only when they can't cover the branch. Most happy paths and error paths should be tested withtempfile::TempDir, the mocked registry, or an integration test that spawns the actual binary.
Use the DI seam — a capability trait on theHostprovider, threaded asSys: <Bounds>— only for branches a real fixture can't reach portably: filesystem error kinds, deterministic time, shared process-global state, or external-service happy paths.
Log before non-assert_eq!assertions,dbg!complex structures, skip logging for simple scalarassert_eq!. Follow the test-logging guidance in CODE_STYLE_GUIDE.md.
Tests must not be tolerant of missing build/runtime environment by silently returning early when a tool isn't found. If the test needs a tool, let it panic when the tool is absent.
Prefer#[cfg_attr(target_os = "windows", ignore = "...")](or#[cfg(unix)]) over a runtime probe-and-skip helper for platform-locked tools.
Port relevant tests from pnpm when porting behavior. Matching test coverage is the easiest way to prove behavioral parity.
When porting behavior from pnpm, consult plans/TEST_PORTING.md before adding ported tests. Useallow_known_failure!at the not-yet-implemented boundary and update checkboxes as items land.
Files:
pacquet/crates/workspace-range-resolver/tests/resolve.rs
🧠 Learnings (2)
📚 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/exportable-manifest/src/lib.rspacquet/crates/resolving-npm-resolver/src/workspace_pref_to_npm.rspacquet/crates/resolving-npm-resolver/src/npm_resolver/tests.rspacquet/crates/workspace-range-resolver/tests/resolve.rspacquet/crates/exportable-manifest/src/tests.rspacquet/crates/resolving-npm-resolver/src/resolve_from_workspace/tests.rspacquet/crates/workspace-range-resolver/src/lib.rspacquet/crates/package-manager/src/install.rspacquet/crates/resolving-npm-resolver/src/lib.rspacquet/crates/workspace-spec/src/lib.rspacquet/crates/package-manager/src/install_without_lockfile.rspacquet/crates/exportable-manifest/src/replace.rspacquet/crates/resolving-npm-resolver/src/workspace_pref_to_npm/tests.rspacquet/crates/resolving-npm-resolver/src/resolve_from_workspace.rspacquet/crates/resolving-npm-resolver/src/npm_resolver.rspacquet/crates/workspace-spec/src/tests.rs
📚 Learning: 2026-05-20T23:07:58.444Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11784
File: pacquet/crates/resolving-deps-resolver/src/hoist_peers.rs:120-133
Timestamp: 2026-05-20T23:07:58.444Z
Learning: When reviewing code in this pacquet Rust port, follow the upstream pnpm compatibility rule: only match pnpm’s behavior exactly. Do not propose review changes that intentionally deviate from pnpm’s documented/observed behavior, even if pnpm appears buggy. If you identify a real bug in pnpm behavior, the review should prioritize fixing it upstream in pnpm first, and avoid implementing a pnpm-behavior workaround here unless the same fix has already landed upstream.
Applied to files:
pacquet/crates/exportable-manifest/src/lib.rspacquet/crates/resolving-npm-resolver/src/workspace_pref_to_npm.rspacquet/crates/resolving-npm-resolver/src/npm_resolver/tests.rspacquet/crates/workspace-range-resolver/tests/resolve.rspacquet/crates/exportable-manifest/src/tests.rspacquet/crates/resolving-npm-resolver/src/resolve_from_workspace/tests.rspacquet/crates/workspace-range-resolver/src/lib.rspacquet/crates/package-manager/src/install.rspacquet/crates/resolving-npm-resolver/src/lib.rspacquet/crates/workspace-spec/src/lib.rspacquet/crates/package-manager/src/install_without_lockfile.rspacquet/crates/exportable-manifest/src/replace.rspacquet/crates/resolving-npm-resolver/src/workspace_pref_to_npm/tests.rspacquet/crates/resolving-npm-resolver/src/resolve_from_workspace.rspacquet/crates/resolving-npm-resolver/src/npm_resolver.rspacquet/crates/workspace-spec/src/tests.rs
🔇 Additional comments (19)
pacquet/crates/workspace-spec/Cargo.toml (1)
1-15: LGTM!pacquet/crates/workspace-spec/src/lib.rs (1)
1-96: LGTM!pacquet/crates/workspace-spec/src/tests.rs (1)
1-87: LGTM!pacquet/crates/workspace-range-resolver/Cargo.toml (1)
1-18: LGTM!pacquet/crates/workspace-range-resolver/src/lib.rs (1)
1-93: LGTM!pacquet/crates/workspace-range-resolver/tests/resolve.rs (1)
1-41: LGTM!pacquet/crates/exportable-manifest/Cargo.toml (1)
1-25: LGTM!pacquet/crates/exportable-manifest/src/lib.rs (1)
1-26: LGTM!pacquet/crates/exportable-manifest/src/tests.rs (1)
1-155: LGTM!pacquet/crates/resolving-npm-resolver/src/workspace_pref_to_npm.rs (1)
1-50: LGTM!pacquet/crates/resolving-npm-resolver/src/workspace_pref_to_npm/tests.rs (1)
1-49: LGTM!pacquet/crates/package-manager/src/install.rs (1)
169-170: LGTM!Also applies to: 515-536
pacquet/crates/package-manager/src/install_without_lockfile.rs (1)
88-104: LGTM!Also applies to: 189-191, 324-342
pacquet/crates/resolving-npm-resolver/Cargo.toml (1)
20-21: LGTM!pacquet/crates/resolving-npm-resolver/src/lib.rs (1)
35-38: LGTM!Also applies to: 69-71, 76-76
pacquet/crates/resolving-npm-resolver/src/resolve_from_workspace.rs (1)
1-347: LGTM!pacquet/crates/resolving-npm-resolver/src/resolve_from_workspace/tests.rs (1)
1-235: LGTM!pacquet/crates/resolving-npm-resolver/src/npm_resolver/tests.rs (1)
130-171: LGTM!Cargo.toml (1)
37-37: LGTM!Also applies to: 57-58
e7eb4d4 to
358de54
Compare
- replace_workspace_protocol_peer_dependency: use replacen("workspace:", "", 1)
so compound peer specs match upstream JS String.replace's first-only
semantics; locked with a new test
peer_workspace_strip_only_removes_first_occurrence.
- npm-resolver module docs: drop the stale "workspace: returns
Ok(None)" bullet and explain that non-path workspace specs now route
through try_resolve_from_workspace while path-relative forms still
fall through to the local resolver.
The third coderabbit nit (read_workspace_manifest error swallowing in
install.rs) was already addressed by the rebase: the workspace manifest
is now read once at the top of Install::run with proper error
propagation, and build_workspace_packages_map takes the pre-loaded
Option<&WorkspaceManifest> instead of re-reading the file.
---
Written by an agent (Claude Code, claude-opus-4-7).
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/resolving-npm-resolver/src/resolve_from_workspace.rs`:
- Around line 96-106: The WorkspacePkgNotFound diagnostic currently declares a
hint field but the Display/diagnostic string doesn't include it, so update the
diagnostic message for the WorkspacePkgNotFound struct to surface the hint
(e.g., append the hint text to the display template or include "{hint}" in the
#[display(...)] attribute) so that the populated hint (set where
WorkspacePkgNotFound is constructed) is shown in error output; locate the
WorkspacePkgNotFound enum variant and modify its display/diagnostic annotation
to include the hint text.
- Around line 145-146: The check that currently returns early for bare strings
starting with "workspace:." is too broad; update the condition in
resolve_from_workspace (the branch that returns Ok(None)) to only match the
exact pnpm patterns "workspace:./" and "workspace:../" (e.g., use
starts_with("workspace:./") || starts_with("workspace:../")) so malformed specs
like "workspace:.foo" continue through workspace parsing/error handling instead
of bypassing it.
🪄 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: dd965249-a0f4-49b3-a3aa-77d64fd30aaf
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (21)
Cargo.tomlpacquet/crates/exportable-manifest/Cargo.tomlpacquet/crates/exportable-manifest/src/lib.rspacquet/crates/exportable-manifest/src/replace.rspacquet/crates/exportable-manifest/src/tests.rspacquet/crates/package-manager/src/install.rspacquet/crates/package-manager/src/install_without_lockfile.rspacquet/crates/resolving-npm-resolver/Cargo.tomlpacquet/crates/resolving-npm-resolver/src/lib.rspacquet/crates/resolving-npm-resolver/src/npm_resolver.rspacquet/crates/resolving-npm-resolver/src/npm_resolver/tests.rspacquet/crates/resolving-npm-resolver/src/resolve_from_workspace.rspacquet/crates/resolving-npm-resolver/src/resolve_from_workspace/tests.rspacquet/crates/resolving-npm-resolver/src/workspace_pref_to_npm.rspacquet/crates/resolving-npm-resolver/src/workspace_pref_to_npm/tests.rspacquet/crates/workspace-range-resolver/Cargo.tomlpacquet/crates/workspace-range-resolver/src/lib.rspacquet/crates/workspace-range-resolver/tests/resolve.rspacquet/crates/workspace-spec/Cargo.tomlpacquet/crates/workspace-spec/src/lib.rspacquet/crates/workspace-spec/src/tests.rs
✅ Files skipped from review due to trivial changes (5)
- pacquet/crates/workspace-spec/Cargo.toml
- pacquet/crates/resolving-npm-resolver/Cargo.toml
- pacquet/crates/exportable-manifest/src/lib.rs
- pacquet/crates/workspace-range-resolver/Cargo.toml
- pacquet/crates/exportable-manifest/Cargo.toml
📜 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). (8)
- GitHub Check: Lint and Test (macos-latest)
- GitHub Check: Code Coverage
- GitHub Check: Run benchmark on ubuntu-latest
- GitHub Check: Lint and Test (ubuntu-latest)
- GitHub Check: Run benchmark on ubuntu-latest
- GitHub Check: Lint and Test (windows-latest)
- GitHub Check: Dylint
- GitHub Check: Compile & Lint
🧰 Additional context used
📓 Path-based instructions (2)
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/workspace-range-resolver/tests/resolve.rspacquet/crates/resolving-npm-resolver/src/resolve_from_workspace/tests.rspacquet/crates/workspace-spec/src/tests.rspacquet/crates/resolving-npm-resolver/src/npm_resolver/tests.rspacquet/crates/resolving-npm-resolver/src/workspace_pref_to_npm.rspacquet/crates/resolving-npm-resolver/src/lib.rspacquet/crates/resolving-npm-resolver/src/npm_resolver.rspacquet/crates/exportable-manifest/src/replace.rspacquet/crates/workspace-spec/src/lib.rspacquet/crates/resolving-npm-resolver/src/workspace_pref_to_npm/tests.rspacquet/crates/package-manager/src/install_without_lockfile.rspacquet/crates/package-manager/src/install.rspacquet/crates/workspace-range-resolver/src/lib.rspacquet/crates/exportable-manifest/src/tests.rspacquet/crates/resolving-npm-resolver/src/resolve_from_workspace.rs
pacquet/**/{tests,test}/**/*.rs
📄 CodeRabbit inference engine (pacquet/AGENTS.md)
pacquet/**/{tests,test}/**/*.rs: Prefer real fixtures; reach for the dependency-injection seam only when they can't cover the branch. Most happy paths and error paths should be tested withtempfile::TempDir, the mocked registry, or an integration test that spawns the actual binary.
Use the DI seam — a capability trait on theHostprovider, threaded asSys: <Bounds>— only for branches a real fixture can't reach portably: filesystem error kinds, deterministic time, shared process-global state, or external-service happy paths.
Log before non-assert_eq!assertions,dbg!complex structures, skip logging for simple scalarassert_eq!. Follow the test-logging guidance in CODE_STYLE_GUIDE.md.
Tests must not be tolerant of missing build/runtime environment by silently returning early when a tool isn't found. If the test needs a tool, let it panic when the tool is absent.
Prefer#[cfg_attr(target_os = "windows", ignore = "...")](or#[cfg(unix)]) over a runtime probe-and-skip helper for platform-locked tools.
Port relevant tests from pnpm when porting behavior. Matching test coverage is the easiest way to prove behavioral parity.
When porting behavior from pnpm, consult plans/TEST_PORTING.md before adding ported tests. Useallow_known_failure!at the not-yet-implemented boundary and update checkboxes as items land.
Files:
pacquet/crates/workspace-range-resolver/tests/resolve.rs
🧠 Learnings (2)
📚 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/workspace-range-resolver/tests/resolve.rspacquet/crates/resolving-npm-resolver/src/resolve_from_workspace/tests.rspacquet/crates/workspace-spec/src/tests.rspacquet/crates/resolving-npm-resolver/src/npm_resolver/tests.rspacquet/crates/resolving-npm-resolver/src/workspace_pref_to_npm.rspacquet/crates/resolving-npm-resolver/src/lib.rspacquet/crates/resolving-npm-resolver/src/npm_resolver.rspacquet/crates/exportable-manifest/src/replace.rspacquet/crates/workspace-spec/src/lib.rspacquet/crates/resolving-npm-resolver/src/workspace_pref_to_npm/tests.rspacquet/crates/package-manager/src/install_without_lockfile.rspacquet/crates/package-manager/src/install.rspacquet/crates/workspace-range-resolver/src/lib.rspacquet/crates/exportable-manifest/src/tests.rspacquet/crates/resolving-npm-resolver/src/resolve_from_workspace.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/workspace-range-resolver/tests/resolve.rspacquet/crates/resolving-npm-resolver/src/resolve_from_workspace/tests.rspacquet/crates/workspace-spec/src/tests.rspacquet/crates/resolving-npm-resolver/src/npm_resolver/tests.rspacquet/crates/resolving-npm-resolver/src/workspace_pref_to_npm.rspacquet/crates/resolving-npm-resolver/src/lib.rspacquet/crates/resolving-npm-resolver/src/npm_resolver.rspacquet/crates/exportable-manifest/src/replace.rspacquet/crates/workspace-spec/src/lib.rspacquet/crates/resolving-npm-resolver/src/workspace_pref_to_npm/tests.rspacquet/crates/package-manager/src/install_without_lockfile.rspacquet/crates/package-manager/src/install.rspacquet/crates/workspace-range-resolver/src/lib.rspacquet/crates/exportable-manifest/src/tests.rspacquet/crates/resolving-npm-resolver/src/resolve_from_workspace.rs
🔇 Additional comments (15)
pacquet/crates/workspace-range-resolver/tests/resolve.rs (1)
1-40: LGTM!pacquet/crates/resolving-npm-resolver/src/resolve_from_workspace/tests.rs (1)
1-235: LGTM!Cargo.toml (1)
16-19: LGTM!Also applies to: 41-41, 61-62
pacquet/crates/workspace-spec/src/tests.rs (1)
1-86: LGTM!pacquet/crates/resolving-npm-resolver/src/npm_resolver/tests.rs (1)
130-171: LGTM!pacquet/crates/resolving-npm-resolver/src/workspace_pref_to_npm.rs (1)
1-50: LGTM!pacquet/crates/resolving-npm-resolver/src/lib.rs (1)
35-35: LGTM!Also applies to: 38-38, 69-71, 76-76
pacquet/crates/resolving-npm-resolver/src/npm_resolver.rs (1)
43-44: LGTM!Also applies to: 126-153
pacquet/crates/exportable-manifest/src/replace.rs (1)
136-142: Use single-occurrenceworkspace:replacement for peer rewrite parity.This still strips all
workspace:occurrences viareplace, which diverges from upstream single-replacement behavior in this branch.Based on learnings: "When reviewing code in this pacquet Rust port, follow the upstream pnpm compatibility rule: only match pnpm’s behavior exactly."
pacquet/crates/workspace-spec/src/lib.rs (1)
22-96: LGTM!pacquet/crates/resolving-npm-resolver/src/workspace_pref_to_npm/tests.rs (1)
6-48: LGTM!pacquet/crates/package-manager/src/install_without_lockfile.rs (1)
89-107: LGTM!Also applies to: 329-350
pacquet/crates/package-manager/src/install.rs (1)
172-185: LGTM!Also applies to: 259-277, 544-567, 772-814
pacquet/crates/workspace-range-resolver/src/lib.rs (1)
23-92: LGTM!pacquet/crates/exportable-manifest/src/tests.rs (1)
24-154: LGTM!
Upstream pnpm's WORKSPACE_PKG_NOT_FOUND error carries a 'hint' field
that PnpmError prints as guidance after the message ('Packages found in
the workspace: ...'). The Rust port was populating the field but the
miette diagnostic didn't reference it, so the help text never reached
the user. Add help("{hint}") to the diagnostic attribute so miette
renders it under the message — matching pnpm's output verbatim.
---
Written by an agent (Claude Code, claude-opus-4-7).
Summary
Ports the
workspace:family of bare specifiers end-to-end into pacquet, mirroring the TypeScript pnpm CLI.workspace:*/workspace:^/workspace:~/workspace:1.2.3/workspace:<alias>@<version>now resolve tolink:(orfile:for injected) lockfile entries, and the publish-time rewrite to npm-style specs is in place for when pacquet'spublish/packlands. Pacquet already loads workspaces (#431); this PR adds the resolution and rewrite layers.What's in scope
New crates
pacquet-workspace-spec— port ofworkspace/spec-parser. Parser +Displayround-trip; full TS test parity.pacquet-workspace-range-resolver— port ofworkspace/range-resolver.*/^/~/""widen to all versions (prereleases included); other inputs follow standard semver. Full TS test parity.pacquet-exportable-manifest— port ofreplaceWorkspaceProtocolandreplaceWorkspaceProtocolPeerDependencyfromreleasing/exportable-manifest. The rest ofcreateExportableManifest(catalog / jsr rewrite, hooks,publishConfigoverrides, manifest serialization) lands as pacquet ports the surrounding commands.pacquet-resolving-npm-resolveradditionsworkspace_pref_to_npm— port ofworkspacePrefToNpm.ts. Translatesworkspace:specs to the npm shape (*,npm:<alias>@<v>).try_resolve_from_workspace— port oftryResolveFromWorkspace+tryResolveFromWorkspacePackages+pickMatchingLocalVersionOrNull+resolveFromLocalPackage. Returnslink:/file:(injected) lockfile resolutions; raisesWORKSPACE_PKG_NOT_FOUND/NO_MATCHING_VERSION_INSIDE_WORKSPACEwith the upstream error codes. HonourspublishConfig.directory+linkDirectory.NpmResolver::resolve_implnow interceptsworkspace:before the npm pick.workspace:./andworkspace:../defer to the local resolver in the chain, unchanged.Install-pipeline wiring
InstallWithoutLockfilenow carrieslockfile_dirandworkspace_packages.Install::runbuilds aWorkspacePackagesmap viafind_workspace_projectswhen apnpm-workspace.yamlis present and threads it intoResolveOptions::workspace_packages. Frozen-lockfile installs already recordlink:entries directly; this matters for the no-lockfile install path.Ported tests
workspace/spec-parser/test/workspace-spec.test.tsworkspace/range-resolver/test/index.test.tsresolving/npm-resolver/test/workspacePrefToNpm.test.tsreleasing/exportable-manifest/test/index.test.ts(workspace-protocol cases)try_resolve_from_workspacecovering the error-code paths, the inject branch, andpublishConfig.directory/linkDirectory.Test plan
cargo fmtcargo check --locked --workspace --all-targetscargo clippy --locked --workspace --all-targets -- --deny warningscargo nextest run --workspace(1758 / 1758 pass)installing/deps-installer/test/install/multipleImporters.tsoncepnpm installend-to-end workspace install is fully exercised in pacquet.Written by an agent (Claude Code, claude-opus-4-7).
Summary by CodeRabbit
New Features
Tests