diff --git a/scripts/check_doc_gate.py b/scripts/check_doc_gate.py index abd970363..023bdd544 100644 --- a/scripts/check_doc_gate.py +++ b/scripts/check_doc_gate.py @@ -42,10 +42,13 @@ # for argparse, never our own code), 3 a config error (broken, missing, or # unparseable config). 3 is kept off 2 so a typo'd flag is never mistaken for # a bad config: a misconfigured gate must be distinguishable from both a real -# documentation-drift violation and a usage mistake. +# documentation-drift violation and a usage mistake. 4 is a git infrastructure +# failure (missing ref, network error, shallow clone, etc.) so it is never +# confused with a rule violation. EXIT_OK = 0 EXIT_VIOLATION = 1 EXIT_CONFIG_ERROR = 3 +EXIT_GIT_ERROR = 4 # A path-like token: one of the four known repo prefixes followed by a run of # non-whitespace / non-quoting characters. The negative lookbehind stops us @@ -283,11 +286,22 @@ def evaluate_rules( return failures -def _run_git(args: list[str]) -> str: - result = subprocess.run( - ["git", *args], cwd=REPO_ROOT, capture_output=True, text=True, check=True, - ) - return result.stdout +class GitCommandError(Exception): + """Raised when a git command fails, so infrastructure failures are + distinguishable from genuine doc-gate violations.""" + + +def _run_git(args: list[str], ref: str | None = None) -> str: + try: + result = subprocess.run( + ["git", *args], cwd=REPO_ROOT, capture_output=True, text=True, check=True, + ) + return result.stdout + except subprocess.CalledProcessError: + msg = f"git {' '.join(args)} failed" + if ref: + msg += f" (ref: {ref})" + raise GitCommandError(msg) from None def _parse_name_status(output: str) -> list[tuple[str, str]]: @@ -309,11 +323,11 @@ def _git_changed_staged() -> list[tuple[str, str]]: def _git_changed_base(base_ref: str) -> list[tuple[str, str]]: - return _parse_name_status(_run_git(["diff", "--name-status", f"{base_ref}...HEAD"])) + return _parse_name_status(_run_git(["diff", "--name-status", f"{base_ref}...HEAD"], ref=base_ref)) def _git_commit_messages(base_ref: str) -> list[str]: - out = _run_git(["log", f"{base_ref}..HEAD", "--format=%B%x00"]) + out = _run_git(["log", f"{base_ref}..HEAD", "--format=%B%x00"], ref=base_ref) return [m for m in out.split("\x00") if m.strip()] @@ -323,7 +337,7 @@ def _git_commits_with_messages(base_ref: str) -> list[tuple[str, str, str]]: # inside it. A record terminator distinct from the field separator is what # makes this parseable: with one separator for both, the flat split cannot # tell a new commit's hash from the previous commit's body. - out = _run_git(["log", f"{base_ref}..HEAD", "--format=%H%x1f%an%x1f%B%x1e"]) + out = _run_git(["log", f"{base_ref}..HEAD", "--format=%H%x1f%an%x1f%B%x1e"], ref=base_ref) commits: list[tuple[str, str, str]] = [] for record in out.split("\x1e"): if not record.strip(): @@ -393,14 +407,18 @@ def main(argv: list[str] | None = None) -> int: return 0 # diff-gate - if args.staged: - changed = _git_changed_staged() - commit_messages: list[str] = [] - else: - changed = _git_changed_base(args.base) - commits_meta = _git_commits_with_messages(args.base) - commit_messages = [msg for _hash, _author, msg in commits_meta] - _log_trailer_usage(commits_meta, get_trailer(config)) + try: + if args.staged: + changed = _git_changed_staged() + commit_messages: list[str] = [] + else: + changed = _git_changed_base(args.base) + commits_meta = _git_commits_with_messages(args.base) + commit_messages = [msg for _hash, _author, msg in commits_meta] + _log_trailer_usage(commits_meta, get_trailer(config)) + except GitCommandError as e: + print(f"doc-gate: git error: {e}", file=sys.stderr) + return EXIT_GIT_ERROR failures = evaluate_rules(changed, commit_messages, config) return _report(failures) diff --git a/tests/test_check_doc_gate.py b/tests/test_check_doc_gate.py index 82845e52c..2abba3367 100644 --- a/tests/test_check_doc_gate.py +++ b/tests/test_check_doc_gate.py @@ -1,8 +1,10 @@ from __future__ import annotations import importlib.util +import subprocess import sys from pathlib import Path +from unittest.mock import MagicMock, patch import pytest @@ -216,3 +218,45 @@ def test_extractor_ignores_hyphen_glued_prefix(self): "read ~/.claude/projects/-home-x-tinyagentos/memory/MEMORY.md at start" ) assert toks == [] + + +class TestGitCommandErrorHandling: + """Git infrastructure failures must never be confused with rule violations.""" + + def test_nonexistent_base_ref_exits_git_error(self, capsys): + """A bad --base ref should produce the new exit code with a clear message.""" + error = subprocess.CalledProcessError( + 128, + ["git", "diff", "--name-status", "origin/no-such-ref...HEAD"], + ) + error.stderr = "fatal: bad revision 'origin/no-such-ref'\n" + + with patch.object(_MOD.subprocess, "run", side_effect=error): + code = _MOD.main(["diff-gate", "--base", "origin/no-such-ref"]) + assert code == _MOD.EXIT_GIT_ERROR + captured = capsys.readouterr() + assert "diff" in captured.err + assert "origin/no-such-ref" in captured.err + assert "Traceback" not in captured.err + + def test_genuine_violation_still_exits_1(self, capsys): + """A real rule violation must still exit 1.""" + mock_result_diff = MagicMock() + mock_result_diff.stdout = "A\ttinyagentos/routes/themes.py\n" + mock_result_log = MagicMock() + mock_result_log.stdout = "\x00" + + with patch.object(_MOD.subprocess, "run", side_effect=[mock_result_diff, mock_result_log]): + code = _MOD.main(["diff-gate", "--base", "origin/HEAD"]) + assert code == _MOD.EXIT_VIOLATION + + def test_clean_run_still_exits_0(self, capsys): + """A clean run must still exit 0.""" + mock_result_diff = MagicMock() + mock_result_diff.stdout = "" + mock_result_log = MagicMock() + mock_result_log.stdout = "\x00" + + with patch.object(_MOD.subprocess, "run", side_effect=[mock_result_diff, mock_result_log]): + code = _MOD.main(["diff-gate", "--base", "origin/HEAD"]) + assert code == _MOD.EXIT_OK diff --git a/tests/test_doc_gate.py b/tests/test_doc_gate.py index 921061009..f4591db4f 100644 --- a/tests/test_doc_gate.py +++ b/tests/test_doc_gate.py @@ -584,7 +584,10 @@ class TestCommitsWithMessagesParsing: ) def _parse(self, monkeypatch, out): - monkeypatch.setattr(dg, "_run_git", lambda args: out) + # Accepts ref= because _run_git now takes it: callers pass the base ref + # so a git failure can name it. A double that does not accept the real + # signature fails with TypeError and says nothing about the parser. + monkeypatch.setattr(dg, "_run_git", lambda args, ref=None: out) return dg._git_commits_with_messages("origin/dev") def test_parses_one_record_per_commit(self, monkeypatch):