Skip to content

Commit 6a219fa

Browse files
nhortonclaude
andcommitted
feat: short-circuit post_commit_reminder when all reviews passed
Move the post-commit reminder hook from bash into a Python hook that checks whether all non-catch-all review rules matching the committed files already have .passed markers. When they do (or no applicable rules exist), emit "No re-review needed" instead of nagging the agent. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 221b7ba commit 6a219fa

7 files changed

Lines changed: 456 additions & 32 deletions

File tree

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,6 @@
11
#!/usr/bin/env bash
2-
# Post-commit reminder hook
3-
# Triggers after Bash tool uses that contain "git commit" to remind
4-
# the agent to run the review skill.
5-
2+
# Post-commit reminder hook — delegates to deepwork Python hook.
63
INPUT=$(cat)
7-
COMMAND=$(echo "$INPUT" | jq -r '.tool_input.command // empty')
8-
9-
if echo "$COMMAND" | grep -q 'git commit'; then
10-
cat << 'EOF'
11-
{"hookSpecificOutput":{"hookEventName":"PostToolUse","additionalContext":"You **MUST** use AskUserQuestion tool to offer to the user to run the `review` skill to review the changes you just committed if you have not run a review recently."}}
12-
EOF
13-
fi
4+
export DEEPWORK_HOOK_PLATFORM="claude"
5+
echo "${INPUT}" | deepwork hook post_commit_reminder
6+
exit $?

specs/deepwork/cli_plugins/PLUG-REQ-001-claude-code-plugin.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,9 @@ The Claude Code plugin is the primary distribution mechanism for DeepWork on the
5555
### PLUG-REQ-001.7: Post-Commit Review Reminder
5656

5757
1. The plugin MUST register a `PostToolUse` hook on the `Bash` tool via `plugins/claude/hooks/hooks.json`.
58-
2. When the agent runs a `git commit` command, the hook MUST prompt the agent to offer the user a review of the committed changes.
58+
2. When the agent runs a `git commit` command and at least one review rule that matches the committed files has not been marked as passed, the hook MUST prompt the agent to offer the user a review of the committed changes.
59+
3. Catch-all review rules (include patterns that are pure `*`/`**` globs) MUST be excluded from this determination.
60+
4. When every applicable review rule for the committed files is already marked as passed (or no non-catch-all rule matches), the hook MUST return a context notice stating that no re-review is needed instead of prompting.
5961

6062
### PLUG-REQ-001.8: Skill Directory Conventions
6163

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
"""PostToolUse hook: nudge the agent to run the review skill after a
2+
git commit -- but only when at least one applicable review has not
3+
already been marked as passed for the committed files."""
4+
5+
from __future__ import annotations
6+
7+
import os
8+
import subprocess
9+
import sys
10+
from pathlib import Path
11+
12+
from deepwork.hooks.wrapper import (
13+
HookInput,
14+
HookOutput,
15+
NormalizedEvent,
16+
Platform,
17+
output_hook_error,
18+
run_hook,
19+
)
20+
21+
REMINDER_CONTEXT = (
22+
"You **MUST** use AskUserQuestion tool to offer to the user to run "
23+
"the `review` skill to review the changes you just committed if you "
24+
"have not run a review recently."
25+
)
26+
27+
ALL_PASSED_CONTEXT = "No re-review needed - all reviews passed for committed files"
28+
29+
30+
def post_commit_reminder_hook(hook_input: HookInput) -> HookOutput:
31+
if hook_input.event != NormalizedEvent.AFTER_TOOL:
32+
return HookOutput()
33+
if hook_input.tool_name != "shell":
34+
return HookOutput()
35+
command = hook_input.tool_input.get("command", "") or ""
36+
if "git commit" not in command:
37+
return HookOutput()
38+
39+
cwd = hook_input.cwd or os.getcwd()
40+
project_root = Path(cwd)
41+
42+
committed = _committed_files(project_root)
43+
if committed is None:
44+
# Git call failed; fall back to the safe old behavior.
45+
return HookOutput(context=REMINDER_CONTEXT)
46+
47+
try:
48+
from deepwork.review.mcp import all_reviews_passed_for_files
49+
50+
passed = all_reviews_passed_for_files(project_root, committed)
51+
except Exception:
52+
return HookOutput(context=REMINDER_CONTEXT)
53+
54+
return HookOutput(context=ALL_PASSED_CONTEXT if passed else REMINDER_CONTEXT)
55+
56+
57+
def _committed_files(project_root: Path) -> list[str] | None:
58+
"""Return files in HEAD's commit, or ``None`` if the git command fails."""
59+
try:
60+
result = subprocess.run(
61+
["git", "show", "--no-patch", "--name-only", "--format=", "HEAD"],
62+
cwd=project_root,
63+
capture_output=True,
64+
text=True,
65+
check=True,
66+
)
67+
except (OSError, subprocess.CalledProcessError):
68+
return None
69+
return [line for line in result.stdout.splitlines() if line.strip()]
70+
71+
72+
def main() -> int:
73+
platform = Platform(os.environ.get("DEEPWORK_HOOK_PLATFORM", "claude"))
74+
return run_hook(post_commit_reminder_hook, platform)
75+
76+
77+
if __name__ == "__main__": # pragma: no cover
78+
try:
79+
sys.exit(main())
80+
except Exception as e:
81+
output_hook_error(e, context="post_commit_reminder hook")
82+
sys.exit(0)

