Conversation
…: deps Under `enableGlobalVirtualStore: true`, pacquet built the slot path for an injected `file:` workspace dep as `<store>/links/@/<name>/file:<path>/<hash>`, embedding the raw depPath version. The `:` (and embedded `/`) make the path invalid on Windows (`ERROR_INVALID_NAME`) and diverge from pnpm. `gvs_version_segment` now mirrors pnpm's `nameVerFromPkgSnapshot` feeding `formatGlobalVirtualStorePath`: a `file:` directory dep has no lockfile `version` and a non-semver depPath, so upstream renders the literal segment `undefined`. Pacquet does the same, keeping the colon out of the path while matching pnpm's frozen-lockfile output byte-for-byte. The materialise step itself already works (the directory fetcher landed with the injectWorkspacePackages/dedupeInjectedDeps ports), so re-enable the Windows-skipped e2e test and strengthen it to assert materialisation. Add a GVS regression test covering the colon fix. Closes #12038
|
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). (7)
🧰 Additional context used📓 Path-based instructions (2)pacquet/**/*.rs📄 CodeRabbit inference engine (pacquet/AGENTS.md)
Files:
pacquet/**/{tests,test}/*.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)
📝 WalkthroughWalkthroughThis PR fixes Global Virtual Store (GVS) slot path computation for ChangesGVS version segment computation for file: dependencies
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
Review Summary by QodoFix GVS path segment for injected file: workspace dependencies
WalkthroughsDescription• Fix GVS path segment for injected file: workspace deps to match pnpm - Render undefined instead of raw file: to avoid Windows ERROR_INVALID_NAME • Re-enable Windows test for injected workspace deps with dedupe disabled - Strengthen test to assert materialisation into virtual store • Add GVS regression test for injected file: deps materialisation • Add unit test for gvs_version_segment function Diagramflowchart LR
A["Injected file: dep<br/>file:packages/b"] -->|gvs_version_segment| B["Version segment<br/>undefined"]
B -->|formatGlobalVirtualStorePath| C["GVS slot path<br/>@/b/undefined/hash"]
C -->|No colon| D["Windows compatible<br/>Matches pnpm"]
File Changes1. pacquet/crates/cli/tests/dedupe_injected_deps.rs
|
Micro-Benchmark ResultsLinux |
|
Actionable comments posted: 0 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #12039 +/- ##
==========================================
+ Coverage 88.67% 88.69% +0.01%
==========================================
Files 233 233
Lines 30051 30062 +11
==========================================
+ Hits 26648 26663 +15
+ Misses 3403 3399 -4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Integrated-Benchmark Report (Linux)Scenario: Isolated linker: fresh restore, cold cache + cold store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 2.11964580446,
"stddev": 0.08957524439868893,
"median": 2.10061290356,
"user": 2.64566326,
"system": 3.3912613799999995,
"min": 2.00059956806,
"max": 2.32339750606,
"times": [
2.07056166806,
2.00059956806,
2.04534565506,
2.15343683806,
2.09618918406,
2.10503662306,
2.14517098506,
2.32339750606,
2.07410087106,
2.18261914606
]
},
{
"command": "pacquet@main",
"mean": 2.0358943150599997,
"stddev": 0.03465961460969254,
"median": 2.02651803956,
"user": 2.64054746,
"system": 3.3623169799999992,
"min": 1.9863280570600002,
"max": 2.09827034006,
"times": [
2.09827034006,
1.9863280570600002,
2.01995194606,
2.03308413306,
2.05033613306,
2.08335537506,
2.01010026406,
2.01069631206,
2.01877762906,
2.04804296106
]
}
]
}Scenario: Isolated linker: fresh restore, hot cache + hot store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 0.65633259718,
"stddev": 0.028341352357959648,
"median": 0.64958071118,
"user": 0.35402624,
"system": 1.3176751799999997,
"min": 0.62929931918,
"max": 0.7256536421800001,
"times": [
0.7256536421800001,
0.62929931918,
0.66288768618,
0.6379874191800001,
0.6545163781800001,
0.6784864461800001,
0.6523909841800001,
0.6348842521800001,
0.6404494061800001,
0.6467704381800001
]
},
{
"command": "pacquet@main",
"mean": 0.64560775788,
"stddev": 0.011937389585277234,
"median": 0.64296404768,
"user": 0.35759574,
"system": 1.3327233799999998,
"min": 0.6312867121800001,
"max": 0.6757071131800001,
"times": [
0.6757071131800001,
0.6529233591800001,
0.6424448061800001,
0.6399437961800001,
0.6411561451800001,
0.6312867121800001,
0.6449407181800001,
0.6384624631800001,
0.6434832891800001,
0.64572917618
]
}
]
}Scenario: Isolated linker: fresh install, cold cache + cold store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 2.3417380375600003,
"stddev": 0.02939970810977194,
"median": 2.34924199096,
"user": 3.7739335000000005,
"system": 3.17679956,
"min": 2.29732427446,
"max": 2.39641507446,
"times": [
2.34902552446,
2.35401996646,
2.34344382746,
2.29732427446,
2.30768511946,
2.35985039546,
2.34996511646,
2.34945845746,
2.31019261946,
2.39641507446
]
},
{
"command": "pacquet@main",
"mean": 2.3583009533599997,
"stddev": 0.04190521710035663,
"median": 2.34346224846,
"user": 3.8048925999999996,
"system": 3.16510376,
"min": 2.30341094146,
"max": 2.44086020646,
"times": [
2.44086020646,
2.30341094146,
2.39308991146,
2.33340720446,
2.33129352746,
2.33228431146,
2.38483394046,
2.38672017046,
2.35351729246,
2.32359202746
]
}
]
}Scenario: Isolated linker: fresh install, hot cache + hot store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 1.5515298031800002,
"stddev": 0.02856380091339649,
"median": 1.54663641548,
"user": 1.7760127,
"system": 1.87099288,
"min": 1.51230408248,
"max": 1.6069151854800001,
"times": [
1.51230408248,
1.54596754048,
1.5636891584800001,
1.52090644948,
1.5718889654800001,
1.53282807248,
1.5765420054800001,
1.5369512814800002,
1.6069151854800001,
1.5473052904800002
]
},
{
"command": "pacquet@main",
"mean": 1.5633565896800001,
"stddev": 0.05157403958270373,
"median": 1.54885185698,
"user": 1.7542197000000002,
"system": 1.8802105800000004,
"min": 1.49739147348,
"max": 1.67480387148,
"times": [
1.49739147348,
1.51707768148,
1.5331999734800001,
1.67480387148,
1.53900812948,
1.5749227214800001,
1.6077556894800002,
1.59170264248,
1.53901047048,
1.55869324348
]
}
]
} |
|
Bencher Report| Branch | pr/12039 |
| 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,341.74 ms(+1.66%)Baseline: 2,303.53 ms | 2,764.24 ms (84.72%) |
| isolated-linker.fresh-install.hot-cache.hot-store | 📈 view plot 🚷 view threshold | 1,551.53 ms(+7.02%)Baseline: 1,449.81 ms | 1,739.77 ms (89.18%) |
| isolated-linker.fresh-restore.cold-cache.cold-store | 📈 view plot 🚷 view threshold | 2,119.65 ms(+2.68%)Baseline: 2,064.32 ms | 2,477.18 ms (85.57%) |
| isolated-linker.fresh-restore.hot-cache.hot-store | 📈 view plot 🚷 view threshold | 656.33 ms(-2.96%)Baseline: 676.34 ms | 811.61 ms (80.87%) |
Problem
Closes #12038.
When materialising an injected
file:workspace dep (dependenciesMeta[<alias>].injected: true,dedupeInjectedDeps: false) under the global virtual store (enableGlobalVirtualStore: true), pacquet built the slot path as:The raw
file:packages/bversion segment embeds a:and a/, which:ERROR_INVALID_NAME(os error 123), failing the install; and@/b/undefined/<hash>(frozen-lockfile) /@/b/1.0.0/<hash>(fresh).The materialise step the issue describes is already implemented — the
LockfileResolution::Directoryarm runs the directory fetcher (landed with theinjectWorkspacePackages/dedupeInjectedDepsports), so on non-GVS installs the dep already copies into the escapedb@file+packages+bslot and resolves correctly. The remaining gap was the GVS path segment.Fix
gvs_version_segment()mirrors pnpm'snameVerFromPkgSnapshot(pkgSnapshot.version ?? pkgInfo.version) feedingformatGlobalVirtualStorePath. An injectedfile:dep has no lockfileversionand a non-semver depPath, so upstream produces the literal segmentundefined; pacquet now does the same. Semver / non-semver deps are unchanged.Verified pacquet now writes
…/links/@/b/undefined/<hash>/…— colon-free and matching pnpm's--frozen-lockfileoutput.Tests
injected_workspace_dep_with_dedupe_off_writes_file_armon Windows (removed the#[cfg_attr(target_os = "windows", ignore)]) and strengthened it to assert the dep actually materialises (resolves to a real dir withpackage.json), not just lockfile contents.injected_workspace_dep_with_dedupe_off_materialises_under_gvs— a GVS regression test asserting the slot resolves and the symlink target carries nofile:/colon segment. Confirmed it fails when the fix is reverted.gvs_version_segmentunit test.just fmt+just lintclean; the affected cli tests and all 365pacquet-package-managerunit tests pass.Written by an agent (Claude Code, claude-opus-4-8).
Summary by CodeRabbit
Bug Fixes
Tests