fix(deps-installer): keep catalog-referencing overrides in sync on update#12158
Conversation
|
💖 Thanks for opening this pull request! 💖 |
|
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 (1)
📝 WalkthroughWalkthroughThis PR fixes a lockfile inconsistency where overrides referencing catalogs were not re-resolved when catalogs were updated, causing subsequent frozen-lockfile installs to fail. The fix re-parses and re-resolves catalog-backed overrides against merged catalogs after catalog updates are applied during install in both pnpm and pacquet. ChangesCatalog Override Synchronization
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Review Summary by QodoFix catalog-referencing overrides sync on pnpm update
WalkthroughsDescription• Re-resolve catalog-referencing overrides after pnpm update bumps catalog • Merge updated catalogs with base catalogs before override resolution • Prevent lockfile inconsistency causing frozen-lockfile install failures • Add comprehensive test for catalog override synchronization on update Diagramflowchart LR
A["pnpm update<br/>bumps catalog"] --> B["updatedCatalogs<br/>detected"]
B --> C["mergeCatalogs<br/>base + updates"]
C --> D["parseOverrides<br/>against merged"]
D --> E["lockfile.overrides<br/>updated"]
E --> F["frozen-lockfile<br/>install succeeds"]
File Changes1. installing/deps-installer/src/install/index.ts
|
There was a problem hiding this comment.
Pull request overview
Fixes a lockfile inconsistency where pnpm update can bump catalog entries but leave lockfile overrides (that resolve via catalog:) stuck on the pre-update catalog resolution, causing subsequent --frozen-lockfile installs to fail with ERR_PNPM_LOCKFILE_CONFIG_MISMATCH.
Changes:
- Re-resolve configured overrides against the post-resolution catalog (base catalogs merged with
updatedCatalogs) and write the updated result intonewLockfile.overrides. - Add an integration test to ensure catalog-referencing overrides are updated alongside catalog updates and that a follow-up frozen install succeeds.
- Add a changeset documenting the behavioral fix.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| installing/deps-installer/src/install/index.ts | Re-resolves overrides against catalogs merged with updatedCatalogs to keep lockfile metadata consistent after pnpm update. |
| installing/deps-installer/test/catalogs.ts | Adds regression coverage for updating catalog-referencing overrides and validating a subsequent frozen install. |
| .changeset/update-catalog-overrides-sync.md | Documents the patch-level fix for pnpm and deps-installer. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
When `pnpm.overrides` reference a catalog (e.g. `overrides: { foo: 'catalog:' }`),
`pnpm update` bumped the catalog entry during resolution but left the resolved
`overrides` in the lockfile pointing at the old version. The lockfile's
`catalogs` advanced while `overrides` stayed stale, producing an internally
inconsistent lockfile that fails a later `pnpm install --frozen-lockfile` with
ERR_PNPM_LOCKFILE_CONFIG_MISMATCH.
After resolution, re-resolve the overrides against the catalog merged with the
update's `updatedCatalogs`, so the lockfile `overrides` track the bumped catalog
just like `catalogs` and direct catalog dependencies do.
…lved
Address review feedback:
- Run the catalog-override re-resolution before the `afterAllResolved`
pnpmfile hook instead of after it, so a hook that edits `lockfile.overrides`
still sees and can amend the final value (the block previously ran after the
hook and would clobber its edits whenever a catalog entry was updated).
- Drop the dead `opts.catalogs ?? {}` fallback; `opts.catalogs` is required on
the install options and always defaulted to `{}`, so it is never nullish here.
ae61a76 to
7dd4658
Compare
Code Review by Qodo🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)
Great, no issues found!Qodo reviewed your code and found no material issues that require reviewPrevious review resultsReview updated until commit 6f6ea2c Results up to commit 7dd4658
Great, no issues found!Qodo reviewed your code and found no material issues that require review |
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.158186189659999,
"stddev": 0.11910367563558069,
"median": 4.160571000759999,
"user": 3.988194579999999,
"system": 3.3774187799999993,
"min": 3.95260782376,
"max": 4.329107330759999,
"times": [
4.329107330759999,
4.07511946976,
3.95260782376,
4.098318475759999,
4.22282352576,
4.253319738759999,
4.25765841576,
4.2473790737599995,
4.077127528759999,
4.0684005137599994
]
},
{
"command": "pacquet@main",
"mean": 4.148887080059999,
"stddev": 0.15217605396686607,
"median": 4.10156965726,
"user": 4.0193648799999995,
"system": 3.4091690800000003,
"min": 3.95632049976,
"max": 4.4965774417599995,
"times": [
4.183269181759999,
3.95632049976,
4.06502797776,
4.11835545376,
4.08478386076,
4.0459603267599995,
4.26310160076,
4.4965774417599995,
4.0561771447599995,
4.219297312759999
]
},
{
"command": "pnpr@HEAD",
"mean": 2.0934192978599997,
"stddev": 0.1547384119218576,
"median": 2.05802686526,
"user": 2.7300830800000004,
"system": 2.8764216799999995,
"min": 1.92150158176,
"max": 2.3534144657600002,
"times": [
2.28155832176,
1.95526489576,
2.10696361376,
2.1579069087600002,
1.92150158176,
1.97054666776,
2.00909011676,
2.22548132676,
1.95246507976,
2.3534144657600002
]
},
{
"command": "pnpr@main",
"mean": 2.10954785676,
"stddev": 0.1300478003078903,
"median": 2.07538745676,
"user": 2.71149658,
"system": 2.8634387799999996,
"min": 1.9707401347600002,
"max": 2.35932325976,
"times": [
2.07871435476,
2.0359874537600002,
1.9707401347600002,
2.12540085176,
2.25142527576,
2.07206055876,
2.00607076676,
1.97252046376,
2.22323544776,
2.35932325976
]
}
]
}Scenario: Isolated linker: fresh restore, hot cache + hot store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 0.6216508630600002,
"stddev": 0.012226335373445142,
"median": 0.61984253696,
"user": 0.35719189999999995,
"system": 1.3140189,
"min": 0.60465235396,
"max": 0.63848467996,
"times": [
0.6296594479600001,
0.60920537096,
0.63564419796,
0.60465235396,
0.6077644239600001,
0.63363606996,
0.6201222469600001,
0.6195628269600001,
0.61777701196,
0.63848467996
]
},
{
"command": "pacquet@main",
"mean": 0.6283614662600001,
"stddev": 0.024244331846595336,
"median": 0.6236381119600001,
"user": 0.3625425,
"system": 1.3043852999999999,
"min": 0.6003528649600001,
"max": 0.67097248696,
"times": [
0.63487103696,
0.6223165379600001,
0.6003528649600001,
0.6635630839600001,
0.6129298159600001,
0.60278850496,
0.61041415196,
0.67097248696,
0.64044649296,
0.6249596859600001
]
},
{
"command": "pnpr@HEAD",
"mean": 0.67051072246,
"stddev": 0.014410829482793392,
"median": 0.66781381296,
"user": 0.37877459999999996,
"system": 1.346096,
"min": 0.65632714396,
"max": 0.7064248499600001,
"times": [
0.67634875196,
0.7064248499600001,
0.65632714396,
0.66503954596,
0.67058807996,
0.67161788296,
0.6603462079600001,
0.65910500096,
0.6758970229600001,
0.66341273796
]
},
{
"command": "pnpr@main",
"mean": 0.67359272406,
"stddev": 0.01363290987654758,
"median": 0.6751460624600001,
"user": 0.37904769999999993,
"system": 1.3467958999999998,
"min": 0.6559231119600001,
"max": 0.69666210096,
"times": [
0.65873158296,
0.6632411729600001,
0.6759282509600001,
0.6860610889600001,
0.69666210096,
0.6616532149600001,
0.6559231119600001,
0.67559154196,
0.68743459196,
0.67470058296
]
}
]
}Scenario: Isolated linker: fresh install, cold cache + cold store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 4.24505336788,
"stddev": 0.05005755927511568,
"median": 4.235113479680001,
"user": 3.8195024999999996,
"system": 3.29508346,
"min": 4.18783972018,
"max": 4.326789719180001,
"times": [
4.20469997318,
4.32521535918,
4.24187983018,
4.24846296018,
4.326789719180001,
4.22360242518,
4.18783972018,
4.22834712918,
4.27328240118,
4.1904141611800005
]
},
{
"command": "pacquet@main",
"mean": 4.275703499180001,
"stddev": 0.06748709715267419,
"median": 4.25368436568,
"user": 3.8243495000000003,
"system": 3.3348065599999996,
"min": 4.1853468541800005,
"max": 4.428949307180001,
"times": [
4.3187354971800005,
4.30655761518,
4.2264108381800005,
4.2436418621800005,
4.25147332718,
4.30192048318,
4.428949307180001,
4.1853468541800005,
4.2381038031800005,
4.25589540418
]
},
{
"command": "pnpr@HEAD",
"mean": 2.1319346321799997,
"stddev": 0.11665190737233663,
"median": 2.09102938768,
"user": 2.5645441999999994,
"system": 2.8119147599999996,
"min": 2.02162333318,
"max": 2.37681921818,
"times": [
2.04784915018,
2.0395373701799997,
2.13215685618,
2.27011592018,
2.04905220018,
2.20013349818,
2.02162333318,
2.11056350318,
2.37681921818,
2.07149527218
]
},
{
"command": "pnpr@main",
"mean": 2.19170610698,
"stddev": 0.13516383897488232,
"median": 2.17749868768,
"user": 2.5267293,
"system": 2.78806496,
"min": 2.01271193918,
"max": 2.40629201618,
"times": [
2.03964318318,
2.18783405818,
2.2433107091799998,
2.32211662018,
2.07536743318,
2.16716331718,
2.11673522518,
2.01271193918,
2.34588656818,
2.40629201618
]
}
]
}Scenario: Isolated linker: fresh install, hot cache + hot store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 1.38681997032,
"stddev": 0.02415056215278689,
"median": 1.38484116882,
"user": 1.38943592,
"system": 1.6968910799999999,
"min": 1.34661628982,
"max": 1.42391107182,
"times": [
1.41637043882,
1.39338656882,
1.34661628982,
1.3633176748200002,
1.38450938782,
1.36873318282,
1.38517294982,
1.3794050158200002,
1.42391107182,
1.4067771228200001
]
},
{
"command": "pacquet@main",
"mean": 1.38888669332,
"stddev": 0.022069130423829107,
"median": 1.3906734433199999,
"user": 1.37200622,
"system": 1.73116168,
"min": 1.34272576482,
"max": 1.41336109682,
"times": [
1.40919962482,
1.38268301482,
1.4012572578200002,
1.39324515482,
1.34272576482,
1.3645680178200001,
1.38810173182,
1.4093369608200002,
1.3843883088200002,
1.41336109682
]
},
{
"command": "pnpr@HEAD",
"mean": 0.6917947824199999,
"stddev": 0.11997013799823006,
"median": 0.6549275318200001,
"user": 0.33662941999999996,
"system": 1.28682448,
"min": 0.6274181338200001,
"max": 1.03062639182,
"times": [
0.6490736068200001,
0.6465061238200001,
0.6274181338200001,
0.6415593948200001,
1.03062639182,
0.6658125918200001,
0.6607814568200001,
0.64848536582,
0.6775406468200001,
0.6701441118200001
]
},
{
"command": "pnpr@main",
"mean": 0.6563520626200001,
"stddev": 0.012799240633194679,
"median": 0.6536378153200001,
"user": 0.33634501999999994,
"system": 1.27914198,
"min": 0.6424865938200001,
"max": 0.6826401138200001,
"times": [
0.6439908168200001,
0.6449828128200001,
0.6424865938200001,
0.6559514778200001,
0.6500193968200001,
0.6614566108200001,
0.66004043982,
0.6826401138200001,
0.6706282108200001,
0.6513241528200001
]
}
]
}Scenario: Isolated linker: fresh install, cold cache + hot store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 3.1068608046799997,
"stddev": 0.09043493367747436,
"median": 3.0918680835799996,
"user": 1.8116799800000003,
"system": 2.02701288,
"min": 3.01735517358,
"max": 3.31219815358,
"times": [
3.31219815358,
3.0941833015799998,
3.15756642158,
3.06419030558,
3.01735517358,
3.03303323358,
3.0199608145799997,
3.18116892058,
3.0993988565799997,
3.08955286558
]
},
{
"command": "pacquet@main",
"mean": 3.07173602908,
"stddev": 0.04807765309280706,
"median": 3.0665906120799997,
"user": 1.8239559800000003,
"system": 1.9892898799999998,
"min": 2.99964836758,
"max": 3.16007503758,
"times": [
3.04548486858,
2.99964836758,
3.06679419958,
3.0704242915799997,
3.10884101958,
3.06638702458,
3.02349902258,
3.16007503758,
3.1256498535799997,
3.0505566055799997
]
},
{
"command": "pnpr@HEAD",
"mean": 0.66537097138,
"stddev": 0.007369242811170983,
"median": 0.66764989108,
"user": 0.3394044799999999,
"system": 1.3194470799999998,
"min": 0.6535303985800001,
"max": 0.67524057658,
"times": [
0.66685689858,
0.66930147458,
0.67524057658,
0.66844288358,
0.6687202305800001,
0.6736449665800001,
0.65908896858,
0.6535303985800001,
0.66360076258,
0.65528255358
]
},
{
"command": "pnpr@main",
"mean": 0.6537102297799999,
"stddev": 0.010073410071606646,
"median": 0.65325371808,
"user": 0.33746128,
"system": 1.27682378,
"min": 0.63811417058,
"max": 0.66996221258,
"times": [
0.6491991915800001,
0.64343628458,
0.66222949558,
0.65591837558,
0.65058906058,
0.63811417058,
0.6459584525800001,
0.65692150358,
0.66996221258,
0.66477355058
]
}
]
} |
|
| Branch | pr/12158 |
| 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,245.05 ms(+1.59%)Baseline: 4,178.67 ms | 5,014.41 ms (84.66%) |
| isolated-linker.fresh-install.cold-cache.hot-store | 📈 view plot 🚷 view threshold | 3,106.86 ms(+3.82%)Baseline: 2,992.57 ms | 3,591.08 ms (86.52%) |
| isolated-linker.fresh-install.hot-cache.hot-store | 📈 view plot 🚷 view threshold | 1,386.82 ms(+5.77%)Baseline: 1,311.15 ms | 1,573.38 ms (88.14%) |
| isolated-linker.fresh-restore.cold-cache.cold-store | 📈 view plot 🚷 view threshold | 4,158.19 ms(+1.97%)Baseline: 4,077.75 ms | 4,893.31 ms (84.98%) |
| isolated-linker.fresh-restore.hot-cache.hot-store | 📈 view plot 🚷 view threshold | 621.65 ms(+1.32%)Baseline: 613.54 ms | 736.25 ms (84.43%) |
|
| Branch | pr/12158 |
| 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,131.93 ms |
| isolated-linker.fresh-install.cold-cache.hot-store | 📈 view plot | 665.37 ms |
| isolated-linker.fresh-install.hot-cache.hot-store | 📈 view plot | 691.79 ms |
| isolated-linker.fresh-restore.cold-cache.cold-store | 📈 view plot | 2,093.42 ms |
| isolated-linker.fresh-restore.hot-cache.hot-store | 📈 view plot | 670.51 ms |
…test Mirrors pnpm's regression test for keeping lockfile overrides that resolve through a catalog in sync when `update --latest` bumps that catalog. pacquet already behaves correctly (it threads the bumped catalogs through to override parsing), so this is a guard against a future refactor reintroducing the inconsistency that pnpm#12158 fixes on the TypeScript side.
|
Code review by qodo was updated up to the latest commit 6f6ea2c |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #12158 +/- ##
==========================================
+ Coverage 88.08% 88.10% +0.01%
==========================================
Files 310 310
Lines 41847 41863 +16
==========================================
+ Hits 36862 36883 +21
+ Misses 4985 4980 -5 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
@zkochan Thanks for the review and merge. |
Summary
pnpm updatecan leavepnpm.overridesthat reference a catalog stale in the lockfile. When an override usescatalog:(e.g.overrides: { foo: 'catalog:' }) andpnpm updatebumps that catalog entry, the lockfile'scatalogssection advances but the resolvedoverrideskeep the old version, so the lockfile is internally inconsistent and a laterpnpm install --frozen-lockfilefails withERR_PNPM_LOCKFILE_CONFIG_MISMATCH. Re-running a non-frozenpnpm installreconciles it, which is why it currently "needs a second install".Reproduction
https://github.com/why-reproductions-are-required/pnpm-update-catalog-overrides
Reproduces on pnpm 11.x (verified on 11.5.1).
Root cause
opts.parsedOverridesis resolved once, against the pre-update catalog, when install options are extended (extendInstallOptions.ts).pnpm updatethen bumps the catalog during resolution (updatedCatalogs) and rewrites the lockfile'scatalogssection, but theoverridesmap written to the lockfile is never re-resolved against the bumped catalog.Fix
After resolution, when
updatedCatalogsis present and overrides are configured, re-resolve the overrides against the catalog merged withupdatedCatalogsand write the result tonewLockfile.overrides(before theafterAllResolvedhook, so a pnpmfile hook can still amend them). This matches what a subsequent install computes (it reads the post-update catalog), so the lockfile is frozen-valid in one pass.Test
installing/deps-installer/test/catalogs.ts-> "overrides that reference a catalog are updated in the lockfile when the catalog is updated": acatalog:dependency plus a scoped override that resolves through the catalog; after anupdatethat bumps the catalog, the lockfileoverridesnow track it and a follow-up frozen install passes. Fails before the fix, passes after.Known limitation (follow-up issue)
This fixes the lockfile metadata (
overrides), which covers the common case where overrides reference acatalog:that is bumped (including overrides on directcatalog:dependencies that are themselves updated). It does not yet cover an active override on a transitive dependency: because the resolutionreadPackageHookis built from the pre-updateparsedOverrides(andupdatedCatalogsis only known afterresolveDependencies), an override that forces a transitive dependency can still be applied with the old catalog value during resolution while the metadata records the new value, leaving the resolved graph out of sync. Fixing that requires re-resolving against the updated catalog. Tracked in #12159.pacquet parity
No pacquet change needed yet: pacquet has no lockfile
catalogssection and no catalog-bump-on-update(updatedCatalogs), so the stale-overrides condition cannot arise. When pacquet ports catalog-updating-during-update, the override re-resolution must be ported alongside it.Summary by CodeRabbit
Bug Fixes
overridesthat reference catalogs could become unsynchronized afterpnpm update, which could later causepnpm install --frozen-lockfileto fail with a lockfile mismatch.catalogsandoverridesaligned.Tests
overridessynchronized and that subsequent--frozen-lockfileinstalls succeed.