Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 10 additions & 4 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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": "...",
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
Expand Down
49 changes: 27 additions & 22 deletions src/commit_check_mcp/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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()
Expand Down
124 changes: 101 additions & 23 deletions tests/test_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
# ---------------------------------------------------------------------------
Expand Down Expand Up @@ -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)
Expand Down
8 changes: 4 additions & 4 deletions uv.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.