diff --git a/.claude/settings.json b/.claude/settings.json index 85d162a1..23d968e8 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -99,6 +99,7 @@ "Edit(./.deepwork/**)", "Write(./.deepwork/**)", "Bash(deepwork:*)", + "Bash(make:*)", "Bash(gh pr edit:*)", "Bash(gh api:*)", "Bash(learning_agents/scripts/*:*)", diff --git a/CHANGELOG.md b/CHANGELOG.md index 48dec1fc..1555dead 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- Post-commit review reminder hook now short-circuits when all applicable (non-catch-all) review rules for the committed files are already marked as passed, emitting "No re-review needed" instead of nagging + ### Fixed ### Removed diff --git a/doc/architecture.md b/doc/architecture.md index 4ec73319..33eaadaf 100644 --- a/doc/architecture.md +++ b/doc/architecture.md @@ -67,6 +67,7 @@ deepwork/ # DeepWork tool repository │ ├── hooks/ # Hook system and cross-platform wrappers │ │ ├── wrapper.py # Cross-platform input/output normalization │ │ ├── deepschema_write.py # DeepSchema write-time validation hook +│ │ ├── post_commit_reminder.py # Post-commit review reminder hook │ │ ├── claude_hook.sh # Shell wrapper for Claude Code │ │ └── gemini_hook.sh # Shell wrapper for Gemini CLI │ ├── deepschema/ # DeepSchema subsystem diff --git a/doc/mcp_interface.md b/doc/mcp_interface.md index ea516956..615a8205 100644 --- a/doc/mcp_interface.md +++ b/doc/mcp_interface.md @@ -227,7 +227,7 @@ Array<{ ### 8. `mark_review_as_passed` -Mark a review as passed so it won't be re-run while reviewed files remain unchanged. The `review_id` is provided in the instruction file's "After Review" section. +Mark a review as passed so it won't be re-run while reviewed files remain unchanged. Call this when a review has no findings, when all findings have been fixed, or when remaining findings have been explicitly dismissed by the user. The `review_id` is provided in the instruction file's "After Review" section. This tool operates outside the workflow lifecycle — it can be called independently at any time. diff --git a/plugins/claude/hooks/post_commit_reminder.sh b/plugins/claude/hooks/post_commit_reminder.sh index a913538e..f36b2eb1 100755 --- a/plugins/claude/hooks/post_commit_reminder.sh +++ b/plugins/claude/hooks/post_commit_reminder.sh @@ -1,13 +1,6 @@ #!/usr/bin/env bash -# Post-commit reminder hook -# Triggers after Bash tool uses that contain "git commit" to remind -# the agent to run the review skill. - +# Post-commit reminder hook — delegates to deepwork Python hook. INPUT=$(cat) -COMMAND=$(echo "$INPUT" | jq -r '.tool_input.command // empty') - -if echo "$COMMAND" | grep -q 'git commit'; then - cat << 'EOF' -{"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."}} -EOF -fi +export DEEPWORK_HOOK_PLATFORM="claude" +echo "${INPUT}" | deepwork hook post_commit_reminder +exit $? diff --git a/specs/deepwork/cli_plugins/PLUG-REQ-001-claude-code-plugin.md b/specs/deepwork/cli_plugins/PLUG-REQ-001-claude-code-plugin.md index 38fbe6d2..93ffc87e 100644 --- a/specs/deepwork/cli_plugins/PLUG-REQ-001-claude-code-plugin.md +++ b/specs/deepwork/cli_plugins/PLUG-REQ-001-claude-code-plugin.md @@ -55,7 +55,9 @@ The Claude Code plugin is the primary distribution mechanism for DeepWork on the ### PLUG-REQ-001.7: Post-Commit Review Reminder 1. The plugin MUST register a `PostToolUse` hook on the `Bash` tool via `plugins/claude/hooks/hooks.json`. -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. +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. +3. Catch-all review rules (include patterns that are pure `*`/`**` globs) MUST be excluded from this determination. +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. ### PLUG-REQ-001.8: Skill Directory Conventions diff --git a/src/deepwork/hooks/README.md b/src/deepwork/hooks/README.md index ae161e0b..b8646197 100644 --- a/src/deepwork/hooks/README.md +++ b/src/deepwork/hooks/README.md @@ -139,6 +139,7 @@ pytest tests/shell_script_tests/test_hook_wrappers.py -v |------|---------| | `wrapper.py` | Cross-platform input/output normalization | | `deepschema_write.py` | DeepSchema write-time validation hook | +| `post_commit_reminder.py` | Post-commit hook that nudges the agent to run `/review` (skips if all reviews already passed) | | `claude_hook.sh` | Shell wrapper for Claude Code | | `gemini_hook.sh` | Shell wrapper for Gemini CLI | | `.deepreview` | Review rule ensuring hooks use correct output routing (DW-REQ-006.6) | diff --git a/src/deepwork/hooks/post_commit_reminder.py b/src/deepwork/hooks/post_commit_reminder.py new file mode 100644 index 00000000..ce2285c8 --- /dev/null +++ b/src/deepwork/hooks/post_commit_reminder.py @@ -0,0 +1,82 @@ +"""PostToolUse hook: nudge the agent to run the review skill after a +git commit -- but only when at least one applicable review has not +already been marked as passed for the committed files.""" + +from __future__ import annotations + +import os +import subprocess +import sys +from pathlib import Path + +from deepwork.hooks.wrapper import ( + HookInput, + HookOutput, + NormalizedEvent, + Platform, + output_hook_error, + run_hook, +) + +REMINDER_CONTEXT = ( + "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." +) + +ALL_PASSED_CONTEXT = "No re-review needed - all reviews passed for committed files" + + +def post_commit_reminder_hook(hook_input: HookInput) -> HookOutput: + if hook_input.event != NormalizedEvent.AFTER_TOOL: + return HookOutput() + if hook_input.tool_name != "shell": + return HookOutput() + command = hook_input.tool_input.get("command", "") or "" + if "git commit" not in command: + return HookOutput() + + cwd = hook_input.cwd or os.getcwd() + project_root = Path(cwd) + + committed = _committed_files(project_root) + if committed is None: + # Git call failed; fall back to the safe old behavior. + return HookOutput(context=REMINDER_CONTEXT) + + try: + from deepwork.review.mcp import all_reviews_passed_for_files + + passed = all_reviews_passed_for_files(project_root, committed) + except Exception: + return HookOutput(context=REMINDER_CONTEXT) + + return HookOutput(context=ALL_PASSED_CONTEXT if passed else REMINDER_CONTEXT) + + +def _committed_files(project_root: Path) -> list[str] | None: + """Return files in HEAD's commit, or ``None`` if the git command fails.""" + try: + result = subprocess.run( + ["git", "diff-tree", "--no-commit-id", "--name-only", "-r", "HEAD"], + cwd=project_root, + capture_output=True, + text=True, + check=True, + ) + except (OSError, subprocess.CalledProcessError): + return None + return [line for line in result.stdout.splitlines() if line.strip()] + + +def main() -> int: + platform = Platform(os.environ.get("DEEPWORK_HOOK_PLATFORM", "claude")) + return run_hook(post_commit_reminder_hook, platform) + + +if __name__ == "__main__": # pragma: no cover + try: + sys.exit(main()) + except Exception as e: + output_hook_error(e, context="post_commit_reminder hook") + sys.exit(0) diff --git a/src/deepwork/jobs/mcp/server.py b/src/deepwork/jobs/mcp/server.py index 3434794e..63fcff92 100644 --- a/src/deepwork/jobs/mcp/server.py +++ b/src/deepwork/jobs/mcp/server.py @@ -490,8 +490,10 @@ async def get_configured_reviews( @mcp.tool( description=( "Mark a review as passed so it won't be re-run while reviewed files " - "remain unchanged. The review_id is provided in the instruction file's " - '"After Review" section.' + "remain unchanged. Call this when a review has no findings, when all " + "findings have been fixed, or when remaining findings have been " + "explicitly dismissed by the user. The review_id is provided in the " + 'instruction file\'s "After Review" section.' ) ) async def mark_review_as_passed(review_id: str, ctx: Context) -> str: diff --git a/src/deepwork/review/instructions.py b/src/deepwork/review/instructions.py index 58525aa5..9dc1a5ab 100644 --- a/src/deepwork/review/instructions.py +++ b/src/deepwork/review/instructions.py @@ -302,7 +302,8 @@ def build_instruction_file( if review_id: parts.append("## After Review\n") parts.append( - "If this review passes with no findings, call the `mark_review_as_passed` tool with:\n" + "If this review passes with no findings, or if all findings have been " + "addressed or explicitly dismissed, call the `mark_review_as_passed` tool with:\n" ) parts.append(f'- `review_id`: `"{review_id}"`') parts.append("") diff --git a/src/deepwork/review/mcp.py b/src/deepwork/review/mcp.py index 8fe59b8c..5277f354 100644 --- a/src/deepwork/review/mcp.py +++ b/src/deepwork/review/mcp.py @@ -7,9 +7,14 @@ from pathlib import Path from deepwork.deepschema.review_bridge import generate_review_rules as gen_schema_rules +from deepwork.review.config import ReviewRule from deepwork.review.discovery import DiscoveryError, load_all_rules from deepwork.review.formatter import format_for_claude -from deepwork.review.instructions import INSTRUCTIONS_DIR, write_instruction_files +from deepwork.review.instructions import ( + INSTRUCTIONS_DIR, + compute_review_id, + write_instruction_files, +) from deepwork.review.matcher import ( GitDiffError, format_source_location, @@ -34,7 +39,7 @@ def _is_catch_all_pattern(pattern: str) -> bool: return pattern != "" and all(c in "*/" for c in pattern) -def _rule_is_catch_all(rule) -> bool: # type: ignore[no-untyped-def] +def _rule_is_catch_all(rule: ReviewRule) -> bool: """Return True if every include pattern on the rule is a catch-all.""" return bool(rule.include_patterns) and all( _is_catch_all_pattern(p) for p in rule.include_patterns @@ -216,3 +221,32 @@ def mark_passed(project_root: Path, review_id: str) -> str: passed_file.write_bytes(b"") return f"Review '{review_id}' marked as passed." + + +def all_reviews_passed_for_files( + project_root: Path, + files: list[str], +) -> bool: + """Return True iff every non-catch-all review rule that matches ``files`` + has a ``.passed`` marker for its computed review_id. + + Vacuously True when no rules match (or ``files`` is empty). + + Catch-all rules (include patterns that are pure ``*``/``**`` globs) are + excluded — matches the scoping used by ``get_configured_reviews`` and + ``run_review`` when files are specified explicitly. + """ + rules, _ = load_all_rules(project_root) + schema_rules, _ = gen_schema_rules(project_root) + rules.extend(schema_rules) + + rules = [r for r in rules if not _rule_is_catch_all(r)] + + tasks = match_files_to_rules(files, rules, project_root) + + instructions_dir = project_root / INSTRUCTIONS_DIR + for task in tasks: + review_id = compute_review_id(task, project_root) + if not (instructions_dir / f"{review_id}.passed").exists(): + return False + return True diff --git a/tests/unit/plugins/test_claude_plugin.py b/tests/unit/plugins/test_claude_plugin.py index 42cafc4f..bc4e08d5 100644 --- a/tests/unit/plugins/test_claude_plugin.py +++ b/tests/unit/plugins/test_claude_plugin.py @@ -323,25 +323,13 @@ def test_registers_post_tool_use_on_bash(self) -> None: def test_hook_script_detects_git_commit(self) -> None: # THIS TEST VALIDATES A HARD REQUIREMENT (PLUG-REQ-001.7.2). # YOU MUST NOT MODIFY THIS TEST UNLESS THE REQUIREMENT CHANGES - """PLUG-REQ-001.7.2: hook script detects git commit and prompts review.""" - data = json.loads(self.hooks_json_path.read_text()) - hooks = data["hooks"]["PostToolUse"] - bash_hook = next(h for h in hooks if h.get("matcher") == "Bash") - - # Get the hook command path - hook_commands = bash_hook.get("hooks", []) - assert len(hook_commands) >= 1 - - # Read the actual hook script - command = hook_commands[0]["command"] - # The command uses ${CLAUDE_PLUGIN_ROOT} — resolve relative to plugin dir - script_name = command.split("/")[-1] - script_path = PLUGIN_DIR / "hooks" / script_name - assert script_path.exists(), f"Hook script not found: {script_path}" - - content = script_path.read_text(encoding="utf-8") - assert "git commit" in content, "Hook script must detect git commit commands" - assert "review" in content.lower(), "Hook script must prompt for review" + """PLUG-REQ-001.7.2: Python hook detects git commit and prompts review.""" + from deepwork.hooks.post_commit_reminder import post_commit_reminder_hook + + hook_py = Path(post_commit_reminder_hook.__code__.co_filename) + content = hook_py.read_text(encoding="utf-8") + assert "git commit" in content, "Python hook must detect git commit commands" + assert "review" in content.lower(), "Python hook must reference review" # --------------------------------------------------------------------------- diff --git a/tests/unit/review/test_mcp.py b/tests/unit/review/test_mcp.py index ef14fdbf..a863e071 100644 --- a/tests/unit/review/test_mcp.py +++ b/tests/unit/review/test_mcp.py @@ -14,6 +14,7 @@ from deepwork.review.config import ReviewRule, ReviewTask from deepwork.review.mcp import ( ReviewToolError, + all_reviews_passed_for_files, get_configured_reviews, mark_passed, run_review, @@ -197,13 +198,13 @@ def _get_tool_names(server: Any) -> set[str]: class TestRunReviewDiscoveryWarnings: """Tests for discovery warning handling in run_review.""" + # THIS TEST VALIDATES A HARD REQUIREMENT (REVIEW-REQ-002.3.4). + # YOU MUST NOT MODIFY THIS TEST UNLESS THE REQUIREMENT CHANGES @patch("deepwork.review.mcp.gen_schema_rules") @patch("deepwork.review.mcp.load_all_rules") def test_no_valid_rules_with_parse_errors_shows_warnings( self, mock_load: Any, mock_schema: Any, tmp_path: Path ) -> None: - # THIS TEST VALIDATES A HARD REQUIREMENT (REVIEW-REQ-002.3.4). - # YOU MUST NOT MODIFY THIS TEST UNLESS THE REQUIREMENT CHANGES """When no valid rules exist but there are discovery errors, shows parse errors.""" from deepwork.review.discovery import DiscoveryError @@ -218,13 +219,13 @@ def test_no_valid_rules_with_parse_errors_shows_warnings( assert "bad/.deepreview" in result assert "bad yaml" in result + # THIS TEST VALIDATES A HARD REQUIREMENT (REVIEW-REQ-002.3.4). + # YOU MUST NOT MODIFY THIS TEST UNLESS THE REQUIREMENT CHANGES @patch("deepwork.review.mcp.gen_schema_rules") @patch("deepwork.review.mcp.load_all_rules") def test_schema_errors_appended_to_discovery_errors( self, mock_load: Any, mock_schema: Any, tmp_path: Path ) -> None: - # THIS TEST VALIDATES A HARD REQUIREMENT (REVIEW-REQ-002.3.4). - # YOU MUST NOT MODIFY THIS TEST UNLESS THE REQUIREMENT CHANGES """DeepSchema errors are appended to discovery_errors.""" mock_load.return_value = ([], []) @@ -235,6 +236,8 @@ def test_schema_errors_appended_to_discovery_errors( assert "No valid review rules found" in result assert "schema parse failed" in result + # THIS TEST VALIDATES A HARD REQUIREMENT (REVIEW-REQ-002.3.4). + # YOU MUST NOT MODIFY THIS TEST UNLESS THE REQUIREMENT CHANGES @patch("deepwork.review.mcp.write_instruction_files") @patch("deepwork.review.mcp.match_files_to_rules") @patch("deepwork.review.mcp.get_changed_files") @@ -249,8 +252,6 @@ def test_discovery_warnings_prepended_to_output( mock_write: Any, tmp_path: Path, ) -> None: - # THIS TEST VALIDATES A HARD REQUIREMENT (REVIEW-REQ-002.3.4). - # YOU MUST NOT MODIFY THIS TEST UNLESS THE REQUIREMENT CHANGES """When discovery errors exist alongside valid rules, warnings are prepended.""" from deepwork.review.discovery import DiscoveryError @@ -757,3 +758,120 @@ def test_get_configured_reviews_ignores_passed_markers( result = get_configured_reviews(tmp_path) assert len(result) == 1 assert result[0]["name"] == "test_rule" + + +@pytest.mark.usefixtures("without_standard_schemas") +class TestAllReviewsPassedForFiles: + """Tests for all_reviews_passed_for_files — validates PLUG-REQ-001.7.""" + + @patch("deepwork.review.mcp.gen_schema_rules", return_value=([], [])) + @patch("deepwork.review.mcp.load_all_rules", return_value=([], [])) + def test_empty_file_list(self, mock_load: Any, mock_schema: Any, tmp_path: Path) -> None: + assert all_reviews_passed_for_files(tmp_path, []) is True + + @patch("deepwork.review.mcp.gen_schema_rules", return_value=([], [])) + @patch("deepwork.review.mcp.load_all_rules", return_value=([], [])) + def test_no_rules(self, mock_load: Any, mock_schema: Any, tmp_path: Path) -> None: + assert all_reviews_passed_for_files(tmp_path, ["src/app.py"]) is True + + # THIS TEST VALIDATES A HARD REQUIREMENT (PLUG-REQ-001.7.3). + # YOU MUST NOT MODIFY THIS TEST UNLESS THE REQUIREMENT CHANGES + @patch("deepwork.review.mcp.gen_schema_rules", return_value=([], [])) + @patch("deepwork.review.mcp.load_all_rules") + def test_only_catch_all_rule(self, mock_load: Any, mock_schema: Any, tmp_path: Path) -> None: + """Catch-all rules are excluded, so only having one means True.""" + mock_load.return_value = ([_make_catch_all_rule(tmp_path)], []) + assert all_reviews_passed_for_files(tmp_path, ["src/app.py"]) is True + + @patch("deepwork.review.mcp.gen_schema_rules", return_value=([], [])) + @patch("deepwork.review.mcp.load_all_rules") + def test_non_matching_rule(self, mock_load: Any, mock_schema: Any, tmp_path: Path) -> None: + """A non-catch-all rule that doesn't match the files returns True.""" + py_rule = _make_rule(tmp_path) # **/*.py + mock_load.return_value = ([py_rule], []) + assert all_reviews_passed_for_files(tmp_path, ["src/app.ts"]) is True + + # THIS TEST VALIDATES A HARD REQUIREMENT (PLUG-REQ-001.7.4). + # YOU MUST NOT MODIFY THIS TEST UNLESS THE REQUIREMENT CHANGES + @patch("deepwork.review.mcp.gen_schema_rules", return_value=([], [])) + @patch("deepwork.review.mcp.load_all_rules") + def test_matching_rule_with_passed_marker( + self, mock_load: Any, mock_schema: Any, tmp_path: Path + ) -> None: + from deepwork.review.instructions import INSTRUCTIONS_DIR, compute_review_id + + rule = _make_rule(tmp_path) + mock_load.return_value = ([rule], []) + + # Create the file so compute_review_id produces a stable hash + (tmp_path / "src").mkdir(parents=True, exist_ok=True) + (tmp_path / "src" / "app.py").write_text("print('hello')") + + # First call to get the tasks and compute their IDs + from deepwork.review.matcher import match_files_to_rules + + tasks = match_files_to_rules(["src/app.py"], [rule], tmp_path) + assert len(tasks) >= 1 + review_id = compute_review_id(tasks[0], tmp_path) + + # Create the .passed marker + instructions_dir = tmp_path / INSTRUCTIONS_DIR + instructions_dir.mkdir(parents=True, exist_ok=True) + (instructions_dir / f"{review_id}.passed").write_bytes(b"") + + assert all_reviews_passed_for_files(tmp_path, ["src/app.py"]) is True + + # THIS TEST VALIDATES A HARD REQUIREMENT (PLUG-REQ-001.7.2). + # YOU MUST NOT MODIFY THIS TEST UNLESS THE REQUIREMENT CHANGES + @patch("deepwork.review.mcp.gen_schema_rules", return_value=([], [])) + @patch("deepwork.review.mcp.load_all_rules") + def test_matching_rule_without_passed_marker( + self, mock_load: Any, mock_schema: Any, tmp_path: Path + ) -> None: + rule = _make_rule(tmp_path) + mock_load.return_value = ([rule], []) + + (tmp_path / "src").mkdir(parents=True, exist_ok=True) + (tmp_path / "src" / "app.py").write_text("print('hello')") + + assert all_reviews_passed_for_files(tmp_path, ["src/app.py"]) is False + + @patch("deepwork.review.mcp.gen_schema_rules", return_value=([], [])) + @patch("deepwork.review.mcp.load_all_rules") + def test_mixed_passed_and_not_passed( + self, mock_load: Any, mock_schema: Any, tmp_path: Path + ) -> None: + from deepwork.review.instructions import INSTRUCTIONS_DIR, compute_review_id + from deepwork.review.matcher import match_files_to_rules + + rule_py = _make_rule(tmp_path) # **/*.py + rule_ts = ReviewRule( + name="ts_rule", + description="TS rule.", + include_patterns=["**/*.ts"], + exclude_patterns=[], + strategy="individual", + instructions="Review TS.", + agent=None, + all_changed_filenames=False, + unchanged_matching_files=False, + precomputed_info_bash_command=None, + source_dir=tmp_path, + source_file=tmp_path / ".deepreview", + source_line=5, + ) + mock_load.return_value = ([rule_py, rule_ts], []) + + (tmp_path / "src").mkdir(parents=True, exist_ok=True) + (tmp_path / "src" / "app.py").write_text("py") + (tmp_path / "src" / "app.ts").write_text("ts") + + # Mark only the py task as passed + py_tasks = match_files_to_rules(["src/app.py"], [rule_py], tmp_path) + review_id = compute_review_id(py_tasks[0], tmp_path) + instructions_dir = tmp_path / INSTRUCTIONS_DIR + instructions_dir.mkdir(parents=True, exist_ok=True) + (instructions_dir / f"{review_id}.passed").write_bytes(b"") + + # ts task is not passed → overall False + assert all_reviews_passed_for_files(tmp_path, ["src/app.py", "src/app.ts"]) is False diff --git a/tests/unit/test_post_commit_reminder_hook.py b/tests/unit/test_post_commit_reminder_hook.py new file mode 100644 index 00000000..2549270d --- /dev/null +++ b/tests/unit/test_post_commit_reminder_hook.py @@ -0,0 +1,226 @@ +"""Tests for the post_commit_reminder hook. + +Validates PLUG-REQ-001.7.2, PLUG-REQ-001.7.3, PLUG-REQ-001.7.4. +""" + +from __future__ import annotations + +import json +import subprocess +import sys +from pathlib import Path +from unittest.mock import patch + +from deepwork.hooks.post_commit_reminder import ( + ALL_PASSED_CONTEXT, + REMINDER_CONTEXT, + _committed_files, + main, + post_commit_reminder_hook, +) +from deepwork.hooks.wrapper import HookInput, HookOutput, NormalizedEvent, Platform + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + + +def _make_hook_input( + event: NormalizedEvent = NormalizedEvent.AFTER_TOOL, + tool_name: str = "shell", + command: str = "git commit -m 'test'", + cwd: str = "/project", +) -> HookInput: + return HookInput( + platform=Platform.CLAUDE, + event=event, + session_id="sess1", + transcript_path="", + cwd=cwd, + tool_name=tool_name, + tool_input={"command": command}, + prompt="", + raw_input={}, + ) + + +# --------------------------------------------------------------------------- +# Early returns +# --------------------------------------------------------------------------- + + +class TestPostCommitReminderEarlyReturns: + def test_ignores_non_after_tool_event(self) -> None: + inp = _make_hook_input(event=NormalizedEvent.BEFORE_TOOL) + assert post_commit_reminder_hook(inp) == HookOutput() + + def test_ignores_non_shell_tool(self) -> None: + inp = _make_hook_input(tool_name="write_file") + assert post_commit_reminder_hook(inp) == HookOutput() + + def test_ignores_non_git_commit_command(self) -> None: + inp = _make_hook_input(command="git status") + assert post_commit_reminder_hook(inp) == HookOutput() + + +# --------------------------------------------------------------------------- +# Review-aware paths +# --------------------------------------------------------------------------- + + +class TestPostCommitReminderReviewPaths: + # THIS TEST VALIDATES A HARD REQUIREMENT (PLUG-REQ-001.7.4). + # YOU MUST NOT MODIFY THIS TEST UNLESS THE REQUIREMENT CHANGES + @patch( + "deepwork.review.mcp.all_reviews_passed_for_files", + return_value=True, + ) + @patch( + "deepwork.hooks.post_commit_reminder._committed_files", + return_value=["src/app.py"], + ) + def test_all_passed_returns_all_passed_context( + self, mock_files: object, mock_passed: object + ) -> None: + result = post_commit_reminder_hook(_make_hook_input()) + assert result == HookOutput(context=ALL_PASSED_CONTEXT) + + # THIS TEST VALIDATES A HARD REQUIREMENT (PLUG-REQ-001.7.4). + # YOU MUST NOT MODIFY THIS TEST UNLESS THE REQUIREMENT CHANGES + @patch( + "deepwork.review.mcp.all_reviews_passed_for_files", + return_value=True, + ) + @patch( + "deepwork.hooks.post_commit_reminder._committed_files", + return_value=["README.md"], + ) + def test_no_applicable_reviews_returns_all_passed_context( + self, mock_files: object, mock_passed: object + ) -> None: + """No non-catch-all rules match → vacuously True → ALL_PASSED_CONTEXT.""" + result = post_commit_reminder_hook(_make_hook_input()) + assert result == HookOutput(context=ALL_PASSED_CONTEXT) + + # THIS TEST VALIDATES A HARD REQUIREMENT (PLUG-REQ-001.7.2). + # YOU MUST NOT MODIFY THIS TEST UNLESS THE REQUIREMENT CHANGES + @patch( + "deepwork.review.mcp.all_reviews_passed_for_files", + return_value=False, + ) + @patch( + "deepwork.hooks.post_commit_reminder._committed_files", + return_value=["src/app.py"], + ) + def test_not_all_passed_returns_reminder(self, mock_files: object, mock_passed: object) -> None: + result = post_commit_reminder_hook(_make_hook_input()) + assert result == HookOutput(context=REMINDER_CONTEXT) + + # THIS TEST VALIDATES A HARD REQUIREMENT (PLUG-REQ-001.7.2). + # YOU MUST NOT MODIFY THIS TEST UNLESS THE REQUIREMENT CHANGES + @patch( + "deepwork.hooks.post_commit_reminder._committed_files", + return_value=None, + ) + def test_git_failure_falls_back_to_reminder(self, mock_files: object) -> None: + result = post_commit_reminder_hook(_make_hook_input()) + assert result == HookOutput(context=REMINDER_CONTEXT) + + # THIS TEST VALIDATES A HARD REQUIREMENT (PLUG-REQ-001.7.2). + # YOU MUST NOT MODIFY THIS TEST UNLESS THE REQUIREMENT CHANGES + @patch( + "deepwork.review.mcp.all_reviews_passed_for_files", + side_effect=RuntimeError("boom"), + ) + @patch( + "deepwork.hooks.post_commit_reminder._committed_files", + return_value=["src/app.py"], + ) + def test_exception_in_passed_check_falls_back_to_reminder( + self, mock_files: object, mock_passed: object + ) -> None: + result = post_commit_reminder_hook(_make_hook_input()) + assert result == HookOutput(context=REMINDER_CONTEXT) + + +# --------------------------------------------------------------------------- +# _committed_files +# --------------------------------------------------------------------------- + + +class TestCommittedFiles: + def test_returns_file_list_on_success(self, tmp_path: Path) -> None: + with patch("deepwork.hooks.post_commit_reminder.subprocess.run") as mock_run: + mock_run.return_value = subprocess.CompletedProcess( + args=[], returncode=0, stdout="src/a.py\nsrc/b.py\n" + ) + result = _committed_files(tmp_path) + assert result == ["src/a.py", "src/b.py"] + + def test_returns_none_on_called_process_error(self, tmp_path: Path) -> None: + with patch( + "deepwork.hooks.post_commit_reminder.subprocess.run", + side_effect=subprocess.CalledProcessError(1, "git"), + ): + assert _committed_files(tmp_path) is None + + def test_returns_none_on_os_error(self, tmp_path: Path) -> None: + with patch( + "deepwork.hooks.post_commit_reminder.subprocess.run", + side_effect=OSError("no git"), + ): + assert _committed_files(tmp_path) is None + + def test_uses_diff_tree_not_show(self, tmp_path: Path) -> None: + """git show --no-patch --name-only are incompatible flags; verify we use diff-tree.""" + with patch("deepwork.hooks.post_commit_reminder.subprocess.run") as mock_run: + mock_run.return_value = subprocess.CompletedProcess( + args=[], returncode=0, stdout="a.py\n" + ) + _committed_files(tmp_path) + cmd = mock_run.call_args[0][0] + assert cmd[0] == "git" + assert cmd[1] == "diff-tree" + assert "--no-patch" not in cmd, "--no-patch conflicts with --name-only" + + def test_strips_blank_lines(self, tmp_path: Path) -> None: + with patch("deepwork.hooks.post_commit_reminder.subprocess.run") as mock_run: + mock_run.return_value = subprocess.CompletedProcess( + args=[], returncode=0, stdout="a.py\n\n \nb.py\n" + ) + result = _committed_files(tmp_path) + assert result == ["a.py", "b.py"] + + +# --------------------------------------------------------------------------- +# main() entry point +# --------------------------------------------------------------------------- + + +class TestMain: + def test_main_delegates_to_run_hook(self) -> None: + with patch("deepwork.hooks.post_commit_reminder.run_hook", return_value=0) as mock_run: + with patch.dict("os.environ", {"DEEPWORK_HOOK_PLATFORM": "gemini"}): + ret = main() + assert ret == 0 + mock_run.assert_called_once() + assert mock_run.call_args[0][1] == Platform("gemini") + + def test_main_defaults_to_claude(self) -> None: + with patch("deepwork.hooks.post_commit_reminder.run_hook", return_value=0) as mock_run: + with patch.dict("os.environ", {}, clear=True): + main() + assert mock_run.call_args[0][1] == Platform.CLAUDE + + def test_dunder_main_runs_as_script(self) -> None: + result = subprocess.run( + [sys.executable, "-m", "deepwork.hooks.post_commit_reminder"], + capture_output=True, + text=True, + timeout=10, + input="{}", + cwd=str(Path(__file__).resolve().parents[2]), + ) + assert result.returncode == 0 + output = json.loads(result.stdout.strip()) + assert isinstance(output, dict)