fix: sync template hook fix (compound commits, visible warnings)

.claude/hooks/pre_commit_sp_check.py

@@ -13,24 +13,40 @@
 import subprocess
 
 
-def _is_git_commit_command(command: str) -> bool:
-    """Return True only when command invokes the git commit subcommand."""
-    try:
-        tokens = shlex.split(command, posix=True)
-    except ValueError:
-        return bool(re.search(r"git\s+commit", command))
+# git global options that consume the following token as their argument
+_ARG_OPTIONS = frozenset({
+    "-C", "-c", "--exec-path", "--git-dir",
+    "--work-tree", "--namespace", "--super-prefix",
+    "--list-cmds",
+})
+_OPERATOR_CHARS = frozenset("();<>|&")
+
+
+def _command_segments(command: str) -> list:
+    """Split a shell command line into simple commands at &&, ||, ;, |, & and newlines.
+
+    Only the first git invocation used to be inspected, so `git add f && git commit` looked like a plain
+    `git add` and the staged-diff check silently never ran. Newlines are turned into `;` first because shlex
+    treats them as plain whitespace; inside quotes (a multi-line commit message) they stay part of one token.
+    """
+    lexer = shlex.shlex(command.replace("\n", " ; "), posix=True, punctuation_chars=True)
+    lexer.whitespace_split = True
+    segments, current = [], []
+    for tok in lexer:
+        if tok and all(ch in _OPERATOR_CHARS for ch in tok):
+            segments.append(current)
+            current = []
+        else:
+            current.append(tok)
+    segments.append(current)
+    return [seg for seg in segments if seg]
 
+
+def _segment_is_git_commit(tokens: list) -> bool:
     try:
         git_idx = next(i for i, t in enumerate(tokens) if t == "git" or t.endswith("/git"))
     except StopIteration:
         return False
-
-    # Options that consume the following token as an argument
-    _ARG_OPTIONS = frozenset({
-        "-C", "-c", "--exec-path", "--git-dir",
-        "--work-tree", "--namespace", "--super-prefix",
-        "--list-cmds",
-    })
     i = git_idx + 1
     while i < len(tokens):
         tok = tokens[i]
@@ -43,6 +59,26 @@ def _is_git_commit_command(command: str) -> bool:
     return False
 
 
+def _is_git_commit_command(command: str) -> bool:
+    """Return True when any simple command in the line invokes the git commit subcommand."""
+    try:
+        segments = _command_segments(command)
+    except ValueError:
+        return bool(re.search(r"git\s+commit", command))
+    return any(_segment_is_git_commit(seg) for seg in segments)
+
+
+def _added_lines_by_file(diff: str) -> list:
+    """Return (path, added line) pairs from a unified diff, so checks can be limited to the right file types."""
+    pairs, path = [], ""
+    for line in diff.splitlines():
+        if line.startswith("+++ "):
+            path = line[4:].removeprefix("b/")
+        elif line.startswith("+"):
+            pairs.append((path, line))
+    return pairs
+
+
 def get_staged_diff() -> str:
     git = shutil.which("git")
     if not git:
@@ -60,7 +96,11 @@ def get_staged_diff() -> str:
 def main():
     try:
         data = json.load(sys.stdin)
-        command = data.get("command", "")
+        # A PreToolUse hook matched on Bash receives the command nested at tool_input.command, not
+        # top-level — reading the top-level key always returned "", so this check silently no-op'd
+        # on every single commit since it was written. Confirmed against Claude Code's real
+        # PreToolUse hook input schema and caught by CodeRabbit on i-machine-arr/reaparr PR #2.
+        command = data.get("tool_input", {}).get("command", "")
     except json.JSONDecodeError:
         sys.exit(0)
 
@@ -71,7 +111,8 @@ def main():
     if not diff:
         sys.exit(0)
 
-    added_lines = [line for line in diff.splitlines() if line.startswith("+") and not line.startswith("+++")]
+    added = _added_lines_by_file(diff)
+    added_lines = [line for _, line in added]  # line-only view used by the checks below
 
     warnings = []
 
@@ -80,8 +121,9 @@ def main():
     # ---------------------------------------------------------------
 
     # Broad except Exception
