From 60d75866258ebb30fe5be78a953e69bbb9036ae7 Mon Sep 17 00:00:00 2001 From: Xianpeng Shen Date: Thu, 13 Aug 2026 10:10:54 +0300 Subject: [PATCH 1/5] test: stop four JSON tests taking their verdict from the checkout (#548) main went red on the push run for #542 with four failures in TestJsonFormat, all reading 'skip' where they assert 'pass'. Nothing regressed: the tests had been measuring the repository they run in. Each of the four supplies a message, on stdin or in a file, which makes it a prospective commit -- so _resolve_current_author reads `git config user.name` and falls back to HEAD's author. Both are ambient. A GitHub runner configures no git identity (no workflow here sets one), so the fallback always decides, and #542 was a dependabot merge: HEAD's author was dependabot[bot], which cchk.toml lists in [commit] ignore_authors. Every commit check skipped, and overall status is 'skip' when they all do. Reproduced against a clone pinned to 9f12a63 with the global and system git config disabled, which is what the runner looks like: HEAD author : dependabot[bot] user.name : '' -> status 'skip' (the four failures) user.name : set -> status 'pass' (why laptops and PRs were green) So it was never about #542's contents, and it will recur on the next bot-authored merge to main. The four now take a `pinned_author` fixture that fixes both identity sources, leaving the verdict to come from the message under test. The two tests in the class that already passed are the two that happened to patch get_commit_info for other reasons -- the same pin, arrived at by accident. Pinning it in a fixture would hide the fallback everywhere it applies, so it is now asserted directly instead: a new test drives an unconfigured identity with a bot as HEAD's author and expects every check to skip, exit code still 0. What silently decided other tests' results is now a contract of its own. Verified in that clone: 4 failed before, 7 passed after, and the full suite is unchanged in both a configured and an unconfigured environment. Claude-Session: https://claude.ai/code/session_01U9zFxq8V4qxG4aMzJhGBFn Co-authored-by: Claude Opus 5 --- tests/main_test.py | 64 +++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 60 insertions(+), 4 deletions(-) diff --git a/tests/main_test.py b/tests/main_test.py index 448a080..028258b 100644 --- a/tests/main_test.py +++ b/tests/main_test.py @@ -35,6 +35,26 @@ def _stdin_gate_open(request, monkeypatch): yield +@pytest.fixture +def pinned_author(mocker): + """Pin the identity the engine resolves, so no verdict comes from the checkout. + + A message given on stdin or in a file describes a *prospective* commit, so + ``_resolve_current_author`` reads ``git config user.name`` and falls back to + the author of ``HEAD``. Both are ambient. A CI runner configures no identity, + so the fallback always wins there, and when ``HEAD`` happens to be a bot's + commit that name is in ``ignore_authors`` — every commit check skips, and a + test asserting ``pass`` sees ``skip`` instead. + + That is not hypothetical: it turned ``main`` red the moment a dependabot + merge landed, having passed on every pull request before it. Any test that + supplies a message and asserts a verdict needs this, or it is really + asserting something about whoever committed last. + """ + mocker.patch("commit_check.engine.get_git_config_value", return_value="test-author") + mocker.patch("commit_check.engine.get_commit_info", return_value="test-author") + + class TestMain: @pytest.mark.benchmark def test_help(self, capfd, monkeypatch): @@ -693,7 +713,9 @@ class TestJsonFormat: """Tests for --format json machine-readable output.""" @pytest.mark.benchmark - def test_json_format_valid_message_returns_pass(self, mocker, capsys, monkeypatch): + def test_json_format_valid_message_returns_pass( + self, mocker, capsys, monkeypatch, pinned_author + ): """JSON output for a valid commit message has status=pass.""" mocker.patch("sys.stdin.isatty", return_value=False) mocker.patch("sys.stdin.read", return_value="feat: add new feature\n") @@ -709,7 +731,9 @@ def test_json_format_valid_message_returns_pass(self, mocker, capsys, monkeypatc assert all("check" in c and "status" in c for c in data["checks"]) @pytest.mark.benchmark - def test_json_format_pass_reports_checked_value(self, mocker, capsys, monkeypatch): + def test_json_format_pass_reports_checked_value( + self, mocker, capsys, monkeypatch, pinned_author + ): """JSON output reports the checked value even when the check passed.""" mocker.patch("sys.stdin.isatty", return_value=False) mocker.patch("sys.stdin.read", return_value="feat: add new feature\n") @@ -765,7 +789,7 @@ def test_json_format_no_ascii_art_in_stdout(self, mocker, capsys, monkeypatch): assert "\033[" not in out @pytest.mark.benchmark - def test_json_format_from_file(self, capsys, monkeypatch): + def test_json_format_from_file(self, capsys, monkeypatch, pinned_author): """JSON mode works when reading commit message from a file.""" with tempfile.NamedTemporaryFile(mode="w", suffix=".txt", delete=False) as f: f.write("fix: resolve null pointer in auth module") @@ -782,7 +806,9 @@ def test_json_format_from_file(self, capsys, monkeypatch): os.unlink(tmp_path) @pytest.mark.benchmark - def test_json_format_exit_code_matches_status(self, mocker, capsys, monkeypatch): + def test_json_format_exit_code_matches_status( + self, mocker, capsys, monkeypatch, pinned_author + ): """Exit code 1 when JSON status is fail, exit code 0 when pass.""" # --- pass case --- mocker.patch("sys.stdin.isatty", return_value=False) @@ -803,6 +829,36 @@ def test_json_format_exit_code_matches_status(self, mocker, capsys, monkeypatch) assert rc_fail == 1 assert json.loads(out)["status"] == "fail" + @pytest.mark.benchmark + def test_json_format_skips_when_head_author_is_ignored( + self, mocker, capsys, monkeypatch + ): + """An unconfigured identity falls back to HEAD's author, ignore list and all. + + The counterpart to ``pinned_author``: rather than let this behaviour + stay ambient — where it silently decides other tests' verdicts — it is + asserted here. With no ``user.name`` configured, as on a CI runner, the + author of ``HEAD`` decides, so a bot's merge commit skips every check + even though the message under test is a perfectly valid one. + + Exit code stays 0: a skip is not a failure. + """ + mocker.patch("commit_check.engine.get_git_config_value", return_value="") + mocker.patch( + "commit_check.engine.get_commit_info", return_value="dependabot[bot]" + ) + mocker.patch("sys.stdin.isatty", return_value=False) + mocker.patch("sys.stdin.read", return_value="feat: add new feature\n") + + monkeypatch.setattr("sys.argv", [CMD, "-m", "--format", "json"]) + rc = main() + + out, _ = capsys.readouterr() + data = json.loads(out) + assert rc == 0 + assert data["status"] == "skip" + assert all(c["status"] == "skip" for c in data["checks"]) + class TestNoBanner: """Tests for --no-banner flag.""" From 41df9728755757ace03c7ef0968d4ab81b056ecc Mon Sep 17 00:00:00 2001 From: Xianpeng Shen Date: Thu, 13 Aug 2026 10:12:33 +0300 Subject: [PATCH 2/5] docs: Clean up README formatting (#549) Removed extra line break before 'Quick Start' section. --- README.md | 1 - 1 file changed, 1 deletion(-) diff --git a/README.md b/README.md index e04f9e3..849cacb 100644 --- a/README.md +++ b/README.md @@ -44,7 +44,6 @@ local hooks, CI, GitHub Actions, and AI automation. - **Machine-readable output:** JSON + Python API for automation and AI agents ![commit-check demo](https://github.com/commit-check/commit-check/raw/main/assets/demo.gif) -
## Quick Start From b446cb85e30c93c46b3ca786f57ded42cd51bdd1 Mon Sep 17 00:00:00 2001 From: Lars Christensen Date: Sun, 16 Aug 2026 14:00:58 +0200 Subject: [PATCH 3/5] fix: only emit ANSI color when stdout is a TTY (#551) --- commit_check/__init__.py | 38 +++++++++++++++++--- tests/util_test.py | 77 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 110 insertions(+), 5 deletions(-) diff --git a/commit_check/__init__.py b/commit_check/__init__.py index 341b9f6..e38ad4c 100644 --- a/commit_check/__init__.py +++ b/commit_check/__init__.py @@ -6,17 +6,45 @@ __version__ (package version) """ +import os +import sys from importlib.metadata import version, PackageNotFoundError # Exit codes used across the package PASS = 0 FAIL = 1 -# ANSI color codes used for CLI output -RED = "\033[91m" -GREEN = "\033[92m" -YELLOW = "\033[93m" -RESET_COLOR = "\033[0m" + +def supports_color() -> bool: + """Whether the terminal renders ANSI color. + + A piped or redirected stream is read as plain text (a CI log, a file, an + agent harness), where the escape sequences are noise, so this errs towards + saying no when ``stdout`` is not a terminal. + + ``FORCE_COLOR`` overrides the detection in both directions: ``0`` turns + color off even on a terminal, any other value turns it on even when piped. + + An empty ``TERM`` is the same "no terminal type" signal as ``dumb``: the + user has deliberately said there are no terminfo capabilities, so emit + plain text. An *unset* ``TERM`` is different — it just means nobody set + it, and a real terminal is still likely color-capable. + """ + forced = os.environ.get("FORCE_COLOR") + if forced: + return forced != "0" + if not sys.stdout.isatty(): + return False + if os.environ.get("TERM") in ("", "dumb"): + return False + return True + + +# ANSI color codes used for CLI output, empty when stdout cannot render color. +RED = "\033[91m" if supports_color() else "" +GREEN = "\033[92m" if supports_color() else "" +YELLOW = "\033[93m" if supports_color() else "" +RESET_COLOR = "\033[0m" if supports_color() else "" # Follow conventional commits DEFAULT_COMMIT_TYPES = [ diff --git a/tests/util_test.py b/tests/util_test.py index e325198..e344524 100644 --- a/tests/util_test.py +++ b/tests/util_test.py @@ -1,5 +1,8 @@ +import importlib import pytest import subprocess +import commit_check +from commit_check import supports_color from commit_check.util import ( fetch_remote_ref, fetch_upstream_ref, @@ -753,6 +756,80 @@ def test_blank_line_closes_the_block(self, capfd, mocker): assert lines[-2].startswith("Docs: ") assert "" not in lines[:-1] + class TestColor: + """ANSI color belongs on a terminal, not in piped output. + + A CI log or an agent harness reads the escape payload as noise, so the + color codes are dropped when ``stdout`` is not a terminal. + """ + + @pytest.mark.benchmark + def test_not_supported_when_piped(self, mocker): + mocker.patch.dict("os.environ", {}, clear=True) + mocker.patch("sys.stdout.isatty", return_value=False) + assert supports_color() is False + + @pytest.mark.benchmark + def test_forced_even_when_piped(self, mocker): + mocker.patch.dict("os.environ", {"FORCE_COLOR": "1"}, clear=True) + mocker.patch("sys.stdout.isatty", return_value=False) + assert supports_color() is True + + @pytest.mark.benchmark + def test_force_zero_turns_color_off(self, mocker): + """Setting it to 0 must not read as "set, therefore on".""" + mocker.patch.dict("os.environ", {"FORCE_COLOR": "0"}, clear=True) + mocker.patch("sys.stdout.isatty", return_value=True) + assert supports_color() is False + + @pytest.mark.benchmark + def test_force_empty_falls_through_to_detection(self, mocker): + mocker.patch.dict("os.environ", {"FORCE_COLOR": ""}, clear=True) + mocker.patch("sys.stdout.isatty", return_value=False) + assert supports_color() is False + + @pytest.mark.benchmark + def test_dumb_term_turns_color_off(self, mocker): + mocker.patch.dict("os.environ", {"TERM": "dumb"}, clear=True) + mocker.patch("sys.stdout.isatty", return_value=True) + assert supports_color() is False + + @pytest.mark.benchmark + def test_empty_term_turns_color_off(self, mocker): + """``TERM=`` is a deliberate "no terminal" signal, like ``dumb``.""" + mocker.patch.dict("os.environ", {"TERM": ""}, clear=True) + mocker.patch("sys.stdout.isatty", return_value=True) + assert supports_color() is False + + @pytest.mark.benchmark + def test_unset_term_still_allows_color_on_a_tty(self, mocker): + """An absent TERM is not a refusal — the terminal may still render.""" + mocker.patch.dict("os.environ", {}, clear=True) + mocker.patch("sys.stdout.isatty", return_value=True) + assert supports_color() is True + + @pytest.mark.benchmark + def test_constants_empty_when_color_off(self, mocker): + """The constants are pre-emptied when stdout cannot render color.""" + mocker.patch.dict("os.environ", {"TERM": "dumb"}, clear=True) + mocker.patch("sys.stdout.isatty", return_value=True) + importlib.reload(commit_check) + assert commit_check.RED == "" + assert commit_check.GREEN == "" + assert commit_check.YELLOW == "" + assert commit_check.RESET_COLOR == "" + + @pytest.mark.benchmark + def test_constants_set_when_color_on(self, mocker): + """FORCE_COLOR=1 keeps the raw escape codes in place.""" + mocker.patch.dict("os.environ", {"FORCE_COLOR": "1"}, clear=True) + mocker.patch("sys.stdout.isatty", return_value=False) + importlib.reload(commit_check) + assert commit_check.RED == "\033[91m" + assert commit_check.GREEN == "\033[92m" + assert commit_check.YELLOW == "\033[93m" + assert commit_check.RESET_COLOR == "\033[0m" + class TestPrintSuggestion: @pytest.mark.benchmark def test_print_suggestion(self, capfd): From 3e4b218a48ba4ff9af2f526d2976b33295c29dde Mon Sep 17 00:00:00 2001 From: Xianpeng Shen Date: Sun, 16 Aug 2026 15:18:12 +0300 Subject: [PATCH 4/5] fix: support the NO_COLOR convention for disabling color (#552) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit supports_color() from #551 answered FORCE_COLOR, the TTY and TERM, but not NO_COLOR — the variable users actually export globally to opt out of color (https://no-color.org). Any non-empty value now disables color, outranking detection and yielding only to an explicit FORCE_COLOR. Two gaps in the #551 tests are closed alongside. The reload-based tests recomputed the module constants under a patched environment and left the last reload's values in place for every test that ran afterwards; a fixture now re-derives them on teardown. And nothing exercised the copies commit_check.util binds at import — the ones the print functions actually read — so two subprocess tests now run the real import path end to end and assert on what gets printed. --- commit_check/__init__.py | 16 ++++-- tests/util_test.py | 111 ++++++++++++++++++++++++++++++++++++++- 2 files changed, 120 insertions(+), 7 deletions(-) diff --git a/commit_check/__init__.py b/commit_check/__init__.py index e38ad4c..d4f4761 100644 --- a/commit_check/__init__.py +++ b/commit_check/__init__.py @@ -22,8 +22,11 @@ def supports_color() -> bool: agent harness), where the escape sequences are noise, so this errs towards saying no when ``stdout`` is not a terminal. - ``FORCE_COLOR`` overrides the detection in both directions: ``0`` turns + ``FORCE_COLOR`` overrides everything else, in both directions: ``0`` turns color off even on a terminal, any other value turns it on even when piped. + ``NO_COLOR`` set to any non-empty value turns color off, per the + convention at https://no-color.org — it is what users export globally, so + it outranks detection but yields to an explicit force. An empty ``TERM`` is the same "no terminal type" signal as ``dumb``: the user has deliberately said there are no terminfo capabilities, so emit @@ -33,6 +36,8 @@ def supports_color() -> bool: forced = os.environ.get("FORCE_COLOR") if forced: return forced != "0" + if os.environ.get("NO_COLOR"): + return False if not sys.stdout.isatty(): return False if os.environ.get("TERM") in ("", "dumb"): @@ -41,10 +46,11 @@ def supports_color() -> bool: # ANSI color codes used for CLI output, empty when stdout cannot render color. -RED = "\033[91m" if supports_color() else "" -GREEN = "\033[92m" if supports_color() else "" -YELLOW = "\033[93m" if supports_color() else "" -RESET_COLOR = "\033[0m" if supports_color() else "" +_colored = supports_color() +RED = "\033[91m" if _colored else "" +GREEN = "\033[92m" if _colored else "" +YELLOW = "\033[93m" if _colored else "" +RESET_COLOR = "\033[0m" if _colored else "" # Follow conventional commits DEFAULT_COMMIT_TYPES = [ diff --git a/tests/util_test.py b/tests/util_test.py index e344524..26fb436 100644 --- a/tests/util_test.py +++ b/tests/util_test.py @@ -1,4 +1,6 @@ import importlib +import os +import sys import pytest import subprocess import commit_check @@ -763,12 +765,48 @@ class TestColor: color codes are dropped when ``stdout`` is not a terminal. """ + @pytest.fixture + def restored_module(self): + """Re-derive the module constants after a test that reloads. + + A reload recomputes ``commit_check.RED`` and friends under the + test's patched environment, and nothing else puts them back: the + last reload's values would leak into every test that runs + afterwards. Listed before ``mocker`` in the signature so this + teardown runs after the environment patches are undone. + """ + yield + importlib.reload(commit_check) + @pytest.mark.benchmark def test_not_supported_when_piped(self, mocker): mocker.patch.dict("os.environ", {}, clear=True) mocker.patch("sys.stdout.isatty", return_value=False) assert supports_color() is False + @pytest.mark.benchmark + def test_no_color_turns_color_off_on_a_tty(self, mocker): + """NO_COLOR is what users export globally (https://no-color.org).""" + mocker.patch.dict("os.environ", {"NO_COLOR": "1"}, clear=True) + mocker.patch("sys.stdout.isatty", return_value=True) + assert supports_color() is False + + @pytest.mark.benchmark + def test_no_color_empty_falls_through_to_detection(self, mocker): + """The convention counts only a non-empty value as set.""" + mocker.patch.dict("os.environ", {"NO_COLOR": ""}, clear=True) + mocker.patch("sys.stdout.isatty", return_value=True) + assert supports_color() is True + + @pytest.mark.benchmark + def test_force_color_outranks_no_color(self, mocker): + """An explicit force wins over the global opt-out.""" + mocker.patch.dict( + "os.environ", {"NO_COLOR": "1", "FORCE_COLOR": "1"}, clear=True + ) + mocker.patch("sys.stdout.isatty", return_value=False) + assert supports_color() is True + @pytest.mark.benchmark def test_forced_even_when_piped(self, mocker): mocker.patch.dict("os.environ", {"FORCE_COLOR": "1"}, clear=True) @@ -809,7 +847,7 @@ def test_unset_term_still_allows_color_on_a_tty(self, mocker): assert supports_color() is True @pytest.mark.benchmark - def test_constants_empty_when_color_off(self, mocker): + def test_constants_empty_when_color_off(self, restored_module, mocker): """The constants are pre-emptied when stdout cannot render color.""" mocker.patch.dict("os.environ", {"TERM": "dumb"}, clear=True) mocker.patch("sys.stdout.isatty", return_value=True) @@ -820,7 +858,7 @@ def test_constants_empty_when_color_off(self, mocker): assert commit_check.RESET_COLOR == "" @pytest.mark.benchmark - def test_constants_set_when_color_on(self, mocker): + def test_constants_set_when_color_on(self, restored_module, mocker): """FORCE_COLOR=1 keeps the raw escape codes in place.""" mocker.patch.dict("os.environ", {"FORCE_COLOR": "1"}, clear=True) mocker.patch("sys.stdout.isatty", return_value=False) @@ -830,6 +868,75 @@ def test_constants_set_when_color_on(self, mocker): assert commit_check.YELLOW == "\033[93m" assert commit_check.RESET_COLOR == "\033[0m" + # Not benchmarked: these spawn an interpreter, and their cost is the + # process, not the code under test. + + def test_the_decision_reaches_printed_output(self): + """FORCE_COLOR=1 must colour what the print path emits. + + The reload tests above stop at the module constants, but the print + functions in ``commit_check.util`` hold their own copies, bound + once at import. Only a fresh interpreter exercises that hand-off, + so this is the test that fails if the decision stops reaching the + output a user sees. + """ + result = subprocess.run( + [ + sys.executable, + "-c", + "from commit_check.util import print_error_message;" + "print_error_message('message', 'err', 'value', rule_id='CC001')", + ], + capture_output=True, + encoding="utf-8", + env={**os.environ, "FORCE_COLOR": "1"}, + ) + assert result.returncode == 0, result.stderr + assert "\033[91m" in result.stdout + + #: Child program that pretends its stdout is a terminal before the + #: import, so terminal detection says yes and the environment becomes + #: the deciding factor. A plain pipe would disable color on its own + #: and mask whether NO_COLOR handling exists at all. + _TTY_CHILD = ( + "import sys, types;" + "out = sys.stdout;" + "sys.stdout = types.SimpleNamespace(" + "write=out.write, flush=out.flush, isatty=lambda: True);" + "from commit_check.util import print_error_message;" + "print_error_message('message', 'err', 'value', rule_id='CC001')" + ) + + def test_no_color_reaches_printed_output(self): + """NO_COLOR must be what decides, not the pipe. + + Two identical runs, differing only in ``NO_COLOR``. The first + proves the pretend-TTY works — it must come out colored, or the + second assertion would pass even with the NO_COLOR handling + deleted. + """ + env = {**os.environ, "TERM": "xterm"} + env.pop("FORCE_COLOR", None) + env.pop("NO_COLOR", None) + + colored = subprocess.run( + [sys.executable, "-c", self._TTY_CHILD], + capture_output=True, + encoding="utf-8", + env=env, + ) + assert colored.returncode == 0, colored.stderr + assert "\033[" in colored.stdout + + plain = subprocess.run( + [sys.executable, "-c", self._TTY_CHILD], + capture_output=True, + encoding="utf-8", + env={**env, "NO_COLOR": "1"}, + ) + assert plain.returncode == 0, plain.stderr + assert "\033[" not in plain.stdout + class TestPrintSuggestion: @pytest.mark.benchmark def test_print_suggestion(self, capfd): From 90ace064c5be9b089013833bcb1ec993ff304d4b Mon Sep 17 00:00:00 2001 From: Xianpeng Shen Date: Sun, 16 Aug 2026 15:23:47 +0300 Subject: [PATCH 5/5] chore: Update commit-check version to v2.15.1 --- README.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 849cacb..07042f5 100644 --- a/README.md +++ b/README.md @@ -60,7 +60,7 @@ commit-check --message --branch ```yaml repos: - repo: https://github.com/commit-check/commit-check - rev: v2.15.0 + rev: v2.15.1 hooks: - id: check-message - id: check-branch @@ -195,7 +195,7 @@ commit-check --message # In pre-commit hooks (.pre-commit-config.yaml) repos: - repo: https://github.com/commit-check/commit-check - rev: v2.15.0 + rev: v2.15.1 hooks: - id: check-message args: @@ -225,7 +225,7 @@ commit-check --no-force-push # In pre-commit hooks (.pre-commit-config.yaml) repos: - repo: https://github.com/commit-check/commit-check - rev: v2.15.0 + rev: v2.15.1 hooks: - id: check-no-force-push stages: [pre-push]