fix: report a fully skipped run as skip, add warnings, require commit-check 2.17 (#34)

Every validation tool reduced its per-check statuses with its own copy of
`"fail" if any failed else "pass"`, so a run in which every check skipped
(the author is on `ignore_authors`, for example) came back as `pass`. An
agent reading that took a bypassed policy for an enforced one.

The four copies are replaced by one `_summarize` helper built on
commit-check's own `overall_status` and `count_warnings`, so the MCP result
now matches the CLI's `--format json`: `status` is `skip` when nothing was
validated, a `warn` check leaves `status` at `pass`, and a top-level
`warnings` field counts them.

Both helpers exist since commit-check 2.17.0, which becomes the floor;
`uv.lock` is regenerated accordingly. The `fix` key has been part of every
release the floor allows, so the `setdefault("fix", "")` shim for older
engines and its test are removed, along with the README caveat about them.

The README documents the `skip` and `warn` statuses and the `warnings`
field.

Author: shenxianpeng

README.md

@@ -29,11 +29,12 @@ All validation tools return the same structured commit-check result shape:
 
 ```json
 {
-  "status": "pass|fail",
+  "status": "pass|fail|skip",
+  "warnings": 0,
   "checks": [
     {
       "check": "message",
-      "status": "pass|fail",
+      "status": "pass|fail|warn|skip",
       "value": "...",
       "error": "...",
       "suggest": "...",
@@ -43,12 +44,17 @@ All validation tools return the same structured commit-check result shape:
 }
 ```
 
+Only `fail` is a rejection. A check reports `skip` when it did not run — the
+author matched `ignore_authors`, or there was nothing to check — and the
+top-level `status` is `skip` only when **every** check skipped, so a run that
+validated nothing is never reported as a pass. A check reports `warn` when the
+config lists it under `warn`: the finding is complete, but it does not fail
+the run, the top-level `status` stays `pass`, and `warnings` counts them.
+
 `suggest` is the advice a person reads. `fix` is the corrected value itself,
 present only when the correction is unambiguous — `Fix: add x` comes back with
 `"fix": "fix: add x"` — and an empty string otherwise, so an agent can apply
 a non-empty `fix` as it stands and fall back to `suggest` when it is empty.
-Populating `fix` needs a commit-check release that carries the field; with an
-older commit-check the key is present and always empty.
 
 A call that cannot run at all — an empty `message`, a `repo_path` that does not
 exist, a `repo_path` that is not a git repository when the tool has to read git

pyproject.toml

@@ -25,7 +25,7 @@ classifiers = [
   "Topic :: Software Development",
 ]
 dependencies = [
-  "commit-check>=2.11.0,<3",
+  "commit-check>=2.17.0,<3",
   "mcp>=2,<3"
 ]
 dynamic = ["version"]

src/commit_check_mcp/server.py

@@ -11,7 +11,13 @@
 
 from commit_check import __version__ as commit_check_version
 from commit_check.config_merger import deep_merge, get_default_config, load_toml_config
-from commit_check.engine import ValidationContext, ValidationEngine, CheckOutcome
+from commit_check.engine import (
+    CheckOutcome,
+    ValidationContext,
+    ValidationEngine,
+    count_warnings,
+    overall_status,
+)
 from commit_check.rule_builder import RuleBuilder, ValidationRule
 from commit_check.rules_catalog import BRANCH_RULES, COMMIT_RULES, PUSH_RULES
 from mcp.server.mcpserver import MCPServer
@@ -159,15 +165,24 @@ def _run_checks(
 
     engine = ValidationEngine(filtered)
     outcomes: list[CheckOutcome] = engine.validate_all_detailed(context)
-    checks = [o.to_dict() for o in outcomes]
-    # commit-check names the corrected value in "fix" when a failure has an
-    # unambiguous one. Older engines have no such field; give the key a
-    # stable presence so an agent can always test it instead of probing for it.
-    for check in checks:
-        check.setdefault("fix", "")
+    return _summarize([o.to_dict() for o in outcomes])
+
 
-    overall = "fail" if any(c["status"] == "fail" for c in checks) else "pass"
-    return {"status": overall, "checks": checks}
+def _summarize(checks: list[dict[str, Any]]) -> dict[str, Any]:
+    """Wrap per-check results in the shape every validation tool returns.
+
+    The overall ``status`` comes from commit-check's own reducer, so a run in
+    which every check skipped is reported as ``"skip"`` rather than ``"pass"``:
+    nothing was validated, and an agent must not read that as approval. A
+    ``"warn"`` is a finding the config asked to report without enforcing; it
+    leaves ``status`` at ``"pass"`` and is counted in ``warnings``.
+    """
+    statuses = [c["status"] for c in checks]
+    return {
+        "status": overall_status(statuses),
+        "warnings": count_warnings(statuses),
+        "checks": checks,
+    }
 
 
 def _validate_message(
@@ -267,11 +282,7 @@ def _validate_author(
                 ValidationContext(stdin_text=email, config=cfg),
                 cfg,
             )
-            checks = name_result["checks"] + email_result["checks"]
-            return {
-                "status": "fail" if any(c["status"] == "fail" for c in checks) else "pass",
-                "checks": checks,
-            }
+            return _summarize(name_result["checks"] + email_result["checks"])
 
         check_names: list[str] = []
         stdin = None
@@ -350,10 +361,7 @@ def _validate_all(
                     )["checks"]
                 )
 
-    return {
-        "status": "fail" if any(c["status"] == "fail" for c in checks) else "pass",
-        "checks": checks,
-    }
+    return _summarize(checks)
 
 
 @mcp.tool()
@@ -620,10 +628,7 @@ def validate_repository_state(
             )["checks"]
         )
 
-    return {
-        "status": "fail" if any(c["status"] == "fail" for c in checks) else "pass",
-        "checks": checks,
-    }
+    return _summarize(checks)
 
 
 @mcp.tool()

tests/test_server.py

@@ -890,6 +890,107 @@ def fake_run(*, transport: str) -> None:
         assert called
 
 
+# ---------------------------------------------------------------------------
+# _summarize: the overall status and the warnings count
+# ---------------------------------------------------------------------------
+
+
+def _outcome(status: str, check: str = "message") -> object:
+    class Outcome:
+        def to_dict(self) -> dict[str, str]:
+            return {
+                "check": check,
+                "status": status,
+                "value": "x",
+                "error": "",
+                "suggest": "",
+                "fix": "",
+            }
+
+    return Outcome()
+
+
+class TestSummarize:
+    def test_a_fully_skipped_run_is_reported_as_skip_not_pass(
+        self, monkeypatch: pytest.MonkeyPatch
+    ) -> None:
+        """Every check skipped means nothing was validated. The old hand-rolled
+        reducer defaulted anything that was not a failure to "pass", which
+        told an agent a bypassed policy had been enforced."""
+        from commit_check.engine import ValidationContext
+
+        monkeypatch.setattr(
+            server.ValidationEngine,
+            "validate_all_detailed",
+            lambda self, context: [_outcome("skip"), _outcome("skip", "author_name")],
+        )
+        result = server._run_checks(
+            ["message", "author_name"], ValidationContext(stdin_text="x"), server._merge_config(None)
+        )
+        assert result["status"] == "skip"
+        assert result["warnings"] == 0
+
+    def test_one_real_verdict_outweighs_the_skips(self, monkeypatch: pytest.MonkeyPatch) -> None:
+        from commit_check.engine import ValidationContext
+
+        monkeypatch.setattr(
+            server.ValidationEngine,
+            "validate_all_detailed",
+            lambda self, context: [_outcome("skip"), _outcome("pass", "branch")],
+        )
+        result = server._run_checks(
+            ["message", "branch"], ValidationContext(stdin_text="x"), server._merge_config(None)
+        )
+        assert result["status"] == "pass"
+
+    def test_a_warned_rule_is_counted_and_does_not_fail_the_run(self) -> None:
+        """Real engine: the config asks for the message rule as a warning."""
+        from commit_check.engine import ValidationContext
+
+        cfg = server._merge_config({"warn": ["message"]})
+        result = server._run_checks(
+            ["message"], ValidationContext(stdin_text="not conventional", config=cfg), cfg
+        )
+        assert result["status"] == "pass"
+        assert result["warnings"] == 1
+        assert result["checks"][0]["status"] == "warn"
+        assert result["checks"][0]["error"]
+
+    def test_a_failure_still_fails(self) -> None:
+        from commit_check.engine import ValidationContext
+
+        cfg = server._merge_config(None)
+        result = server._run_checks(
+            ["message"], ValidationContext(stdin_text="not conventional", config=cfg), cfg
+        )
+        assert result["status"] == "fail"
+        assert result["warnings"] == 0
+
+    def test_the_combined_tools_reduce_with_the_same_rule(
+        self, monkeypatch: pytest.MonkeyPatch
+    ) -> None:
+        """validate_commit_context, validate_author_info with both inputs and
+        validate_repository_state used to carry their own copy of the reducer."""
+
+        def skipped(check_names, context, config):
+            return server._summarize(
+                [
+                    {"check": cn, "status": "skip", "value": "", "error": "", "suggest": "", "fix": ""}
+                    for cn in check_names
+                ]
+            )
+
+        monkeypatch.setattr(server, "_run_checks", skipped)
+
+        context = server._validate_all(message="x", branch="main")
+        assert context["status"] == "skip"
+        assert context["warnings"] == 0
+
+        author = server._validate_author(name="A", email="a@b.c")
+        assert author["status"] == "skip"
+        assert "warnings" in author
+
+
 # ---------------------------------------------------------------------------
 # _run_checks: the "fix" key
 # ---------------------------------------------------------------------------
@@ -931,29 +1032,6 @@ def to_dict(self) -> dict[str, str]:
         assert result["checks"][0]["fix"] == "fix: add x"
         assert result["checks"][0]["suggest"] == 'Use "fix: add x"'
 
-    def test_an_engine_without_the_field_yields_an_empty_fix(
-        self, monkeypatch: pytest.MonkeyPatch
-    ) -> None:
-        from commit_check.engine import ValidationContext
-
-        class OldOutcome:
-            def to_dict(self) -> dict[str, str]:
-                return {
-                    "check": "message",
-                    "status": "fail",
-                    "value": "Fix: add x",
-                    "error": "Not conventional",
-                    "suggest": "Use a conventional type",
-                }
-
-        monkeypatch.setattr(
-            server.ValidationEngine, "validate_all_detailed", lambda self, context: [OldOutcome()]
-        )
-        result = server._run_checks(
-            ["message"], ValidationContext(stdin_text="Fix: add x"), server._merge_config(None)
-        )
-        assert result["checks"][0]["fix"] == ""
-
 
 # ---------------------------------------------------------------------------
 # Errors reach the agent as tool errors (through the MCP tool manager)