feat: Quality Gates をリポジトリ自動検出方式に刷新 - #498
Conversation
package.json の scripts を自動解析して利用可能なチェックを検出する方式に変更。 npm/pnpm/yarn/bun のパッケージマネージャー自動判定にも対応し、 config-base イメージ経由で全 DevContainer/Codespaces 環境にデフォルト適用される。 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
📝 WalkthroughWalkthroughRefactors pre-git quality gates to dynamically detect and run checks from Changes
Sequence Diagram(s)sequenceDiagram
participant Git as Git (pre-commit/push)
participant Hook as pre_git_quality_gates.py
participant FS as Repo FS (package.json, scripts)
participant PM as Package Manager / Tooling (npm/pnpm/yarn/bun, node_modules)
Git->>Hook: Trigger hook
Hook->>FS: resolve repo_root (git rev-parse)
Hook->>PM: detect package manager & node_modules
Hook->>FS: read package.json scripts
alt checks found
Hook->>PM: run detected checks (format, lint, test, typecheck, shellcheck, optional scripts)
PM-->>Hook: stdout/stderr, exit codes
Hook->>Git: return success or failure (0 or 2) with summarized output
else no applicable checks
Hook->>Git: exit 0 (no-op)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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 |
🔍 AI Code Review (Local Hook)Models: Codex (default) / Gemini (default) 🤖 Codex Review
patch is incorrect。理由: Yarn PnP 構成で無条件に失敗する回帰があり、通常の開発フローを阻害します。信頼度 0.64
|
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.claude/hooks/README.md (1)
97-131:⚠️ Potential issue | 🟡 MinorInconsistency: README instructs manual
settings.local.jsonsetup, but CLAUDE.md says hooks are already insettings.json
CLAUDE.md(line 137) states: "これらは.claude/settings.jsonのhooksフィールドで設定されており" — meaning the hooks are already committed insettings.jsonand active by default (especially in the config-base image). But this README section still tells users to manually add the same config tosettings.local.jsonas a required setup step, which is contradictory.Consider one of:
- Removing or retitling this section to "カスタマイズ / オーバーライド方法" so it's clear the steps are optional.
- Adding a note at the top: "
settings.jsonに既に設定済みです。追加カスタマイズが必要な場合のみ、以下の手順でsettings.local.jsonに上書き設定できます。"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/hooks/README.md around lines 97 - 131, README inconsistency: update the Hooksの設定方法 section to clarify that the `hooks` are already present in `.claude/settings.json` (as stated in CLAUDE.md) and that adding entries to `.claude/settings.local.json` is optional for customization/overrides; either change the section title to "カスタマイズ / オーバーライド方法" or add a top note such as "`settings.json` に既に設定済みです。追加カスタマイズが必要な場合のみ、以下の手順で `settings.local.json` に上書き設定できます。" and keep the example JSON showing the `hooks` field for reference. Ensure you reference the `hooks` field and the filenames `.claude/settings.json` and `.claude/settings.local.json` in the edit.
🧹 Nitpick comments (1)
.claude/hooks/pre_git_quality_gates.py (1)
40-49: Consider checkingreturncodeexplicitly aftergit rev-parseThe missing-path case is handled by the
if not repo_rootguard at line 48, but an explicitreturncodecheck would be clearer and avoids relying on stdout being empty when git fails.🔧 Suggested clarification
result = subprocess.run( ["git", "rev-parse", "--show-toplevel"], capture_output=True, text=True, timeout=10 ) - repo_root = result.stdout.strip() + if result.returncode != 0: + sys.exit(0) + repo_root = result.stdout.strip() except Exception: sys.exit(0)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/hooks/pre_git_quality_gates.py around lines 40 - 49, Check result.returncode after calling subprocess.run in the block that invokes git rev-parse: if result.returncode is non-zero, exit early (sys.exit(0)) instead of relying on result.stdout being empty; update the logic around the result variable and repo_root assignment in pre_git_quality_gates.py so you only set repo_root from result.stdout when returncode == 0 and otherwise handle the failure path explicitly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.claude/hooks/pre_git_quality_gates.py:
- Around line 99-105: The comment above CHECK_CANDIDATES mentions a fallback to
"format" but the Format Check candidate's script_names currently only lists
["format:check"], causing repos with only a "format" script to skip checks;
either remove "format" from the comment if fallback is not intended, or add
"format" to the Format Check candidate's script_names (i.e., update the
CHECK_CANDIDATES entry for the Format Check to include both "format:check" and
"format") so that projects with only a "format" script are handled; modify the
CHECK_CANDIDATES array accordingly and keep the description/label ("Format
Check") unchanged.
---
Outside diff comments:
In @.claude/hooks/README.md:
- Around line 97-131: README inconsistency: update the Hooksの設定方法 section to
clarify that the `hooks` are already present in `.claude/settings.json` (as
stated in CLAUDE.md) and that adding entries to `.claude/settings.local.json` is
optional for customization/overrides; either change the section title to "カスタマイズ
/ オーバーライド方法" or add a top note such as "`settings.json`
に既に設定済みです。追加カスタマイズが必要な場合のみ、以下の手順で `settings.local.json` に上書き設定できます。" and keep
the example JSON showing the `hooks` field for reference. Ensure you reference
the `hooks` field and the filenames `.claude/settings.json` and
`.claude/settings.local.json` in the edit.
---
Nitpick comments:
In @.claude/hooks/pre_git_quality_gates.py:
- Around line 40-49: Check result.returncode after calling subprocess.run in the
block that invokes git rev-parse: if result.returncode is non-zero, exit early
(sys.exit(0)) instead of relying on result.stdout being empty; update the logic
around the result variable and repo_root assignment in pre_git_quality_gates.py
so you only set repo_root from result.stdout when returncode == 0 and otherwise
handle the failure path explicitly.
| # 優先度順: format:check > format > lint > test > typecheck > shellcheck | ||
| CHECK_CANDIDATES = [ | ||
| { | ||
| "name": "Format Check", | ||
| "command": ["npm", "run", "format:check"], | ||
| "script_names": ["format:check"], | ||
| "label": "Format Check", | ||
| "description": "コードフォーマットの検証", | ||
| "install_hint": "npm install" | ||
| }, |
There was a problem hiding this comment.
Comment-code discrepancy: format fallback is documented but not implemented
Line 99 comment lists format as a fallback priority (after format:check), but script_names for the Format Check candidate only contains ["format:check"]. Projects with only a format script will silently skip format checking.
If the omission is intentional — because format auto-modifies files and is unsuitable for a blocking gate — remove format from the comment. If it's meant to be a fallback, add it to script_names.
🐛 Option A — correct the comment (recommended if auto-fix scripts should be excluded)
-# 優先度順: format:check > format > lint > test > typecheck > shellcheck
+# 優先度順: format:check > lint > test > typecheck > shellcheck🐛 Option B — implement the fallback (if read-only `format` scripts exist in your repos)
{
- "script_names": ["format:check"],
+ "script_names": ["format:check", "format"],
"label": "Format Check",
"description": "コードフォーマットの検証",
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # 優先度順: format:check > format > lint > test > typecheck > shellcheck | |
| CHECK_CANDIDATES = [ | |
| { | |
| "name": "Format Check", | |
| "command": ["npm", "run", "format:check"], | |
| "script_names": ["format:check"], | |
| "label": "Format Check", | |
| "description": "コードフォーマットの検証", | |
| "install_hint": "npm install" | |
| }, | |
| # 優先度順: format:check > lint > test > typecheck > shellcheck | |
| CHECK_CANDIDATES = [ | |
| { | |
| "script_names": ["format:check"], | |
| "label": "Format Check", | |
| "description": "コードフォーマットの検証", | |
| }, |
| # 優先度順: format:check > format > lint > test > typecheck > shellcheck | |
| CHECK_CANDIDATES = [ | |
| { | |
| "name": "Format Check", | |
| "command": ["npm", "run", "format:check"], | |
| "script_names": ["format:check"], | |
| "label": "Format Check", | |
| "description": "コードフォーマットの検証", | |
| "install_hint": "npm install" | |
| }, | |
| # 優先度順: format:check > format > lint > test > typecheck > shellcheck | |
| CHECK_CANDIDATES = [ | |
| { | |
| "script_names": ["format:check", "format"], | |
| "label": "Format Check", | |
| "description": "コードフォーマットの検証", | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.claude/hooks/pre_git_quality_gates.py around lines 99 - 105, The comment
above CHECK_CANDIDATES mentions a fallback to "format" but the Format Check
candidate's script_names currently only lists ["format:check"], causing repos
with only a "format" script to skip checks; either remove "format" from the
comment if fallback is not intended, or add "format" to the Format Check
candidate's script_names (i.e., update the CHECK_CANDIDATES entry for the Format
Check to include both "format:check" and "format") so that projects with only a
"format" script are handled; modify the CHECK_CANDIDATES array accordingly and
keep the description/label ("Format Check") unchanged.
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 `@README.md`:
- Line 236: The README's opening paragraph claims checks are detected by parsing
package.json scripts but two table rows are file paths
(script/security-credential-scan.sh, script/code-complexity-check.sh), causing
inconsistency; update the documentation to explicitly state that checks are
detected via two mechanisms—parsing package.json scripts and checking for the
existence of specific shell-script files—and either split the table into two
labeled sections ("package.json script detection" and "file-existence
detection") or add a clarifying note above the table listing the file-based
checks (including the two paths script/security-credential-scan.sh and
script/code-complexity-check.sh) so readers understand why those rows differ
from script names.
| 4. ShellCheck (`npm run shellcheck`) | ||
| 5. Security Credential Scan (`./script/security-credential-scan.sh`) | ||
| 6. Code Complexity Check (`./script/code-complexity-check.sh`) | ||
| `package.json` の `scripts` を解析し、利用可能なチェックを自動検出して実行します。パッケージマネージャー(npm / pnpm / yarn / bun)もロックファイルから自動判定されます。 |
There was a problem hiding this comment.
Documentation inconsistency: package.json script detection vs. file-existence detection
The opening paragraph states that checks are detected by parsing package.json scripts, but the last two table rows list shell-script file paths (script/security-credential-scan.sh, script/code-complexity-check.sh) — these are clearly file-existence checks, not package.json script names. Readers following the description will not understand why those two rows look different from the others.
✏️ Suggested clarification
-`package.json` の `scripts` を解析し、利用可能なチェックを自動検出して実行します。パッケージマネージャー(npm / pnpm / yarn / bun)もロックファイルから自動判定されます。
+`package.json` の `scripts` を解析し、利用可能なチェックを自動検出して実行します。一部のチェックはスクリプトファイルの存在確認によって検出されます。パッケージマネージャー(npm / pnpm / yarn / bun)もロックファイルから自動判定されます。Or, split the table into two sections (package.json script detection / file-existence detection) to make the mechanism explicit.
Also applies to: 245-246
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@README.md` at line 236, The README's opening paragraph claims checks are
detected by parsing package.json scripts but two table rows are file paths
(script/security-credential-scan.sh, script/code-complexity-check.sh), causing
inconsistency; update the documentation to explicitly state that checks are
detected via two mechanisms—parsing package.json scripts and checking for the
existence of specific shell-script files—and either split the table into two
labeled sections ("package.json script detection" and "file-existence
detection") or add a clarifying note above the table listing the file-based
checks (including the two paths script/security-credential-scan.sh and
script/code-complexity-check.sh) so readers understand why those rows differ
from script names.
Claude Code レビュー
全体評価良い点:
指摘事項1. Bun環境での
|
|
🎉 This PR is included in version 1.81.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Summary
pre_git_quality_gates.pyをpackage.jsonのscripts自動検出方式にリファクタリングTest plan
git commit入力でチェック実行を確認git push入力でチェック実行を確認package.jsonがないリポジトリではスキップを確認🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes / Improvements
Documentation