build: migrate from yarn to pnpm to harden the supply chain - #456
Conversation
|
Warning Review limit reached
More reviews will be available in 6 minutes and 43 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughウォークスルーこのPRはプロジェクトを Yarn から pnpm に移行します。トップレベル設定と workspace ファイルを追加し、GitHub Actions ワークフローを pnpm 実行に置き換え、開発ドキュメントとテストコメントを pnpm ベースに更新します。 変更内容Yarn から pnpm への統合移行
🎯 3 (Moderate) | ⏱️ ~20 minutes
🚥 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 |
There was a problem hiding this comment.
Code Review
This pull request migrates the project's package manager from Yarn to pnpm, updating the configuration files, documentation, and scripts accordingly, while also removing Husky. Feedback on the changes highlights that the supply-chain hardening configurations (such as minimum release age and build script blocks) are incorrectly placed in pnpm-workspace.yaml where they are ignored by pnpm. The reviewer recommends moving these settings to .npmrc and package.json respectively, deleting the invalid workspace file, and updating the corresponding documentation in CLAUDE.md.
パッケージマネージャを yarn 1 (classic) から pnpm 11 へ移行する。 単なる置換ではなく、pnpm のサプライチェーン防御機能を有効化する ことが主目的: - インストール時ビルドスクリプトを既定でブロック (allowBuilds)。 unrs-resolver (jest のモジュール解決器) はプリビルドバイナリで 動作するためビルド不要 → false で明示的に拒否。 - minimumReleaseAge: 4320 (3日) で公開直後の新バージョンの自動 取り込みを抑止し、乗っ取り直後の悪性版を回避。緊急時は minimumReleaseAgeExclude で個別に即時採用可能。 - pnpm 標準の content-addressable store + 整合性検証。 minimumReleaseAgeExclude について: pnpm の minimumReleaseAge は解決時に古い版へ自動降格せず検証で弾く ため、browserslist 系のデータパッケージ (baseline-browser-mapping / caniuse-lite / electron-to-chromium) のように毎日リリースされる依存があると、3日クールダウンでは解決の たびに窓内の最新版を掴み CI の --frozen-lockfile が失敗する。これら は dev 専用・データのみで dist には一切バンドルされない (公開される action に含まれない) ため、供給網リスクが小さく除外する。 主な変更: - package.json: packageManager を pnpm@11.5.0 (SHA512 ピン) へ。 機能していなかった husky (v4 形式設定・.husky 無し) を撤去し 依存を削減。 - pnpm-workspace.yaml 新規: サプライチェーン設定を集約。 - yarn.lock 削除 → pnpm-lock.yaml 新規。 - CI (test.yml / windows.yml): pnpm/action-setup + setup-node (cache: pnpm) を SHA ピンで追加し、yarn → pnpm へ。 - dist/index.js: 依存をレンジ内最新へ再解決したため ncc バンドルを 再生成。lib/ は src 由来のため差分なし。 - README / CLAUDE.md / テストコメントの yarn 表記を pnpm へ更新。 検証: pnpm install --frozen-lockfile / build / package / test (97 passed) をローカルで確認済み。 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
ecc70bd to
8ec4112
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
README.md (1)
164-164:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winWindows バージョン表記の区切りを修正してください。
Line 164 に不要な空白があり、表の可読性を下げています。
📝 提案差分
-| **Windows** | `windows-latest`, `windows-2025 , windows-2022` | +| **Windows** | `windows-latest`, `windows-2025`, `windows-2022` |🤖 Prompt for 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. In `@README.md` at line 164, The Windows version row in the table contains an extra space in the cell string "| **Windows** | `windows-latest`, `windows-2025 , windows-2022` |"; update that cell to consistently format the items and remove the stray space and trailing spaces so it reads "`windows-latest`, `windows-2025`, `windows-2022`" (i.e., remove the space before the comma and any trailing whitespace).
🤖 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 @.github/workflows/test.yml:
- Around line 49-53: Update the two jobs that call actions/setup-node (the test
job and test_default_version job) to disable the pnpm cache when the PR comes
from a fork: replace the unconditional cache: pnpm setting with a conditional
that only sets cache: pnpm when github.event.pull_request.head.repo.fork is
false (i.e., not a fork); specifically modify the actions/setup-node steps so
their cache: pnpm is omitted or set to false for forked PRs and preserved
otherwise.
In @.github/workflows/windows.yml:
- Around line 37-41: The actions/setup-node step currently uses cache: pnpm
unconditionally (refer to the actions/setup-node usage and the pnpm install
--frozen-lockfile step); update the workflow to detect forked PRs (e.g., via
github.event.pull_request.head.repo.fork or comparing github.repository_owner to
github.event.pull_request.head.repo.owner) and either disable cache: pnpm for
forked PRs or derive a trust-isolated cache key (include the repo owner/name or
a trust flag in the cache key) so forks do not share cached pnpm state, and
ensure the pnpm install step still runs without cache when caching is disabled.
---
Outside diff comments:
In `@README.md`:
- Line 164: The Windows version row in the table contains an extra space in the
cell string "| **Windows** | `windows-latest`, `windows-2025 , windows-2022`
|"; update that cell to consistently format the items and remove the stray space
and trailing spaces so it reads "`windows-latest`, `windows-2025`,
`windows-2022`" (i.e., remove the space before the comma and any trailing
whitespace).
🪄 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
Run ID: 4808ddac-c14e-4fd0-b5d9-422daa18221a
⛔ Files ignored due to path filters (4)
dist/index.jsis excluded by!**/dist/**dist/index.js.mapis excluded by!**/dist/**,!**/*.mappnpm-lock.yamlis excluded by!**/pnpm-lock.yamlyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (8)
.github/workflows/test.yml.github/workflows/windows.yml.gitignoreCLAUDE.mdREADME.md__tests__/chromedriver-api.test.tspackage.jsonpnpm-workspace.yaml
✅ Files skipped from review due to trivial changes (2)
- .gitignore
- tests/chromedriver-api.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- package.json
- CLAUDE.md
actions/setup-node の cache: pnpm が pull_request (fork 含む) で無条件 に有効だと、信頼境界を跨いだキャッシュ汚染の余地が残る。fork PR の 場合のみ cache を空にして無効化し、自リポジトリの push / 同一リポPR ではキャッシュを維持する。 GitHub Actions 式では空文字が falsy のため、'pnpm' を truthy 側に置く 条件 (fork != true && 'pnpm' || '') で fork 時のみ '' になるようにする。 CodeRabbit のレビュー指摘 (test.yml / windows.yml) に対応。 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Overview
Migrate the package manager from yarn 1 (classic) to pnpm 11 as a supply-chain hardening measure. This is not a plain swap: the main goal is to turn on pnpm's built-in defenses.
Why pnpm (supply-chain defenses)
allowBuilds(install-time build scripts blocked by default)postinstalletc. becomes opt-in. Tampered packages can't run code on install.minimumReleaseAge: 4320(3-day cooldown)node_modulesunrs-resolver(jest's module resolver) runs from a prebuilt binary (@unrs/resolver-binding-*, an optional dep) and needs no native build, so it is explicitly denied viaallowBuilds: { unrs-resolver: false }.Note on
minimumReleaseAgeExcludepnpm's
minimumReleaseAgeis a verification gate, not a resolution filter: it does not automatically fall back to an older version, it rejects the install when the resolved tree contains a version that is too new. The browserslist data packages (baseline-browser-mapping,caniuse-lite,electron-to-chromium) publish almost daily, so a 3-day cooldown would makepnpm install --frozen-lockfile(used in CI) fail on every resolution. These packages are dev-only, data-only, and are never bundled intodist/(they don't ship in the published action), so their supply-chain risk is negligible and they are excluded from the cooldown. The 3-day default still fully applies to everything that ships or runs install scripts.Main changes
package.json: setpackageManagertopnpm@11.5.0(SHA512-pinned). Removed the non-functional husky setup (v4-style config block, no.huskydirectory) to cut a dependency.pnpm-workspace.yaml(new): consolidates the supply-chain settings.yarn.lockremoved →pnpm-lock.yaml(new).test.yml/windows.yml): addedpnpm/action-setup+actions/setup-node(cache: pnpm), both SHA-pinned, and switchedyarn→pnpm.dist/index.js: regenerated the ncc bundle because dependencies were re-resolved to the latest within their declared ranges (this is why the diff is large).lib/is built from our ownsrcand is unchanged.yarnreferences topnpmin README / CLAUDE.md / test comments.Verification
pnpm install --frozen-lockfile/pnpm build/pnpm package/pnpm testlocally: 97 passed.pnpm packageregenerates the artifact deterministically.Rollback
node_modulesis untracked, so reverting this PR restores the yarn setup completely.🤖 Generated with Claude Code
Summary by CodeRabbit
Chores
Documentation