diff options
| -rw-r--r-- | claude-rules/commits.md | 2 | ||||
| -rw-r--r-- | claude-rules/verification.md | 2 | ||||
| -rw-r--r-- | hooks/_common.py | 18 | ||||
| -rwxr-xr-x | hooks/git-commit-confirm.py | 57 | ||||
| -rw-r--r-- | hooks/tests/test_git_commit_confirm.py | 58 |
5 files changed, 135 insertions, 2 deletions
diff --git a/claude-rules/commits.md b/claude-rules/commits.md index 5fe8f1b..4509cee 100644 --- a/claude-rules/commits.md +++ b/claude-rules/commits.md @@ -457,7 +457,7 @@ independent gate. 2. Scan the message for AI-attribution language (including emojis and footers), and on a public or shared-remote repo for tooling-path enumeration — prose that lists `CLAUDE.md`, `.claude/`, `.ai/`, `todo.org`, `notes.org`, or `session-context`. Name the category, not the paths. Exempt: a commit whose change is one of those files, and private single-user repos. 3. Review the diff — only intended changes staged; no unrelated files. 4. Confirm staged files belong in the repo: nothing that the project's policy keeps untracked (the personal-tooling set in gitignore-mode projects), and in repos with a canonical/mirror split, the edit is on the canonical side — a mirror-only edit gets reverted by the next sync. -5. Run tests and linters (see `verification.md`). +5. Run the full test suite and linters as their own step, read the result, and commit only on zero failures — never chain the run into the commit command (see `verification.md`). ## If You Catch Yourself diff --git a/claude-rules/verification.md b/claude-rules/verification.md index 1bbd8dd..388e0b5 100644 --- a/claude-rules/verification.md +++ b/claude-rules/verification.md @@ -92,7 +92,7 @@ Use this whenever the verification gap from "When You Cannot Verify" above is a ## Before Committing Before any commit: -1. Run the test suite — confirm all tests pass +1. Run the full test suite as its own command, read the result, and commit only when failures are zero — never bundle the run with the commit (e.g. `make test; git commit`), where a red suite can't stop the commit. Run the whole suite, not just the touched file: a change can break a test elsewhere. If the suite can't run, that's "unable to verify" (see When You Cannot Verify above) — surface it, don't commit silently. 2. Run the linter — confirm no new warnings 3. Run the type checker — confirm no new errors 4. Review the diff — confirm only intended changes are staged diff --git a/hooks/_common.py b/hooks/_common.py index e82f7ed..c1e0578 100644 --- a/hooks/_common.py +++ b/hooks/_common.py @@ -65,6 +65,24 @@ def respond_ask(reason: str, system_message: Optional[str] = None) -> None: print(json.dumps(output)) +def respond_deny(reason: str, system_message: Optional[str] = None) -> None: + """Emit a PreToolUse response that blocks the tool call outright. + + Unlike `respond_ask`, the user gets no approve option — the call is denied + and `reason` tells the agent why, so it can restructure and retry. + """ + output: dict = { + "hookSpecificOutput": { + "hookEventName": "PreToolUse", + "permissionDecision": "deny", + "permissionDecisionReason": reason, + } + } + if system_message: + output["systemMessage"] = system_message + print(json.dumps(output)) + + def read_referenced_file(path: str, max_bytes: int = 1_000_000) -> Optional[str]: """Read a local file referenced by -F/--file/--body-file so its text can be attribution-scanned. Return the text, or None if it can't be safely read diff --git a/hooks/git-commit-confirm.py b/hooks/git-commit-confirm.py index 618ac20..cf01948 100755 --- a/hooks/git-commit-confirm.py +++ b/hooks/git-commit-confirm.py @@ -46,6 +46,7 @@ from _common import ( read_payload, read_referenced_file, respond_ask, + respond_deny, scan_attribution, ) @@ -53,6 +54,28 @@ from _common import ( MAX_FILES_SHOWN = 25 MAX_MESSAGE_LINES = 30 +# Recognized full-suite test runners across languages. +TEST_RUNNER_RE = re.compile( + r"\b(" + r"make\s+(?:test|check)" + r"|pytest" + r"|go\s+test" + r"|cargo\s+test" + r"|npm\s+(?:run\s+)?test" + r"|yarn\s+test" + r"|pnpm\s+(?:run\s+)?test" + r"|jest|vitest|bats|tox|rspec|phpunit|ctest" + r")\b" +) + +GIT_COMMIT_RE = re.compile(r"(?:^|[\s;&|()\n])git\s+(?:-[^\s]+\s+)*commit\b") + +BUNDLED_TEST_REASON = ( + "Blocked: a test run is bundled with `git commit` in one command, where a " + "red suite won't stop the commit. Run the full test suite as its own step, " + "read the result, and commit only on zero failures (see verification.md)." +) + UNPARSEABLE_MESSAGE = ( "(commit message not parseable from command line; " "will be edited interactively)" @@ -68,6 +91,12 @@ def main() -> int: if not is_git_commit(cmd): return 0 + # Hard gate: a test run bundled into the commit command is denied outright, + # because an ungated chain lets a red suite commit anyway. + if detect_bundled_test_run(cmd): + respond_deny(BUNDLED_TEST_REASON) + return 0 + message = extract_commit_message(cmd) staged = get_staged_files() stats = get_diff_stats() @@ -117,6 +146,34 @@ def collect_issues(message: str, staged: list[str], author: str) -> list[str]: return issues +def detect_bundled_test_run(cmd: str): + """Return a truthy reason if a `git commit` is chained after a test run in + one command via an ungated connector (a red suite wouldn't stop the commit). + + `&&` between the test run and the commit is the one safe connector — the + commit runs only on a green suite — and is allowed. Any other chaining (`;`, + `&`, `|`, `||`, newline, or a pipe that masks the suite's exit) is flagged. + The test runner is matched only in the command *prefix* before `git commit`, + so a runner name inside the commit message never trips the detector. + """ + if not cmd: + return None + commit = GIT_COMMIT_RE.search(cmd) + if not commit: + return None + prefix = cmd[: commit.start()] + runs = list(TEST_RUNNER_RE.finditer(prefix)) + if not runs: + return None + # Everything between the last test run and the commit. Strip the safe `&&` + # connectors; anything left that chains commands means the commit isn't + # gated on the suite's success. + segment = prefix[runs[-1].end():].replace("&&", "") + if re.search(r"[;|\n&]", segment): + return BUNDLED_TEST_REASON + return None + + def is_git_commit(cmd: str) -> bool: """True if the command invokes `git commit` (possibly with env/cd prefix).""" # Strip leading assignments and subshells; find a `git commit` word boundary diff --git a/hooks/tests/test_git_commit_confirm.py b/hooks/tests/test_git_commit_confirm.py index 83519ad..4bf95cf 100644 --- a/hooks/tests/test_git_commit_confirm.py +++ b/hooks/tests/test_git_commit_confirm.py @@ -95,3 +95,61 @@ def test_oversized_file_falls_through_and_hook_asks(tmp_path, monkeypatch): # And the hook would ask, because UNPARSEABLE_MESSAGE is a flagged issue. issues = hook.collect_issues(msg, staged=["a.py"], author="Dev <d@e.com>") assert any("not parseable" in i for i in issues) + + +# --- bundled test-run + commit: the hard gate ------------------------------ + +def test_bundle_semicolon_make_test_is_flagged(): + assert hook.detect_bundled_test_run('make test; git commit -m "x"') + + +def test_bundle_ampersand_gated_is_allowed(): + # `&&` runs the commit only on a green suite — safe, not flagged. + assert hook.detect_bundled_test_run('make test && git commit -m "x"') is None + + +def test_bundle_pytest_semicolon_is_flagged(): + assert hook.detect_bundled_test_run('pytest ; git commit -m "x"') + + +def test_bundle_npm_test_is_flagged(): + assert hook.detect_bundled_test_run('npm test; git commit -m "x"') + + +def test_bundle_go_test_is_flagged(): + assert hook.detect_bundled_test_run('go test ./...; git commit -m "x"') + + +def test_bundle_cargo_test_is_flagged(): + assert hook.detect_bundled_test_run('cargo test ; git commit -m "x"') + + +def test_bundle_bats_is_flagged(): + assert hook.detect_bundled_test_run('bats tests/ ; git commit -m "x"') + + +def test_bundle_pipe_masks_exit_is_flagged(): + # `make test | tee log` exits with tee's status, so && gates on tee, not + # the suite — a red suite would still commit. Flag it. + assert hook.detect_bundled_test_run('make test | tee log && git commit -m "x"') + + +def test_bundle_or_connector_is_flagged(): + assert hook.detect_bundled_test_run('make test || git commit -m "x"') + + +def test_runner_only_in_message_is_not_flagged(): + # "make test" inside the commit message must not trip the detector. + assert hook.detect_bundled_test_run('git commit -m "remember to make test"') is None + + +def test_plain_commit_is_not_flagged(): + assert hook.detect_bundled_test_run('git commit -m "fix: thing"') is None + + +def test_gated_chain_before_commit_is_allowed(): + assert hook.detect_bundled_test_run('cd proj && pytest && git commit -m "x"') is None + + +def test_empty_command_is_not_flagged(): + assert hook.detect_bundled_test_run("") is None |