src/deepwork/review/mcp.py

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,11 @@
99
from deepwork.deepschema.review_bridge import generate_review_rules as gen_schema_rules
1010
from deepwork.review.discovery import DiscoveryError, load_all_rules
1111
from deepwork.review.formatter import format_for_claude
12-
from deepwork.review.instructions import INSTRUCTIONS_DIR, write_instruction_files
12+
from deepwork.review.instructions import (
13+
INSTRUCTIONS_DIR,
14+
compute_review_id,
15+
write_instruction_files,
16+
)
1317
from deepwork.review.matcher import (
1418
GitDiffError,
1519
format_source_location,
@@ -216,3 +220,32 @@ def mark_passed(project_root: Path, review_id: str) -> str:
216220
passed_file.write_bytes(b"")
217221

218222
return f"Review '{review_id}' marked as passed."
223+
224+
225+
def all_reviews_passed_for_files(
226+
project_root: Path,
227+
files: list[str],
228+
) -> bool:
229+
"""Return True iff every non-catch-all review rule that matches ``files``
230+
has a ``.passed`` marker for its computed review_id.
231+
232+
Vacuously True when no rules match (or ``files`` is empty).
233+
234+
Catch-all rules (include patterns that are pure ``*``/``**`` globs) are
235+
excluded — matches the scoping used by ``get_configured_reviews`` and
236+
``run_review`` when files are specified explicitly.
237+
"""
238+
rules, _ = load_all_rules(project_root)
239+
schema_rules, _ = gen_schema_rules(project_root)
240+
rules.extend(schema_rules)
241+
242+
rules = [r for r in rules if not _rule_is_catch_all(r)]
243+
244+
tasks = match_files_to_rules(files, rules, project_root)
245+
246+
instructions_dir = project_root / INSTRUCTIONS_DIR
247+
for task in tasks:
248+
review_id = compute_review_id(task, project_root)
249+
if not (instructions_dir / f"{review_id}.passed").exists():
250+
return False
251+
return True

tests/unit/plugins/test_claude_plugin.py

Lines changed: 7 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -323,25 +323,13 @@ def test_registers_post_tool_use_on_bash(self) -> None:
323323
def test_hook_script_detects_git_commit(self) -> None:
324324
# THIS TEST VALIDATES A HARD REQUIREMENT (PLUG-REQ-001.7.2).
325325
# YOU MUST NOT MODIFY THIS TEST UNLESS THE REQUIREMENT CHANGES
326-
"""PLUG-REQ-001.7.2: hook script detects git commit and prompts review."""
327-
data = json.loads(self.hooks_json_path.read_text())
328-
hooks = data["hooks"]["PostToolUse"]
329-
bash_hook = next(h for h in hooks if h.get("matcher") == "Bash")
330-
331-
# Get the hook command path
332-
hook_commands = bash_hook.get("hooks", [])
333-
assert len(hook_commands) >= 1
334-
335-
# Read the actual hook script
336-
command = hook_commands[0]["command"]
337-
# The command uses ${CLAUDE_PLUGIN_ROOT} — resolve relative to plugin dir
338-
script_name = command.split("/")[-1]
339-
script_path = PLUGIN_DIR / "hooks" / script_name
340-
assert script_path.exists(), f"Hook script not found: {script_path}"
341-
342-
content = script_path.read_text(encoding="utf-8")
343-
assert "git commit" in content, "Hook script must detect git commit commands"
344-
assert "review" in content.lower(), "Hook script must prompt for review"
326+
"""PLUG-REQ-001.7.2: Python hook detects git commit and prompts review."""
327+
from deepwork.hooks.post_commit_reminder import post_commit_reminder_hook
328+
329+
hook_py = Path(post_commit_reminder_hook.__code__.co_filename)
330+
content = hook_py.read_text(encoding="utf-8")
331+
assert "git commit" in content, "Python hook must detect git commit commands"
332+
assert "review" in content.lower(), "Python hook must reference review"
345333

346334

347335
# ---------------------------------------------------------------------------

tests/unit/review/test_mcp.py

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
from deepwork.review.config import ReviewRule, ReviewTask
1515
from deepwork.review.mcp import (
1616
ReviewToolError,
17+
all_reviews_passed_for_files,
1718
get_configured_reviews,
1819
mark_passed,
1920
run_review,
@@ -757,3 +758,114 @@ def test_get_configured_reviews_ignores_passed_markers(
757758
result = get_configured_reviews(tmp_path)
758759
assert len(result) == 1
759760
assert result[0]["name"] == "test_rule"
761+
762+
763+
@pytest.mark.usefixtures("without_standard_schemas")
764+
class TestAllReviewsPassedForFiles:
765+
"""Tests for all_reviews_passed_for_files — validates PLUG-REQ-001.7."""
766+
767+
@patch("deepwork.review.mcp.gen_schema_rules", return_value=([], []))
768+
@patch("deepwork.review.mcp.load_all_rules", return_value=([], []))
769+
def test_empty_file_list(self, mock_load: Any, mock_schema: Any, tmp_path: Path) -> None:
770+
assert all_reviews_passed_for_files(tmp_path, []) is True
771+
772+
@patch("deepwork.review.mcp.gen_schema_rules", return_value=([], []))
773+
@patch("deepwork.review.mcp.load_all_rules", return_value=([], []))
774+
def test_no_rules(self, mock_load: Any, mock_schema: Any, tmp_path: Path) -> None:
775+
assert all_reviews_passed_for_files(tmp_path, ["src/app.py"]) is True
776+
777+
@patch("deepwork.review.mcp.gen_schema_rules", return_value=([], []))
778+
@patch("deepwork.review.mcp.load_all_rules")
779+
def test_only_catch_all_rule(self, mock_load: Any, mock_schema: Any, tmp_path: Path) -> None:
780+
"""Catch-all rules are excluded, so only having one means True."""
781+
mock_load.return_value = ([_make_catch_all_rule(tmp_path)], [])
782+
assert all_reviews_passed_for_files(tmp_path, ["src/app.py"]) is True
783+
784+
@patch("deepwork.review.mcp.gen_schema_rules", return_value=([], []))
785+
@patch("deepwork.review.mcp.load_all_rules")
786+
def test_non_matching_rule(self, mock_load: Any, mock_schema: Any, tmp_path: Path) -> None:
787+
"""A non-catch-all rule that doesn't match the files returns True."""
788+
py_rule = _make_rule(tmp_path) # **/*.py
789+
mock_load.return_value = ([py_rule], [])
790+
assert all_reviews_passed_for_files(tmp_path, ["src/app.ts"]) is True
791+
792+
@patch("deepwork.review.mcp.gen_schema_rules", return_value=([], []))
793+
@patch("deepwork.review.mcp.load_all_rules")
794+
def test_matching_rule_with_passed_marker(
795+
self, mock_load: Any, mock_schema: Any, tmp_path: Path
796+
) -> None:
797+
from deepwork.review.instructions import INSTRUCTIONS_DIR, compute_review_id
798+
799+
rule = _make_rule(tmp_path)
800+
mock_load.return_value = ([rule], [])
801+
802+
# Create the file so compute_review_id produces a stable hash
803+
(tmp_path / "src").mkdir(parents=True, exist_ok=True)
804+
(tmp_path / "src" / "app.py").write_text("print('hello')")
805+
806+
# First call to get the tasks and compute their IDs
807+
from deepwork.review.matcher import match_files_to_rules
808+
809+
tasks = match_files_to_rules(["src/app.py"], [rule], tmp_path)
810+
assert len(tasks) >= 1
811+
review_id = compute_review_id(tasks[0], tmp_path)
812+
813+
# Create the .passed marker
814+
instructions_dir = tmp_path / INSTRUCTIONS_DIR
815+
instructions_dir.mkdir(parents=True, exist_ok=True)
816+
(instructions_dir / f"{review_id}.passed").write_bytes(b"")
817+
818+
assert all_reviews_passed_for_files(tmp_path, ["src/app.py"]) is True
819+
820+
@patch("deepwork.review.mcp.gen_schema_rules", return_value=([], []))
821+
@patch("deepwork.review.mcp.load_all_rules")
822+
def test_matching_rule_without_passed_marker(
823+
self, mock_load: Any, mock_schema: Any, tmp_path: Path
824+
) -> None:
825+
rule = _make_rule(tmp_path)
826+
mock_load.return_value = ([rule], [])
827+
828+
(tmp_path / "src").mkdir(parents=True, exist_ok=True)
829+
(tmp_path / "src" / "app.py").write_text("print('hello')")
830+
831+
assert all_reviews_passed_for_files(tmp_path, ["src/app.py"]) is False
832+
833+
@patch("deepwork.review.mcp.gen_schema_rules", return_value=([], []))
834+
@patch("deepwork.review.mcp.load_all_rules")
835+
def test_mixed_passed_and_not_passed(
836+
self, mock_load: Any, mock_schema: Any, tmp_path: Path
837+
) -> None:
838+
from deepwork.review.instructions import INSTRUCTIONS_DIR, compute_review_id
839+
from deepwork.review.matcher import match_files_to_rules
840+
841+
rule_py = _make_rule(tmp_path) # **/*.py
842+
rule_ts = ReviewRule(
843+
name="ts_rule",
844+
description="TS rule.",
845+
include_patterns=["**/*.ts"],
846+
exclude_patterns=[],
847+
strategy="individual",
848+
instructions="Review TS.",
849+
agent=None,
850+
all_changed_filenames=False,
851+
unchanged_matching_files=False,
852+
precomputed_info_bash_command=None,
853+
source_dir=tmp_path,
854+
source_file=tmp_path / ".deepreview",
855+
source_line=5,
856+
)
857+
mock_load.return_value = ([rule_py, rule_ts], [])
858+
859+
(tmp_path / "src").mkdir(parents=True, exist_ok=True)
860+
(tmp_path / "src" / "app.py").write_text("py")
861+
(tmp_path / "src" / "app.ts").write_text("ts")
862+
863+
# Mark only the py task as passed
864+
py_tasks = match_files_to_rules(["src/app.py"], [rule_py], tmp_path)
865+
review_id = compute_review_id(py_tasks[0], tmp_path)
866+
instructions_dir = tmp_path / INSTRUCTIONS_DIR
867+
instructions_dir.mkdir(parents=True, exist_ok=True)
868+
(instructions_dir / f"{review_id}.passed").write_bytes(b"")
869+
870+
# ts task is not passed → overall False
871+
assert all_reviews_passed_for_files(tmp_path, ["src/app.py", "src/app.ts"]) is False

0 commit comments

Comments
 (0)