feat: Harness Engineering Hooks を追加(自動リント・設定保護・完了前テスト) - #602
Conversation
3つの新しい Claude Code Hooks を追加: 1. post_edit_auto_lint.py (PostToolUse) - ファイル編集後に自動フォーマット + リントを実行 - 残った違反を additionalContext で返しエージェントの自己修正を促す - 対応: TS/JS (biome/oxlint), Python (ruff), Shell (shellcheck) 2. block_config_edit.py (PreToolUse) - リンター/フォーマッター設定ファイルの編集をブロック - エージェントが設定を緩和してテストをパスさせることを防止 - 保護対象: eslint, biome, prettier, tsconfig, ruff 等 3. stop_test_verification.py (Stop) - エージェント完了前にテストスイートを自動実行 - 失敗時はフィードバックを返し修正を促す - 無限ループ防止 (STOP_HOOK_ACTIVE), 変更なし時スキップ settings.json と claude-settings.json の両方に登録済み。 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR introduces three new Claude Code hooks that enhance development workflows: a pre-tool-use hook preventing edits to linter/formatter config files, a post-tool-use hook that auto-formats and lints code, and a stop hook that runs tests before agent completion. Configuration and documentation files are updated to register and document these hooks. Changes
Sequence DiagramsequenceDiagram
participant User/Claude
participant PreToolUse as PreToolUse<br/>block_config_edit
participant Tool as Tool Execution<br/>(Write/Edit/MultiEdit)
participant PostToolUse as PostToolUse<br/>post_edit_auto_lint
participant Stop as Stop Hook<br/>stop_test_verification
User/Claude->>PreToolUse: Initiates tool use
PreToolUse->>PreToolUse: Read file_path from stdin
alt Protected config file?
PreToolUse->>User/Claude: Block edit (exit 2)
else Safe file
PreToolUse->>Tool: Allow tool execution (exit 0)
Tool->>Tool: Apply edits to file
Tool->>PostToolUse: Edits complete
PostToolUse->>PostToolUse: Detect language<br/>(TS/JS, Python, Shell)
PostToolUse->>PostToolUse: Phase 1: Run auto-fix tools<br/>(Biome, ruff, shellcheck)
PostToolUse->>PostToolUse: Phase 2: Run linters<br/>Capture diagnostics
PostToolUse->>PostToolUse: Post-process output<br/>Suppress "no issues" messages
PostToolUse->>User/Claude: Emit diagnostics if any<br/>(exit 0)
end
User/Claude->>Stop: Session stops
Stop->>Stop: Check for git changes
alt Git changes exist?
Stop->>Stop: Locate package.json<br/>Detect test script & package manager
alt Test script found?
Stop->>Stop: Run tests (5 min timeout)<br/>with CI=true
alt Tests pass?
Stop->>User/Claude: Exit silently (exit 0)
else Tests fail
Stop->>User/Claude: Capture last 30 lines<br/>Output failure context (exit 0)
end
else No test script
Stop->>User/Claude: Exit silently (exit 0)
end
else No changes
Stop->>User/Claude: Exit silently (exit 0)
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 docstrings
🧪 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
|
🔍 AI Code Review (Local Hook)Models: Codex (default) / Gemini (default) 🤖 Codex Review以下は、マージベース 指摘事項:
patch is incorrect
|
Claude Code レビューPR の目的(エージェントが設定を緩和したりテスト未検証で完了したりするのを防ぐ)は明確で価値があります。全体的な設計は良好ですが、いくつか指摘事項があります。 🔴 要対応1.
|
| 優先度 | 件数 |
|---|---|
| 🔴 要対応 | 2件 |
| 🟡 軽微 | 4件 |
| ✅ 良好 | 多数 |
Import 位置の修正(#1)と npx --yes の削除(#2)は最低限対応を推奨します。他はオプションですが、堅牢性が上がります。
🤖 Generated with Claude Code
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 44bbbc1f85
ℹ️ 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".
| # 問題がない場合はスキップ(oxlint の "Found 0 warnings and 0 errors" 等) | ||
| if diagnostics: | ||
| lower = diagnostics.lower() | ||
| if "0 warnings and 0 errors" in lower or "found 0 " in lower: |
There was a problem hiding this comment.
Do not drop lint errors when warning count is zero
The success filter "found 0 " is too broad and clears diagnostics even when errors remain (for example, summaries like Found 0 warnings and N errors). In that case additionalContext is never emitted, so the auto-fix loop silently misses real lint failures and can allow completion with unresolved issues.
Useful? React with 👍 / 👎.
| [PM, "run", test_script], | ||
| capture_output=True, text=True, timeout=300, cwd=repo_root, | ||
| env={**os.environ, "CI": "true", "STOP_HOOK_ACTIVE": "1"}, | ||
| ) | ||
| except subprocess.TimeoutExpired: |
There was a problem hiding this comment.
Catch missing package manager executable in Stop hook
This block only handles TimeoutExpired, but subprocess.run([PM, "run", ...]) can also raise FileNotFoundError when the lockfile-selected package manager (pnpm/yarn/bun) is not installed in the current environment. That causes the hook to crash instead of gracefully skipping or reporting, which can interrupt the completion flow.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
🧹 Nitpick comments (4)
.claude/hooks/block_config_edit.py (1)
73-76: Consider moving import to the top of the file.The
from pathlib import PurePosixPathimport on line 74 is placed after the early-exit logic, which is unconventional. While this works and may be intentional for micro-optimization (avoiding import if path is empty), it violates PEP 8 style conventions.♻️ Move import to top
import sys import json +from pathlib import PurePosixPath data = json.load(sys.stdin)Then remove line 74:
# ── パス判定 ─────────────────────────────────────────────── -from pathlib import PurePosixPath - basename = PurePosixPath(file_path).name🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/hooks/block_config_edit.py around lines 73 - 76, Move the inline import into the module top-level: remove the in-function/in-block "from pathlib import PurePosixPath" and add that import alongside other imports at the top of the file, then keep the existing usage of PurePosixPath(file_path).name (the basename variable) unchanged; this restores PEP8 style and avoids an inline import while preserving the logic in the basename computation that uses PurePosixPath and file_path..claude/hooks/post_edit_auto_lint.py (1)
96-102: Simplify the "no issues" detection logic.The conditions for clearing diagnostics can be combined. Also, the comment on line 100 starts with
# ruff:which may confuse static analysis tools expecting a directive format.♻️ Consolidated condition
# 問題がない場合はスキップ(oxlint の "Found 0 warnings and 0 errors" 等) if diagnostics: lower = diagnostics.lower() - if "0 warnings and 0 errors" in lower or "found 0 " in lower: - diagnostics = "" - # ruff: "All checks passed!" をスキップ - if "all checks passed" in lower: + # oxlint: "0 warnings and 0 errors", ruff: "All checks passed!" をスキップ + if any(phrase in lower for phrase in [ + "0 warnings and 0 errors", + "found 0 ", + "all checks passed", + ]): diagnostics = ""🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/hooks/post_edit_auto_lint.py around lines 96 - 102, The diagnostics cleanup logic should combine the multiple substring checks into a single conditional: compute lower = diagnostics.lower() and if any of the "no issues" phrases (e.g., "0 warnings and 0 errors", "found 0 ", or "all checks passed") are present then set diagnostics = "". Update the comment above this block to remove the `# ruff:` prefix (or rewrite it as a plain explanatory comment) so linters won't treat it as a directive; keep references to the diagnostics variable and the lower local to help locate the change..claude/hooks/stop_test_verification.py (2)
82-90: Consider addingtest:cito test script candidates.Many projects define
test:cifor CI environments. Since this hook setsCI=true, runningtest:ciwhen available might be more appropriate.♻️ Add test:ci to the list
# ── テスト実行 ──────────────────────────────────────────── -TEST_SCRIPTS = ["test", "test:unit"] +TEST_SCRIPTS = ["test:ci", "test", "test:unit"] test_script = None🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/hooks/stop_test_verification.py around lines 82 - 90, Update the TEST_SCRIPTS candidate list to include "test:ci" (preferably before "test") so the hook will pick and run a CI-specific test script when present; modify the TEST_SCRIPTS constant referenced by the loop that sets test_script (and keeps the existing fallback to "test" and "test:unit") so the detection logic in the for s in TEST_SCRIPTS: / if s in scripts: block will choose "test:ci" when available.
36-43: Potential edge case: initial commit with no HEAD.
git diff --name-only HEADwill fail on a repository with no commits yet. While rare, this could cause the hook to silently skip (due to exception handling), which is acceptable behavior, but worth noting.📝 Alternative using --cached for initial commit safety
try: diff_result = subprocess.run( - ["git", "diff", "--name-only", "HEAD"], + ["git", "diff", "--name-only"], capture_output=True, text=True, timeout=10, cwd=repo_root )Note: Using
git diff --name-only(without HEAD) shows unstaged changes, whilegit diff --cached --name-onlyshows staged changes. Your current approach combining both is correct, but the HEAD reference could fail on initial commits.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/hooks/stop_test_verification.py around lines 36 - 43, The code calls subprocess.run(["git","diff","--name-only","HEAD"], ...) which will fail on a repository with no commits; before running that diff use subprocess.run(["git","rev-parse","--verify","HEAD"], capture_output=True, text=True, timeout=5, cwd=repo_root) to detect an initial commit and, if it returns non-zero, treat diff_result as empty (skip running git diff HEAD or set diff_names = ""), otherwise run the existing diff command; update the logic around diff_result and the existing staged_result handling to account for this fallback so functions using diff_result (e.g., diff_result.stdout parsing) won’t break on initial commits.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.claude/hooks/block_config_edit.py:
- Around line 73-76: Move the inline import into the module top-level: remove
the in-function/in-block "from pathlib import PurePosixPath" and add that import
alongside other imports at the top of the file, then keep the existing usage of
PurePosixPath(file_path).name (the basename variable) unchanged; this restores
PEP8 style and avoids an inline import while preserving the logic in the
basename computation that uses PurePosixPath and file_path.
In @.claude/hooks/post_edit_auto_lint.py:
- Around line 96-102: The diagnostics cleanup logic should combine the multiple
substring checks into a single conditional: compute lower = diagnostics.lower()
and if any of the "no issues" phrases (e.g., "0 warnings and 0 errors", "found 0
", or "all checks passed") are present then set diagnostics = "". Update the
comment above this block to remove the `# ruff:` prefix (or rewrite it as a
plain explanatory comment) so linters won't treat it as a directive; keep
references to the diagnostics variable and the lower local to help locate the
change.
In @.claude/hooks/stop_test_verification.py:
- Around line 82-90: Update the TEST_SCRIPTS candidate list to include "test:ci"
(preferably before "test") so the hook will pick and run a CI-specific test
script when present; modify the TEST_SCRIPTS constant referenced by the loop
that sets test_script (and keeps the existing fallback to "test" and
"test:unit") so the detection logic in the for s in TEST_SCRIPTS: / if s in
scripts: block will choose "test:ci" when available.
- Around line 36-43: The code calls
subprocess.run(["git","diff","--name-only","HEAD"], ...) which will fail on a
repository with no commits; before running that diff use
subprocess.run(["git","rev-parse","--verify","HEAD"], capture_output=True,
text=True, timeout=5, cwd=repo_root) to detect an initial commit and, if it
returns non-zero, treat diff_result as empty (skip running git diff HEAD or set
diff_names = ""), otherwise run the existing diff command; update the logic
around diff_result and the existing staged_result handling to account for this
fallback so functions using diff_result (e.g., diff_result.stdout parsing) won’t
break on initial commits.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: bd559b2a-5559-4526-b2cc-d87f6f04c412
📒 Files selected for processing (6)
.claude/hooks/block_config_edit.py.claude/hooks/post_edit_auto_lint.py.claude/hooks/stop_test_verification.py.claude/settings.json.devcontainer/claude-settings.jsonREADME.md
|
🎉 This PR is included in version 1.99.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Summary
Harness Engineering ベストプラクティス から3つの Hook パターンを導入:
1.
post_edit_auto_lint.py— PostToolUse 自動リントadditionalContextとして返し、エージェントの自己修正ループを駆動2.
block_config_edit.py— PreToolUse 設定保護3.
stop_test_verification.py— Stop テスト検証STOP_HOOK_ACTIVE)、Git 変更なし時はスキップ設定
.claude/settings.json(ローカル)と.devcontainer/claude-settings.json(DevContainer)の両方に登録済みTest plan
block_config_edit.py: 保護対象ファイル → exit 2 でブロック確認block_config_edit.py: 通常ファイル → パス確認post_edit_auto_lint.py: JS ファイル → oxlint 実行確認post_edit_auto_lint.py: 非対象ファイル → スキップ確認post_edit_auto_lint.py: 0 errors/warnings → 出力なし確認🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Documentation