From 8300c86f1dddec9780907a7b0582c76f3368066e Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 5 Sep 2026 19:09:40 +0000 Subject: [PATCH] feat: carry the corrected value in a fix field on every check commit-check now names the correction when a failed check has an unambiguous one and serialises it as "fix" (commit-check#564). The server already returns to_dict() as is, so the value flows through once that release is installed; with an older commit-check the key was simply absent, and an agent had to probe for it. Give the key a stable presence, empty when the engine has nothing to say, and tell agents in the tool descriptions how to use it: apply a non-empty fix as it stands, rewrite from the suggestion otherwise. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_018xYa7m3qup5wyN5MaXFgf6 --- README.md | 10 +++++- src/commit_check_mcp/server.py | 15 +++++--- tests/test_server.py | 65 ++++++++++++++++++++++++++++++++++ 3 files changed, 84 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index c07991b..1a00a2a 100644 --- a/README.md +++ b/README.md @@ -36,12 +36,20 @@ All validation tools return the same structured commit-check result shape: "status": "pass|fail", "value": "...", "error": "...", - "suggest": "..." + "suggest": "...", + "fix": "..." } ] } ``` +`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. + ## Installation ```bash diff --git a/src/commit_check_mcp/server.py b/src/commit_check_mcp/server.py index 22b0022..b4391e0 100644 --- a/src/commit_check_mcp/server.py +++ b/src/commit_check_mcp/server.py @@ -123,6 +123,11 @@ 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", "") overall = "fail" if any(c["status"] == "fail" for c in checks) else "pass" return {"status": overall, "checks": checks} @@ -328,7 +333,7 @@ def validate_commit_message( repo_path: str | None = None, config_path: str | None = None, ) -> dict[str, Any]: - """Validate a commit message against commit-check rules. Read-only validation. Returns a structured result with overall status ('pass'/'fail') and a list of per-check results. Each check includes the check name, status, value, error message (on failure), and suggestion (on failure). + """Validate a commit message against commit-check rules. Read-only validation. Returns a structured result with overall status ('pass'/'fail') and a list of per-check results. Each check includes the check name, status, value, error message (on failure), suggestion (on failure), and fix: the corrected value when the correction is unambiguous (a type's case, a missing colon, a WIP marker, a missing sign-off), otherwise an empty string. Apply a non-empty fix as it stands; when fix is empty, rewrite from the suggestion. Use this tool when you have a specific commit message string to validate. For batch validation of message, branch, and author together, use validate_commit_context instead. @@ -356,7 +361,7 @@ def validate_branch_name( repo_path: str | None = None, config_path: str | None = None, ) -> dict[str, Any]: - """Validate branch naming conventions with commit-check. Read-only validation. Returns a structured result with overall status ('pass'/'fail') and per-check results (check name, status, value, error, suggest). + """Validate branch naming conventions with commit-check. Read-only validation. Returns a structured result with overall status ('pass'/'fail') and per-check results (check name, status, value, error, suggest, and fix: the corrected value when unambiguous, else empty). Use this when you need to verify a branch name follows configured convention rules (e.g., feature/*, bugfix/*). For combined message+branch+author validation, use validate_commit_context. @@ -386,7 +391,7 @@ def validate_author_info( repo_path: str | None = None, config_path: str | None = None, ) -> dict[str, Any]: - """Validate commit author name and/or email with commit-check. Read-only validation. Returns a structured result with overall status and per-check results (check name, status, value, error, suggest). + """Validate commit author name and/or email with commit-check. Read-only validation. Returns a structured result with overall status and per-check results (check name, status, value, error, suggest, and fix: the corrected value when unambiguous, else empty). Use this when you need to verify author metadata against configured rules (e.g., allowed email domains, name patterns). When both name and email are provided, both are validated. If neither is provided, both are checked against repo context. For combined validation, use validate_commit_context. @@ -422,7 +427,7 @@ def validate_push_safety( repo_path: str | None = None, config_path: str | None = None, ) -> dict[str, Any]: - """Validate that a push is not a force push. Read-only validation. Returns a structured result with overall status and per-check results (check name, status, value, error, suggest). By default, force push is rejected; configure via 'push.allow_force_push' in config. + """Validate that a push is not a force push. Read-only validation. Returns a structured result with overall status and per-check results (check name, status, value, error, suggest, and fix: the corrected value when unambiguous, else empty). By default, force push is rejected; configure via 'push.allow_force_push' in config. Use this before performing a git push to ensure force-push protection rules are satisfied. Only validates the no_force_push rule. Use validate_commit_context for combined checks. @@ -452,7 +457,7 @@ def validate_commit_context( repo_path: str | None = None, config_path: str | None = None, ) -> dict[str, Any]: - """Run combined commit-check validations for message, branch, and/or author in one call. Read-only validation. Returns a structured result with overall status and a unified list of per-check results (check name, status, value, error, suggest). + """Run combined commit-check validations for message, branch, and/or author in one call. Read-only validation. Returns a structured result with overall status and a unified list of per-check results (check name, status, value, error, suggest, and fix: the corrected value when unambiguous, else empty). Use this when you need to validate multiple commit aspects simultaneously in a single call. At least one of message, branch, author_name, or author_email must be provided. For individual aspects, use the specific validate_commit_message, validate_branch_name, or validate_author_info tools. diff --git a/tests/test_server.py b/tests/test_server.py index 6f07456..ddb2f56 100644 --- a/tests/test_server.py +++ b/tests/test_server.py @@ -885,3 +885,68 @@ def fake_run(*, transport: str) -> None: monkeypatch.setattr(server.mcp, "run", fake_run) server.main() assert called + + +# --------------------------------------------------------------------------- +# _run_checks: the "fix" key +# --------------------------------------------------------------------------- + +class TestRunChecksFixField: + def test_every_check_carries_a_fix_key(self) -> None: + """Whatever the installed commit-check emits, an agent can read check["fix"].""" + from commit_check.engine import ValidationContext + + result = server._run_checks( + ["message"], ValidationContext(stdin_text="Fix: add x"), server._merge_config(None) + ) + assert result["status"] == "fail" + assert all("fix" in c for c in result["checks"]) + assert all(isinstance(c["fix"], str) for c in result["checks"]) + + def test_engine_fix_is_passed_through_untouched(self, monkeypatch: pytest.MonkeyPatch) -> None: + from commit_check.engine import ValidationContext + + class Outcome: + def to_dict(self) -> dict[str, str]: + return { + "rule_id": "CC001", + "check": "message", + "status": "fail", + "value": "Fix: add x", + "error": "Not conventional", + "suggest": 'Use "fix: add x"', + "fix": "fix: add x", + "docs_url": "https://commit-check.com/rules/#cc001", + } + + monkeypatch.setattr( + server.ValidationEngine, "validate_all_detailed", lambda self, context: [Outcome()] + ) + result = server._run_checks( + ["message"], ValidationContext(stdin_text="Fix: add x"), server._merge_config(None) + ) + 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"] == ""