Conversation
After an update bumped a catalog entry in pnpm-workspace.yaml, the workspace state cache stored the pre-update catalog versions, so reverting the entry back to its original version was reported as "Already up to date" instead of reinstalling the previous version. Fold the catalogs written during the install into the catalogs recorded in the workspace state so a later install detects the reverted entry as outdated. Closes #12418
Code Review by Qodo
1.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughFixes a bug where ChangesCatalog Revert Detection Fix
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
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 |
PR Summary by QodoFix stale workspace-state catalogs after catalog updates/reverts WalkthroughsDescription• Merge post-install catalog updates into workspace-state cache to avoid false “up to date”. • Thread updatedCatalogs through recursive installs so cache matches pnpm-workspace.yaml. • Add unit + e2e regression coverage for catalog revert detection (#12418). Diagramgraph TD
A["installDeps / recursive"] --> B["install/mutateModules"] --> C["updatedCatalogs"]
C --> D["updateWorkspaceManifest"] --> E[("pnpm-workspace.yaml")]
C --> F["mergeCatalogs"] --> G["updateWorkspaceState"] --> H[("workspace state cache")]
High-Level AssessmentThe following are alternative approaches to this PR: 1. Re-read pnpm-workspace.yaml before saving workspace state
2. Mutate the in-memory config (opts.catalogs) at the moment catalogs are updated
Recommendation: Current approach (mergeCatalogs + wrapping settings at workspace-state save sites, and returning updatedCatalogs from recursive()) is a good balance: it avoids extra I/O, keeps changes localized to persistence boundaries, and explicitly ties the cache content to what was written during the install. The merge helper is small and well-tested, and the e2e regression directly covers the reported failure mode. File ChangesBug fix (4)
Tests (2)
Other (4)
|
Address review feedback on the catalog-merge helper: - mergeCatalogs now builds null-prototype records and copies entries with Object.defineProperty, so a catalog or dependency name like __proto__ (which can flow in from parsed pnpm-workspace.yaml) becomes an ordinary own property instead of corrupting the result's prototype. - The recursive per-project install path now accumulates updatedCatalogs with mergeCatalogs instead of a shallow Object.assign, so two projects updating different entries of the same catalog no longer clobber each other.
|
Code review by qodo was updated up to the latest commit 025f803 |
Integrated-Benchmark Report (Linux)Each scenario reports direct installs and pnpr installs. Bencher consumes pacquet@HEAD and pnpr@HEAD. Scenario: Isolated linker: fresh restore, cold cache + cold store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 4.227448212680001,
"stddev": 0.16244438894929075,
"median": 4.260115538380001,
"user": 3.9195439400000005,
"system": 3.50645942,
"min": 4.02162238538,
"max": 4.4919547713800005,
"times": [
4.4919547713800005,
4.1741322833800005,
4.38711042238,
4.03365940038,
4.2506287483800005,
4.02162238538,
4.32760126238,
4.02199954938,
4.29617097538,
4.26960232838
]
},
{
"command": "pacquet@main",
"mean": 4.132037335980001,
"stddev": 0.12312495391744227,
"median": 4.09574185888,
"user": 3.8883461399999995,
"system": 3.46704692,
"min": 4.0181571403800005,
"max": 4.44005065438,
"times": [
4.44005065438,
4.04260198338,
4.0830550073800005,
4.18051589238,
4.09526700238,
4.0181571403800005,
4.11712215038,
4.2038553823800004,
4.04353143138,
4.09621671538
]
},
{
"command": "pnpr@HEAD",
"mean": 2.1490929237800005,
"stddev": 0.12072989929518013,
"median": 2.13664417388,
"user": 2.66644904,
"system": 2.99819052,
"min": 1.9536655103799998,
"max": 2.3498929303800002,
"times": [
2.14005922138,
2.27011092538,
2.20547442838,
2.3498929303800002,
2.09868893338,
1.9536655103799998,
2.06269557838,
2.24874761938,
2.1332291263800003,
2.02836496438
]
},
{
"command": "pnpr@main",
"mean": 2.16043376198,
"stddev": 0.10993928838287559,
"median": 2.1158063723800002,
"user": 2.6783078399999996,
"system": 2.9904567199999996,
"min": 2.03136996338,
"max": 2.35332162738,
"times": [
2.1183222173800003,
2.12574826538,
2.1132905273800002,
2.23753743538,
2.33637298938,
2.03136996338,
2.08322105838,
2.35332162738,
2.09359267938,
2.11156085638
]
}
]
}Scenario: Isolated linker: fresh restore, hot cache + hot store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 0.6256250395399999,
"stddev": 0.011536321444978405,
"median": 0.62821929404,
"user": 0.37882854,
"system": 1.32377748,
"min": 0.60414664754,
"max": 0.63979705354,
"times": [
0.60414664754,
0.63979705354,
0.61234059454,
0.6290872175400001,
0.6238589035400001,
0.62735137054,
0.6393435725400001,
0.63171505254,
0.61681236054,
0.63179762254
]
},
{
"command": "pacquet@main",
"mean": 0.62932822724,
"stddev": 0.03002030656772011,
"median": 0.62653922354,
"user": 0.37796044,
"system": 1.32042248,
"min": 0.5958595815400001,
"max": 0.7056113635400001,
"times": [
0.6278271025400001,
0.61272489854,
0.60786608354,
0.5958595815400001,
0.61484026554,
0.62699661254,
0.63199904854,
0.62608183454,
0.7056113635400001,
0.64347548154
]
},
{
"command": "pnpr@HEAD",
"mean": 0.6896278659399999,
"stddev": 0.0537641896553501,
"median": 0.67089430454,
"user": 0.38580264000000003,
"system": 1.35193178,
"min": 0.64780920554,
"max": 0.82868334154,
"times": [
0.64780920554,
0.66328979954,
0.6784988095400001,
0.7249881275400001,
0.69350798554,
0.68153131454,
0.65569864854,
0.82868334154,
0.6629745515400001,
0.65929687554
]
},
{
"command": "pnpr@main",
"mean": 0.68683698284,
"stddev": 0.033558047374578855,
"median": 0.68446964104,
"user": 0.38911224,
"system": 1.3374874799999996,
"min": 0.6488802745400001,
"max": 0.7354817945400001,
"times": [
0.7354817945400001,
0.68890772454,
0.65277730454,
0.68380934954,
0.68512993254,
0.6496075665400001,
0.73129277954,
0.7246368335400001,
0.6488802745400001,
0.6678462685400001
]
}
]
}Scenario: Isolated linker: fresh install, cold cache + cold store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 4.218846120519999,
"stddev": 0.05704778277829939,
"median": 4.22412946122,
"user": 3.760807860000001,
"system": 3.33264644,
"min": 4.12150286322,
"max": 4.3208487802199995,
"times": [
4.21858115622,
4.184970594219999,
4.230427618219999,
4.12150286322,
4.18798355722,
4.22967776622,
4.261969167219999,
4.16533256522,
4.3208487802199995,
4.2671671372199995
]
},
{
"command": "pacquet@main",
"mean": 4.2397024300199995,
"stddev": 0.0496489813985601,
"median": 4.22098907922,
"user": 3.7724350600000003,
"system": 3.34622644,
"min": 4.18255282922,
"max": 4.326103368219999,
"times": [
4.19869337922,
4.19218445522,
4.21348966422,
4.22586358222,
4.285168242219999,
4.30093202222,
4.21611457622,
4.25592218122,
4.18255282922,
4.326103368219999
]
},
{
"command": "pnpr@HEAD",
"mean": 2.16543381072,
"stddev": 0.10207837940886759,
"median": 2.1597443957199998,
"user": 2.52474346,
"system": 2.86920744,
"min": 2.03002311422,
"max": 2.3581064022200002,
"times": [
2.14641850822,
2.19212567822,
2.09728364022,
2.05122888122,
2.18320843922,
2.12496935022,
2.3581064022200002,
2.2979038102200002,
2.17307028322,
2.03002311422
]
},
{
"command": "pnpr@main",
"mean": 2.2285060530200003,
"stddev": 0.11132130907184831,
"median": 2.22578812472,
"user": 2.49095036,
"system": 2.8883723399999996,
"min": 2.06798836222,
"max": 2.3949883182200002,
"times": [
2.30737912722,
2.29308039622,
2.33245658422,
2.06798836222,
2.31335264722,
2.3949883182200002,
2.1584958532200003,
2.14581956622,
2.11693160522,
2.1545680702200003
]
}
]
}Scenario: Isolated linker: fresh install, hot cache + hot store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 1.29913076204,
"stddev": 0.01321993618101548,
"median": 1.29683775244,
"user": 1.2996912199999997,
"system": 1.67503918,
"min": 1.27372408344,
"max": 1.31869073144,
"times": [
1.29386577344,
1.2913741304400002,
1.30605398544,
1.31869073144,
1.2951193094400002,
1.3001450034400002,
1.29684431344,
1.27372408344,
1.2968311914400001,
1.3186590984400002
]
},
{
"command": "pacquet@main",
"mean": 1.3681346021400003,
"stddev": 0.041320498714305445,
"median": 1.35722125244,
"user": 1.3529331199999999,
"system": 1.69247068,
"min": 1.33168760244,
"max": 1.4766356064400001,
"times": [
1.3564549964400001,
1.4766356064400001,
1.3647804644400001,
1.33168760244,
1.38756709644,
1.35497602144,
1.36507119144,
1.3579875084400002,
1.33307697244,
1.35310856144
]
},
{
"command": "pnpr@HEAD",
"mean": 0.6649979473400001,
"stddev": 0.10053256236147205,
"median": 0.63059595994,
"user": 0.33796861999999994,
"system": 1.2746517799999997,
"min": 0.62479429144,
"max": 0.95042285644,
"times": [
0.62972977944,
0.62817399444,
0.62763120144,
0.62961200444,
0.62479429144,
0.95042285644,
0.63146214044,
0.64011223444,
0.64242213044,
0.64561884044
]
},
{
"command": "pnpr@main",
"mean": 0.64437688034,
"stddev": 0.040337800244081366,
"median": 0.6323946379400001,
"user": 0.32909202000000004,
"system": 1.2624224800000001,
"min": 0.61498279844,
"max": 0.75270071444,
"times": [
0.65752931344,
0.61912180044,
0.63212359844,
0.63266567744,
0.61634842844,
0.61498279844,
0.64403953344,
0.63177634444,
0.6424805944399999,
0.75270071444
]
}
]
}Scenario: Isolated linker: fresh install, cold cache + hot store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 2.9466997998799997,
"stddev": 0.043484735254703887,
"median": 2.9518625479800003,
"user": 1.74392854,
"system": 1.92010024,
"min": 2.8466885699800004,
"max": 3.00912896098,
"times": [
3.00912896098,
2.95609783698,
2.98475936098,
2.94001992798,
2.8466885699800004,
2.91861348598,
2.95277144898,
2.9368460669800003,
2.97111869298,
2.9509536469800004
]
},
{
"command": "pacquet@main",
"mean": 3.0088656627800003,
"stddev": 0.06722817997020267,
"median": 3.00858065248,
"user": 1.7825675399999998,
"system": 1.9405848399999996,
"min": 2.91181581998,
"max": 3.10029420998,
"times": [
2.9767119919800002,
2.9302192319800002,
2.9968200029800003,
2.9516224009800003,
2.91181581998,
3.0203413019800003,
3.08192453898,
3.08835194298,
3.10029420998,
3.03055518598
]
},
{
"command": "pnpr@HEAD",
"mean": 0.6537135485800001,
"stddev": 0.012558531353046045,
"median": 0.65491503098,
"user": 0.32970104,
"system": 1.3202575400000005,
"min": 0.63256899298,
"max": 0.67863416598,
"times": [
0.65860449998,
0.65853253698,
0.65247661098,
0.6369105819800001,
0.65214841498,
0.65742961998,
0.67863416598,
0.65567702798,
0.65415303398,
0.63256899298
]
},
{
"command": "pnpr@main",
"mean": 0.62956624088,
"stddev": 0.009910694914385577,
"median": 0.6320611354800001,
"user": 0.31853344000000006,
"system": 1.2666482399999999,
"min": 0.61729625598,
"max": 0.6476006439800001,
"times": [
0.6347262179800001,
0.63630948798,
0.6235714409800001,
0.6349675809800001,
0.62952342098,
0.61959665298,
0.63459884998,
0.6174718569800001,
0.61729625598,
0.6476006439800001
]
}
]
} |
|
| Branch | pr/12438 |
| 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 | 4,218.85 ms(+1.97%)Baseline: 4,137.31 ms | 4,964.77 ms (84.98%) |
| isolated-linker.fresh-install.cold-cache.hot-store | 📈 view plot 🚷 view threshold | 2,946.70 ms(-0.62%)Baseline: 2,965.09 ms | 3,558.11 ms (82.82%) |
| isolated-linker.fresh-install.hot-cache.hot-store | 📈 view plot 🚷 view threshold | 1,299.13 ms(+0.76%)Baseline: 1,289.38 ms | 1,547.25 ms (83.96%) |
| isolated-linker.fresh-restore.cold-cache.cold-store | 📈 view plot 🚷 view threshold | 4,227.45 ms(+8.08%)Baseline: 3,911.56 ms | 4,693.87 ms (90.06%) |
| isolated-linker.fresh-restore.hot-cache.hot-store | 📈 view plot 🚷 view threshold | 625.63 ms(+1.53%)Baseline: 616.17 ms | 739.40 ms (84.61%) |
|
| Branch | pr/12438 |
| Testbed | pnpr |
⚠️ WARNING: No Threshold found!Without a Threshold, no Alerts will ever be generated.
Click here to create a new Threshold
For more information, see the Threshold documentation.
To only post results if a Threshold exists, set the--ci-only-thresholdsflag.
Click to view all benchmark results
| Benchmark | Latency | milliseconds (ms) |
|---|---|---|
| isolated-linker.fresh-install.cold-cache.cold-store | 📈 view plot | 2,165.43 ms |
| isolated-linker.fresh-install.cold-cache.hot-store | 📈 view plot | 653.71 ms |
| isolated-linker.fresh-install.hot-cache.hot-store | 📈 view plot | 665.00 ms |
| isolated-linker.fresh-restore.cold-cache.cold-store | 📈 view plot | 2,149.09 ms |
| isolated-linker.fresh-restore.hot-cache.hot-store | 📈 view plot | 689.63 ms |
Problem
Closes #12418.
When
pnpm update(oradd) bumps a catalog version inpnpm-workspace.yaml, the install correctly writes the new version to the manifest, lockfile, andnode_modules— but it saved the workspace-state cache (node_modules/.pnpm-workspace-state-v1.json) using the pre-update in-memory config, which was read at process start, before the manifest was rewritten.So after an update the cache held the old catalog version. Reverting the catalog entry back to that old version and running
pnpm installthen compared the freshly-read config (old) against the cache (also old) → they matched → "Already up to date", even thoughnode_modulesstill had the updated version.Reproduction
Fix
The post-install catalogs (
updatedCatalogs, already computed and written to the manifest) are now folded into the catalogs recorded in the workspace state, so the cache reflects what is actually on disk.@pnpm/catalogs.config— newmergeCatalogs()helper (deep two-level merge, later entries win).@pnpm/installing.commands— wrap the settings passed toupdateWorkspaceStatewith the merged catalogs at every save site (single-project add, install, and the recursive path including thelinkWorkspacePackagesbranch);recursive()now returns the catalogs it wrote so the workspace-recursive path can persist them.pacquet parity
No pacquet change needed — pacquet already handles this correctly. Its
updatethreads the post-update catalogs into the install viacatalogs_override, andbuild_workspace_statederives the stored catalogs from that same value, so its cache already records the new versions. This fix brings the TypeScript CLI to parity with pacquet's behavior.Tests
pnpm/test/catalogUpToDate.ts(matches the manually reproduced broken behavior).mergeCatalogs.saveCatalog,addRecursive/importRecursive/miscRecursive,update/remove/importcommand suites,deps/status, andverifyDepsBeforeRune2e all pass.Written by an agent (Claude Code, claude-opus-4-8).
Summary by CodeRabbit
pnpm installincorrectly reporting “Already up to date” after apnpm-workspace.yamlcatalog entry is reverted to an earlier version. Workspace state is now correctly detected as changed, ensuring the dependency version is reconciled.