-    for line in added_lines:
-        if re.search(r"except\s+Exception\b", line):
+    # Python files only: docs that show the anti-pattern as an example must not trip it.
+    for path, line in added:
+        if path.endswith(".py") and re.search(r"except\s+Exception\b", line):
             warnings.append(
                 "Broad 'except Exception' detected — use specific exceptions "
                 "(e.g. OSError, AttributeError)."
@@ -100,14 +142,18 @@ def main():
     # ---------------------------------------------------------------
 
     if warnings:
-        print()
-        print(f"Coding Notes Pre-commit Check — {len(warnings)} issue(s) found:")
-        for w in warnings:
-            print(f"  * {w}")
-        print()
-        print("Review .claude/CODING_NOTES.md before proceeding. Commit is NOT blocked — "
-              "fix on next commit if intentional.")
-        print()
+        message = (
+            f"Coding Notes Pre-commit Check — {len(warnings)} issue(s) found:\n"
+            + "\n".join(f"  * {w}" for w in warnings)
+            + "\n\nReview .claude/CODING_NOTES.md before proceeding. Commit is NOT blocked — "
+              "fix on next commit if intentional."
+        )
+        # Plain stdout from a PreToolUse hook is only logged. systemMessage shows the warning to the user;
+        # additionalContext puts it in front of Claude. Neither blocks the tool call (no permissionDecision).
+        print(json.dumps({
+            "systemMessage": message,
+            "hookSpecificOutput": {"hookEventName": "PreToolUse", "additionalContext": message},
+        }))
 
     sys.exit(0)
 

.claude/hooks/test_pre_commit_sp_check.py

@@ -0,0 +1,83 @@
+"""Tests for the pre-commit hook helpers. Run from the repo root: python -m unittest discover -s .claude/hooks"""
+
+import json
+import os
+import subprocess
+import sys
+import unittest
+
+sys.path.insert(0, os.path.dirname(__file__))
+import pre_commit_sp_check as hook  # noqa: E402
+
+
+class IsGitCommit(unittest.TestCase):
+    def test_plain_commit(self):
+        self.assertTrue(hook._is_git_commit_command('git commit -m "x"'))
+
+    def test_commit_after_add(self):
+        self.assertTrue(hook._is_git_commit_command('git add a.py && git commit -m "x"'))
+        self.assertTrue(hook._is_git_commit_command("git add -A; git commit -m x"))
+        self.assertTrue(hook._is_git_commit_command("git add a || git commit -m x"))
+        self.assertTrue(hook._is_git_commit_command("git add a\ngit commit -m x"))
+
+    def test_global_options_before_subcommand(self):
+        self.assertTrue(hook._is_git_commit_command("git -C /tmp/repo -c user.name=x commit -m x"))
+
+    def test_non_commit(self):
+        self.assertFalse(hook._is_git_commit_command("git status"))
+        self.assertFalse(hook._is_git_commit_command("git add a.py && git push"))
+        self.assertFalse(hook._is_git_commit_command("ls -la"))
+
+    def test_commit_word_inside_message_is_not_a_commit(self):
+        self.assertFalse(hook._is_git_commit_command('git log --grep "git commit"'))
+        self.assertFalse(hook._is_git_commit_command('echo "run git commit later"'))
+
+    def test_multiline_message_stays_one_command(self):
+        self.assertTrue(hook._is_git_commit_command('git commit -m "line one\nline two"'))
+        self.assertFalse(hook._is_git_commit_command('git log --grep "a\ngit commit b"'))
+
+
+class AddedLines(unittest.TestCase):
+    DIFF = (
+        "--- a/x.py\n+++ b/x.py\n@@\n+try:\n+    pass\n+except Exception as e:\n"
+        "--- a/notes.md\n+++ b/notes.md\n@@\n+        except Exception as e:\n"
+    )
+
+    def test_pairs_lines_with_files(self):
+        pairs = hook._added_lines_by_file(self.DIFF)
+        self.assertIn(("x.py", "+except Exception as e:"), pairs)
+        self.assertIn(("notes.md", "+        except Exception as e:"), pairs)
+        self.assertFalse(any(line.startswith("+++") for _, line in pairs))
+
+
+class EndToEnd(unittest.TestCase):
+    def run_hook(self, command, cwd):
+        payload = json.dumps({"tool_input": {"command": command}})
+        return subprocess.run([sys.executable, hook.__file__], input=payload, capture_output=True,
+                              text=True, cwd=cwd)
+
+    def test_warning_is_json_system_message_and_python_only(self):
+        import tempfile
+        with tempfile.TemporaryDirectory() as tmp:
+            def git(*args):
+                subprocess.run(["git", *args], cwd=tmp, check=True, capture_output=True)
+            git("init", "-q")
+            with open(os.path.join(tmp, "notes.md"), "w") as f:
+                f.write("    except Exception as e:\n")
+            git("add", "notes.md")
+            out = self.run_hook("git commit -m x", tmp)
+            self.assertEqual(out.returncode, 0)
+            self.assertEqual(out.stdout.strip(), "")  # markdown example must not warn
+
+            with open(os.path.join(tmp, "a.py"), "w") as f:
+                f.write("try:\n    pass\nexcept Exception:\n    pass\n")
+            git("add", "a.py")
+            out = self.run_hook("git add a.py && git commit -m x", tmp)
+            self.assertEqual(out.returncode, 0)
+            data = json.loads(out.stdout)
+            self.assertIn("except Exception", data["systemMessage"])
+            self.assertEqual(data["hookSpecificOutput"]["hookEventName"], "PreToolUse")
+
+
+if __name__ == "__main__":
+    unittest.main()