Repository navigation
fix(manifest): reject invalid dependency specifiers during install - #16416
Merged
zkochan merged 3 commits intoOct 5, 2026
Merged
Conversation
Contributor
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
🧰 Additional context used📚 Code guidelines (3)No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (2)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 3 remain after this review. 📝 WalkthroughWalkthroughPackage manifests now validate dependency specifier types. Install execution applies this validation before lockfile settlement and propagates validation errors. Tests cover invalid specifiers and a ChangesDependency manifest validation
Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Bug fix Suggested labels: Merge Risk: ⚪ Minimal · up to The change makes pnpm install fail on non-string dependency specifiers instead of silently omitting them from the lockfile. A readPackage hook can still repair such a value. No merge-blocking risk was identified. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The change strengthens validation before installation records dependency state, while preserving hook-based repair and lockfile-driven commands. No introduced security issue was substantiated. Concurrent-install and abrupt-interruption behavior remain only partially established. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
✨ 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #16416 +/- ##
=======================================
Coverage 92.32% 92.32%
=======================================
Files 1577 1579 +2
Lines 208925 208973 +48
=======================================
+ Hits 192882 192932 +50
+ Misses 16043 16041 -2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
tunglambk
force-pushed
the
fix/reject-invalid-dependency-specifiers
branch
from
September 30, 2026 03:03
1934737 to
1009af9
Compare
Contributor
Same-run regression checkHEAD and main are measured on the same runner. The tolerance is 5% of the main median or 2 ms, whichever is larger. A regression requires both the median slowdown and the gap between the fastest retained HEAD sample and slowest retained main sample to exceed this tolerance, after trimming up to 10% from each tail. Suspected regressions are rerun once with target order reversed and only fail if reproduced. This conservative noise guard is not a statistical confidence interval. Inconclusive rows do not fail the check. They remain visible for performance review. Bencher retains absolute minimum timings for historical tracking; historical alerts do not gate this PR. Sequential measurements can still be affected by runner drift.
Integrated-Benchmark Report (Linux)Commit: Regular scenarios report direct and pnpr installs. The peer-heavy resolver scenario compares current Rust, main Rust, and TypeScript pnpm; the linked-workspace scenario compares current Rust against main Rust. Bencher consumes pacquet@HEAD and pnpr@HEAD. The tables below show mean ± σ. Bencher tracks absolute minimum latency; PR regression checks compare HEAD and main samples from the same runner. Historical timing alerts do not gate PRs. Scenario: Isolated linker: fresh restore, cold cache + cold store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 2.58496891334,
"stddev": 0.11938740894484227,
"median": 2.5517389327399997,
"user": 2.23592878,
"system": 2.13610028,
"min": 2.4251325917399997,
"max": 2.78242060274
},
{
"command": "pacquet@main",
"mean": 2.63018490564,
"stddev": 0.11923383223053997,
"median": 2.5983330907399997,
"user": 2.2911168799999997,
"system": 2.00406878,
"min": 2.4374393197399997,
"max": 2.83259059074
},
{
"command": "pnpr@HEAD",
"mean": 1.48918022714,
"stddev": 0.13700813098271897,
"median": 1.44955985474,
"user": 1.9041612799999998,
"system": 1.8287870800000001,
"min": 1.3618438417399998,
"max": 1.81823962574
},
{
"command": "pnpr@main",
"mean": 1.55450342594,
"stddev": 0.14116013161545676,
"median": 1.53342835774,
"user": 1.7732795799999999,
"system": 1.56826468,
"min": 1.3693096107399998,
"max": 1.74231250374
}
]
}Scenario: Isolated linker: fresh restore, hot cache + hot store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 0.4782792625400001,
"stddev": 0.055076706159470114,
"median": 0.47127401324000007,
"user": 0.5541088000000001,
"system": 0.9618801400000001,
"min": 0.39750233674,
"max": 0.55500731374
},
{
"command": "pacquet@main",
"mean": 0.4319133655400001,
"stddev": 0.06876966620801953,
"median": 0.41752734624000004,
"user": 0.49246239999999997,
"system": 0.94969214,
"min": 0.35041284974000003,
"max": 0.54065598674
},
{
"command": "pnpr@HEAD",
"mean": 0.33762437364000003,
"stddev": 0.039707985975331726,
"median": 0.33024181874,
"user": 0.3953228,
"system": 0.72276824,
"min": 0.28662089674,
"max": 0.40393037274000004
},
{
"command": "pnpr@main",
"mean": 0.34022724064000004,
"stddev": 0.018622308458317274,
"median": 0.34461614924000006,
"user": 0.39552319999999996,
"system": 0.71205054,
"min": 0.31788843274000006,
"max": 0.37738038574000005
}
]
}Scenario: Isolated linker: repeat install, hot cache + hot store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 0.01314975606895833,
"stddev": 0.0012603480509404874,
"median": 0.01311323122,
"user": 0.007307915833333332,
"system": 0.007353552083333335,
"min": 0.01055378172,
"max": 0.016618775720000003
},
{
"command": "pacquet@main",
"mean": 0.012652790435686278,
"stddev": 0.0015786272272089968,
"median": 0.012518456720000001,
"user": 0.007265436666666669,
"system": 0.006805005686274512,
"min": 0.00939323672,
"max": 0.01750969872
},
{
"command": "pnpr@HEAD",
"mean": 0.01289791511473684,
"stddev": 0.0016291464904458729,
"median": 0.012742630720000002,
"user": 0.006961635789473683,
"system": 0.007384250947368421,
"min": 0.009364333720000001,
"max": 0.017152228720000003
},
{
"command": "pnpr@main",
"mean": 0.011552512952876714,
"stddev": 0.0011118472122350566,
"median": 0.01134109672,
"user": 0.006653204931506848,
"system": 0.006177777534246575,
"min": 0.009461653720000001,
"max": 0.015419394720000002
}
]
}Scenario: Isolated linker: repeat install, cold cache + hot store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 0.014716992162068966,
"stddev": 0.0019181080368622899,
"median": 0.014758927900000001,
"user": 0.008113046206896549,
"system": 0.00821852206896552,
"min": 0.011145192900000002,
"max": 0.019544617900000002
},
{
"command": "pacquet@main",
"mean": 0.015346116249999995,
"stddev": 0.001865532670470421,
"median": 0.015246872400000002,
"user": 0.008702174285714282,
"system": 0.008272245714285717,
"min": 0.0107779189,
"max": 0.0210608429
},
{
"command": "pnpr@HEAD",
"mean": 0.016175009656249992,
"stddev": 0.001481028477748176,
"median": 0.0162434714,
"user": 0.009105847499999998,
"system": 0.00878352625,
"min": 0.012608423900000002,
"max": 0.0196280409
},
{
"command": "pnpr@main",
"mean": 0.01598363341492537,
"stddev": 0.0016745127119314291,
"median": 0.0160926624,
"user": 0.009334870447761196,
"system": 0.008418589552238809,
"min": 0.012481657900000002,
"max": 0.021307921900000002
}
]
}Scenario: Isolated linker: fresh install, cold cache + cold store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 2.68151949014,
"stddev": 0.08661734346315392,
"median": 2.6902136895399997,
"user": 2.3978456599999998,
"system": 1.92280972,
"min": 2.56350364504,
"max": 2.81810348604
},
{
"command": "pacquet@main",
"mean": 2.7410159411399997,
"stddev": 0.10504518647225156,
"median": 2.7578099740399997,
"user": 2.6860465600000003,
"system": 2.0182102199999994,
"min": 2.59837336604,
"max": 2.93298869104
},
{
"command": "pnpr@HEAD",
"mean": 1.59492896864,
"stddev": 0.11465473895989106,
"median": 1.55756745954,
"user": 1.5867414599999998,
"system": 1.6007975199999998,
"min": 1.4672694320400002,
"max": 1.8762052670400002
},
{
"command": "pnpr@main",
"mean": 1.76685689964,
"stddev": 0.09270546276775526,
"median": 1.79340604704,
"user": 1.8121738600000001,
"system": 1.9473538199999996,
"min": 1.6413280190400001,
"max": 1.8955774600400002
}
]
}Scenario: Isolated linker: fresh install, hot cache + hot store
Confirmation (reversed target order)
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 0.86049864414,
"stddev": 0.13985207753987278,
"median": 0.8116109368400001,
"user": 1.2490173199999999,
"system": 1.1799193799999999,
"min": 0.6955043763400001,
"max": 1.07072889234
},
{
"command": "pacquet@main",
"mean": 0.9407711393400001,
"stddev": 0.07944394762447222,
"median": 0.9270608788400001,
"user": 1.3433591199999997,
"system": 1.3237859799999998,
"min": 0.8311616153400001,
"max": 1.06946099734
},
{
"command": "pnpr@HEAD",
"mean": 0.5145157799400001,
"stddev": 0.03878055781254938,
"median": 0.49794497634,
"user": 0.5278888199999999,
"system": 0.9050731799999999,
"min": 0.47298002534000005,
"max": 0.5858444163400001
},
{
"command": "pnpr@main",
"mean": 0.43512407844000006,
"stddev": 0.01922190381326951,
"median": 0.43543564534,
"user": 0.42985342,
"system": 0.7981237800000001,
"min": 0.3992323863400001,
"max": 0.4741599443400001
}
],
"confirmation": {
"results": [
{
"command": "pnpr@main",
"mean": 0.44840504890000005,
"stddev": 0.013515615750907739,
"median": 0.4472053507,
"user": 0.41358754,
"system": 0.8563954599999999,
"min": 0.4242549922,
"max": 0.46555976320000003
},
{
"command": "pnpr@HEAD",
"mean": 0.43913928659999996,
"stddev": 0.02975526212696939,
"median": 0.4424297962,
"user": 0.4381093399999999,
"system": 0.7997845599999999,
"min": 0.3757288992,
"max": 0.4796881692
},
{
"command": "pacquet@main",
"mean": 0.8519231347000001,
"stddev": 0.07666480714081769,
"median": 0.8288284367000001,
"user": 1.20981924,
"system": 1.19719586,
"min": 0.7363042002000001,
"max": 1.0150319022
},
{
"command": "pacquet@HEAD",
"mean": 0.8388991438000002,
"stddev": 0.05399250226527655,
"median": 0.8198058172,
"user": 1.2196888399999997,
"system": 1.1774732600000002,
"min": 0.8006252422000001,
"max": 0.9703991432000001
}
]
}
}Scenario: Isolated linker: fresh install, cold cache + hot store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 2.03961946678,
"stddev": 0.11329699586770871,
"median": 2.00161730038,
"user": 1.59034836,
"system": 1.5207397999999999,
"min": 1.91553017188,
"max": 2.26224771288
},
{
"command": "pacquet@main",
"mean": 2.06779276248,
"stddev": 0.05100717862348642,
"median": 2.06883693038,
"user": 1.6027347600000001,
"system": 1.5507034999999998,
"min": 1.98024542988,
"max": 2.1591072588799998
},
{
"command": "pnpr@HEAD",
"mean": 0.43651016998000003,
"stddev": 0.0339931876345218,
"median": 0.42773429738,
"user": 0.41637806,
"system": 0.9029425000000002,
"min": 0.39428612888,
"max": 0.49461479688
},
{
"command": "pnpr@main",
"mean": 0.47968209268000006,
"stddev": 0.020568081758240403,
"median": 0.48166645238000005,
"user": 0.46690666,
"system": 0.9302247999999999,
"min": 0.44720739588,
"max": 0.50976449288
}
]
}Scenario: Isolated linker: fresh resolve, hot cache, offline
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 0.31324830914,
"stddev": 0.028723274103936002,
"median": 0.30229782684000006,
"user": 0.3090987399999999,
"system": 0.1404082,
"min": 0.27905042184,
"max": 0.35831388484000004
},
{
"command": "pacquet@main",
"mean": 0.32784208474000004,
"stddev": 0.03705960832285307,
"median": 0.33007714734000004,
"user": 0.32556613999999995,
"system": 0.15041219999999997,
"min": 0.26399274884,
"max": 0.38583440084000004
},
{
"command": "pnpr@HEAD",
"mean": 0.11371739453565217,
"stddev": 0.0076161424319804456,
"median": 0.11402791684000001,
"user": 0.05041284434782609,
"system": 0.02300598260869565,
"min": 0.09719921384,
"max": 0.12741329484
},
{
"command": "pnpr@main",
"mean": 0.12381823703047619,
"stddev": 0.007570397698565201,
"median": 0.12254460584000001,
"user": 0.05728434952380952,
"system": 0.027596342857142853,
"min": 0.11024399484000001,
"max": 0.13821848684000002
}
]
}Scenario: Isolated linker: peer-heavy resolve, hot cache, offline
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 0.3256610305222223,
"stddev": 0.037964270358299725,
"median": 0.31012074030000003,
"user": 0.34726703555555555,
"system": 0.07501299111111111,
"min": 0.28910716430000005,
"max": 0.3997495823000001
},
{
"command": "pacquet@main",
"mean": 0.2934884658555556,
"stddev": 0.04136907103798834,
"median": 0.2768445493,
"user": 0.31485725777777784,
"system": 0.07211665777777777,
"min": 0.25053615530000006,
"max": 0.3750065443
},
{
"command": "pnpm@HEAD",
"mean": 4.571887614855555,
"stddev": 0.29745631558451985,
"median": 4.5787718613,
"user": 7.3703751466666665,
"system": 0.4480034355555555,
"min": 4.0664488343,
"max": 5.1592603563
}
]
}Scenario: Isolated linker: linked-workspace resolve, hot cache, offline
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 0.2574032541545454,
"stddev": 0.01753612407177372,
"median": 0.2602130487,
"user": 0.21462687272727274,
"system": 0.09864746909090907,
"min": 0.2307332387,
"max": 0.2921561187
},
{
"command": "pacquet@main",
"mean": 0.24946024260909094,
"stddev": 0.014676297966604836,
"median": 0.24266234370000003,
"user": 0.18770950909090905,
"system": 0.11574292363636363,
"min": 0.23265458870000003,
"max": 0.27045687370000004
}
]
}Scenario: Isolated linker: fresh restore, cold cache + cold store + cold pnpr
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 4.5939439570200005,
"stddev": 0.18189866906611424,
"median": 4.5269532983200005,
"user": 1.76113982,
"system": 1.70205982,
"min": 4.39553735332,
"max": 4.90539829532
},
{
"command": "pacquet@main",
"mean": 4.768916291720001,
"stddev": 0.2844395351422548,
"median": 4.694843626320001,
"user": 2.09645082,
"system": 1.87125652,
"min": 4.4779034633200006,
"max": 5.32993937432
},
{
"command": "pnpr@HEAD",
"mean": 3.6044115333199995,
"stddev": 0.10032250519621434,
"median": 3.6469709243199997,
"user": 1.7432073199999998,
"system": 1.6979422199999998,
"min": 3.4449748373199998,
"max": 3.71566273332
},
{
"command": "pnpr@main",
"mean": 3.6165626867199996,
"stddev": 0.13404994504730652,
"median": 3.57194835482,
"user": 1.6264912200000006,
"system": 1.4801732199999997,
"min": 3.46363041532,
"max": 3.84502972232
}
]
}Scenario: GVS linker: fresh restore, hot cache + hot storeSame install as the isolated fresh-restore hot/hot scenario, into the shared virtual store, the layout pnpm 12 installs into by default. Scenarios run on separate machines, so compare main and branch within each table.
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 0.48275442150000003,
"stddev": 0.04782596309557285,
"median": 0.48391411070000007,
"user": 0.34209082,
"system": 0.22161408,
"min": 0.42531569820000004,
"max": 0.5539115422
},
{
"command": "pacquet@main",
"mean": 0.42836263100000005,
"stddev": 0.048915631513759765,
"median": 0.42204262620000005,
"user": 0.31695552,
"system": 0.20878917999999996,
"min": 0.36800009820000007,
"max": 0.5098966332
},
{
"command": "pnpr@HEAD",
"mean": 0.32635574370000003,
"stddev": 0.12999073563401028,
"median": 0.25652227020000007,
"user": 0.31176212000000003,
"system": 0.23594278000000002,
"min": 0.2247049392,
"max": 0.5783667312
},
{
"command": "pnpr@main",
"mean": 0.22511260279999998,
"stddev": 0.028212916879756363,
"median": 0.22176917770000001,
"user": 0.28125962000000004,
"system": 0.21424657999999996,
"min": 0.1805084542,
"max": 0.26870691720000006
}
]
} |
tunglambk
force-pushed
the
fix/reject-invalid-dependency-specifiers
branch
from
September 30, 2026 03:20
1009af9 to
e000b4a
Compare
tunglambk
marked this pull request as ready for review
September 30, 2026 03:32
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? |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
github-actions
Bot
added
the
reviewed: coderabbit
tunglambk
force-pushed
the
fix/reject-invalid-dependency-specifiers
branch
from
October 5, 2026 02:50
e000b4a to
efb28ac
Compare
tunglambk
force-pushed
the
fix/reject-invalid-dependency-specifiers
branch
from
October 5, 2026 03:07
efb28ac to
3c79530
Compare
…tion rustfmt wants `scope.project_manifests` on the same line as the `&&`. This is `node pnpm/scripts/rustfmt.mjs --all` on its own; the token stream is unchanged.
zkochan
force-pushed
the
fix/reject-invalid-dependency-specifiers
branch
from
October 5, 2026 15:32
e5c502f to
fd788c1
Compare
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
pnpm v11 installs a project whose dependency group is null, an array, or a string, and treats it as declaring no dependencies. Only a non-string specifier inside an object group makes it fail. Rejecting the group shapes would have broken projects with, for example, "devDependencies": null. Restrict validation to specifiers, replace the resolved TODOs on PackageManifest::dependencies with its contract, and reword the changeset around the user-visible change.
Code Review by Qodo
1. Dependency metadata maps can still carry malformed values
|
PR Code Suggestions ✨Warning
No code suggestions found for the PR. |
Contributor
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
|
Code review by qodo was updated up to the latest commit 1da1604 |
PR Code Suggestions ✨Warning
No code suggestions found for the PR. |
Member
|
@coderabbitai review |
Contributor
✅ Action performedReview finished.
|
Member
|
Review-and-fix pass on head Fix pushed ( Qodo findings, verified and declined:
Status: all checks green. CodeRabbit reviewed Written by an agent (Claude Code, claude-opus-5-5). |
zkochan
approved these changes
Oct 5, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
pnpm install --lockfile-onlyon a project with"dependencies": {"is-positive": 42}currently exits successfully and writes a lockfile withoutis-positive. A later frozen install also succeeds, so the malformed declaration can go unnoticed in CI.Validate the specifiers in the four dependency groups after
readPackagehooks run and before settling the lockfile. A group that is not an object (for example"devDependencies": null) is still accepted as declaring no dependencies, matching pnpm v11. A malformed declaration bypasses the repeat-install shortcut so a hook can repair it; otherwise installation reportsERR_PNPM_PACKAGE_MANIFEST_INVALID_ATTRIBUTE. pnpm v11 already fails on a non-string specifier, so the change applies to the Rust v12 CLI.The regression test failed on the target branch and passes with the fix. Added coverage for frozen installs, hook repair, and all four dependency groups. Local checks passed: 142 package-manifest tests, 884 package-manager tests (one ignored), 18 install-configuration integration tests, workspace
cargo check, workspacecargo clippy --deny warnings, and pinned rustfmt.Squash Commit Body
Checklist
pnpm changeset) if this PR changes any published package. Keep it short and written for pnpm users — it becomes a release note.Written by an agent (Codex, GPT-6).
A formatting fix was pushed by a different agent (DeepSeek Harness, deepseek-v4.1-flash). The non-object group fix was pushed by Claude Code (claude-opus-5-5).
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by CodeRabbit
readPackagehook can correct an invalid dependency specifier before validation, allowing installation to continue with the corrected dependency.