fix: npm warnings and add BATS to Dockerfile - #556
Conversation
- Comment out pnpm-specific settings in .npmrc to avoid npm warnings - Add bats package to Dockerfile for integration testing 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
🔍 AI Code Review (Local Hook)Models: Codex (default) / Gemini (default) 🤖 Codex Review指摘事項はありません。 判定 ✨ Gemini ReviewMCP server 'supabase' requires authentication using: /mcp auth supabaseMCP server 'vercel' requires authentication using: /mcp auth vercelコードレビューありがとうございます。以下に発見事項をまとめました。 発見事項
全体的な判定
|
🔍 AI Code Review (Local Hook)Models: Codex (default) / Gemini (default) 🤖 Codex Review指摘事項はありません。変更は 判定: patch is correct（理由: 変更はいずれも既存挙動を壊す具体的リスクがなく、導入された問題が見当たらないため） ✨ Gemini ReviewMCP server 'supabase' requires authentication using: /mcp auth supabaseMCP server 'vercel' requires authentication using: /mcp auth vercelはい、承知いたしました。コード変更のレビューを実施します。 発見事項
全体的な判定
|
PR レビュー: fix: npm warnings and add BATS to Dockerfile概要変更内容を確認しました。概ね方向性は正しいですが、いくつか懸念点があります。 Dockerfile の変更
|
| 設定 | 目的 |
|---|---|
strict-peer-dependencies=true |
peer dependency の競合でインストール失敗（セキュリティ強化） |
shamefully-hoist=false |
ファントム依存を防ぐ |
verify-store-integrity=true |
コンテンツ整合性の検証 |
推奨する対応:
pnpm 専用の設定ファイル .pnpmrc を作成し、そちらに pnpm 固有の設定を移す方法が最もクリーンです：
# .pnpmrc
strict-peer-dependencies=true
auto-install-peers=true
shamefully-hoist=false
verify-store-integrity=trueこれにより npm の警告も消え、pnpm 使用時のセキュリティ設定も維持されます。
セキュリティ観点
CLAUDE.md のポリシー「SAST / 依存脆弱性: Critical検知でFail」に照らすと、verify-store-integrity=true と shamefully-hoist=false は依存関係のセキュリティ担保に直結するため、コメントアウトのままにしておくことは望ましくありません。
まとめ
| 項目 | 評価 |
|---|---|
Dockerfile への bats 追加 |
✅ 問題なし（動作確認を推奨） |
.npmrc の pnpm 設定コメントアウト |
|
| コメントの誤記載 |
.pnpmrc ファイルに pnpm 固有設定を移す対応をお願いします。
🤖 Reviewed by Claude Code
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2db0cacd2c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # pnpm-specific settings (ignored by npm, used when pnpm is detected) | ||
| # ============================================================ | ||
| # Strict peer dependency resolution - fail on conflicts | ||
| # strict-peer-dependencies=true |
There was a problem hiding this comment.
Commenting out strict-peer-dependencies (and related pnpm flags) removes those protections for pnpm as well, because commented .npmrc entries are never read by any package manager. In this repo we still execute pnpm commands (for example in .devcontainer/Dockerfile), so peer dependency conflicts that were previously blocked can now be silently accepted during dependency resolution, which weakens the intended supply-chain/consistency guardrails.
Useful? React with 👍 / 👎.
📝 WalkthroughWalkthroughAdded bats testing tool to the development container, and commented out strict pnpm peer dependency and store integrity verification settings in the npm configuration, reducing enforcement constraints. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.npmrc:
- Around line 6-19: Remove the commented pnpm settings from .npmrc and re-add
them in pnpm-workspace.yaml using pnpm-workspace's camelCase keys:
strictPeerDependencies: true, autoInstallPeers: true, shamefullyHoist: false,
and verifyStoreIntegrity: true; ensure the four keys are top-level in
pnpm-workspace.yaml so pnpm installs and CI workflows (which expect these
safeguards) honor them and npm no longer emits warnings from the .npmrc
comments.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f443fdf9-97ec-413e-8825-89fa0be95eea
📒 Files selected for processing (2)
.devcontainer/Dockerfile.npmrc
| # ============================================================ | ||
| # pnpm-specific settings (ignored by npm, used when pnpm is detected) | ||
| # ============================================================ | ||
| # Strict peer dependency resolution - fail on conflicts | ||
| # strict-peer-dependencies=true | ||
|
|
||
| # Auto-install missing peer dependencies | ||
| # auto-install-peers=true | ||
|
|
||
| # Prevent phantom dependencies (packages hoisting to root node_modules) | ||
| shamefully-hoist=false | ||
| # shamefully-hoist=false | ||
|
|
||
| # Use content-addressable storage (pnpm default - integrity verification) | ||
| verify-store-integrity=true | ||
| # verify-store-integrity=true |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What is pnpm's recommended way to keep strict-peer-dependencies, auto-install-peers, shamefully-hoist, and verify-store-integrityenabled for pnpm without npm warning about unknown.npmrc settings?
💡 Result:
pnpm’s recommended way to avoid npm warnings about pnpm-only config keys is to move those settings out of the project .npmrc and into the project-level pnpm-workspace.yaml, which pnpm reads for configuration but npm does not. pnpm explicitly supports reading config from pnpm-workspace.yaml in addition to .npmrc. [1]
Example pnpm-workspace.yaml:
# pnpm-workspace.yaml
strictPeerDependencies: true
autoInstallPeers: true
shamefullyHoist: true
verifyStoreIntegrity: trueThese exact settings are documented as supported pnpm-workspace.yaml keys (note the camelCase names). [1]
🏁 Script executed:
# Check if pnpm-workspace.yaml exists
fd -t f "pnpm-workspace.yaml" .Repository: keito4/config
Length of output: 78
🏁 Script executed:
# Check the .npmrc file to see the exact settings
cat -n .npmrcRepository: keito4/config
Length of output: 833
🏁 Script executed:
# Check the pnpm-workspace.yaml file
cat -n pnpm-workspace.yamlRepository: keito4/config
Length of output: 381
Migrate pnpm safeguards from .npmrc to pnpm-workspace.yaml instead of disabling them.
Commenting out Lines 10, 13, 16, and 19 disables pnpm's peer-dependency, hoisting, and store-integrity checks. The repo still provisions pnpm in .devcontainer/Dockerfile and workflows rely on these settings (.github/workflows/templates/unified-ci.yml:84-89), so weakening them affects all pnpm installs.
Rather than comment them out, move these settings to pnpm-workspace.yaml using the camelCase format that pnpm-workspace supports:
strictPeerDependencies: trueautoInstallPeers: trueshamefullyHoist: falseverifyStoreIntegrity: true
This avoids npm warnings while keeping pnpm safeguards active across the repo.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.npmrc around lines 6 - 19, Remove the commented pnpm settings from .npmrc
and re-add them in pnpm-workspace.yaml using pnpm-workspace's camelCase keys:
strictPeerDependencies: true, autoInstallPeers: true, shamefullyHoist: false,
and verifyStoreIntegrity: true; ensure the four keys are top-level in
pnpm-workspace.yaml so pnpm installs and CI workflows (which expect these
safeguards) honor them and npm no longer emits warnings from the .npmrc
comments.
|
🎉 This PR is included in version 1.93.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
1 participant
Summary
.npmrcto avoid npm warningsbatspackage to Dockerfile for integration testingTest plan
🤖 Generated with Claude Code
Summary by CodeRabbit