diff --git a/README.md b/README.md index 77bb0dc..973efe3 100644 --- a/README.md +++ b/README.md @@ -83,7 +83,7 @@ mcpServers: |---|---|---| | `validate_commit_message` | A commit message: Conventional Commits, subject length and case, body, sign-off, WIP and fixup markers, AI attribution | `message` | | `validate_branch_name` | A branch name, or the checked-out one, and its rebase target when configured | `branch?` | -| `validate_author_info` | Author name and email; read from the repository's git config when omitted | `author_name?` `author_email?` | +| `validate_author_info` | Author name and email, and whether an AI tool is the author (CC017); read from the repository's git config when omitted | `author_name?` `author_email?` | | `validate_commit_context` | Message, branch and author in one call, for whichever you pass | `message?` `branch?` `author_name?` `author_email?` | | `validate_push_safety` | That a push is not a force push (CC301) | `push_refs?` — pre-push lines; omit to compare the branch with its upstream | | `validate_repository_state` | The latest commit's message and author, the checked-out branch and, optionally, the push | `include_message` `include_branch` `include_author` (default `true`), `include_push` (default `false`) | diff --git a/src/commit_check_mcp/server.py b/src/commit_check_mcp/server.py index 1aac4b8..9c7dd07 100644 --- a/src/commit_check_mcp/server.py +++ b/src/commit_check_mcp/server.py @@ -468,6 +468,58 @@ def _validate_push( return result +@contextmanager +def _pending_author(name: str | None, email: str | None): + """Make git report the given name/email as the author of the next commit. + + commit-check judges a message not yet committed (CC017, ``ai_author``) by + the author ``git var GIT_AUTHOR_IDENT`` reports, which honours + ``GIT_AUTHOR_NAME``/``GIT_AUTHOR_EMAIL`` as ``git commit`` does. Setting + them lets that rule judge the author the caller passed instead of the + server's own git identity. Use it only inside :func:`_working_directory`, + whose lock keeps other calls from seeing the change. + """ + overrides = { + key: value + for key, value in (("GIT_AUTHOR_NAME", name), ("GIT_AUTHOR_EMAIL", email)) + if value is not None + } + saved = {key: os.environ.get(key) for key in overrides} + os.environ.update(overrides) + try: + yield + finally: + for key, value in saved.items(): + if value is None: + os.environ.pop(key, None) + else: + os.environ[key] = value + + +def _author_checks( + name: str | None, email: str | None, cfg: dict[str, Any] +) -> list[dict[str, Any]]: + """Check the author the caller passed: name and email patterns, then CC017. + + CC017 (``ai_author``) only exists when ``[commit] ai_attribution`` is + ``forbid`` or ``disclose``, so with the default policy it adds nothing. + """ + checks: list[dict[str, Any]] = [] + if name is not None: + checks += _run_checks(["author_name"], ValidationContext(stdin_text=name, config=cfg), cfg)[ + "checks" + ] + if email is not None: + checks += _run_checks( + ["author_email"], ValidationContext(stdin_text=email, config=cfg), cfg + )["checks"] + with _pending_author(name, email): + checks += _run_checks(["ai_author"], ValidationContext(stdin_text="", config=cfg), cfg)[ + "checks" + ] + return checks + + def _validate_author( name: str | None = None, email: str | None = None, @@ -480,31 +532,11 @@ def _validate_author( cfg = _merge_config(config, repo_path=repo_path, config_path=config_path) with _working_directory(repo_path): - if name is not None and email is not None: - name_result = _run_checks( - ["author_name"], - ValidationContext(stdin_text=name, config=cfg), - cfg, - ) - email_result = _run_checks( - ["author_email"], - ValidationContext(stdin_text=email, config=cfg), - cfg, + if name is None and email is None: + return _run_checks( + ["author_name", "author_email"], ValidationContext(stdin_text=None, config=cfg), cfg ) - return _summarize(name_result["checks"] + email_result["checks"]) - - check_names: list[str] = [] - stdin = None - if name is not None: - check_names.append("author_name") - stdin = name - if email is not None: - check_names.append("author_email") - stdin = email - if not check_names: - check_names = ["author_name", "author_email"] - - return _run_checks(check_names, ValidationContext(stdin_text=stdin, config=cfg), cfg) + return _summarize(_author_checks(name, email, cfg)) def _validate_all( @@ -521,11 +553,14 @@ def _validate_all( cfg = _merge_config(config, repo_path=repo_path, config_path=config_path) checks: list[dict[str, Any]] = [] + has_author = author_name is not None or author_email is not None with _working_directory(repo_path): if message is not None: + # With an author passed, CC017 runs below against that author. + message_checks = MESSAGE_CHECKS - {"ai_author"} if has_author else MESSAGE_CHECKS checks.extend( _run_checks( - MESSAGE_CHECKS, + message_checks, ValidationContext(stdin_text=message, config=cfg), cfg, )["checks"] @@ -538,23 +573,8 @@ def _validate_all( cfg, )["checks"] ) - if author_name is not None or author_email is not None: - if author_name is not None: - checks.extend( - _run_checks( - ["author_name"], - ValidationContext(stdin_text=author_name, config=cfg), - cfg, - )["checks"] - ) - if author_email is not None: - checks.extend( - _run_checks( - ["author_email"], - ValidationContext(stdin_text=author_email, config=cfg), - cfg, - )["checks"] - ) + if has_author: + checks.extend(_author_checks(author_name, author_email, cfg)) return _summarize(checks) @@ -640,7 +660,8 @@ def validate_author_info( config_path: ConfigPathParam = None, ) -> dict[str, Any]: """Validate a commit author's name and/or email against the configured rules (e.g. allowed - email domains, name patterns). Read-only. + email domains, name patterns, and under ai_attribution forbid/disclose that the author is not + an AI tool). Read-only. {result_shape} @@ -888,9 +909,7 @@ def describe_validation_rules( "commit_check_version": commit_check_version, "config": merged_config, "supported_checks": list( - dict.fromkeys( - entry.check for entry in COMMIT_RULES + BRANCH_RULES + PUSH_RULES - ) + dict.fromkeys(entry.check for entry in COMMIT_RULES + BRANCH_RULES + PUSH_RULES) ), "enabled_rules": rules, } diff --git a/tests/test_server.py b/tests/test_server.py index 00ea4fc..49d285e 100644 --- a/tests/test_server.py +++ b/tests/test_server.py @@ -283,8 +283,8 @@ def fake_run_checks(check_names, context, config): result = server._validate_author("Alice", "alice@example.com") assert result["status"] == "pass" - assert len(result["checks"]) == 2 - assert calls == [["author_name"], ["author_email"]] + assert len(result["checks"]) == 3 + assert calls == [["author_name"], ["author_email"], ["ai_author"]] def test_only_name(self, monkeypatch: pytest.MonkeyPatch) -> None: calls: list[list[str]] = [] @@ -303,8 +303,8 @@ def fake_run_checks(check_names, context, config): result = server._validate_author(name="Bob") assert result["status"] == "pass" - assert len(result["checks"]) == 1 - assert calls == [["author_name"]] + assert len(result["checks"]) == 2 + assert calls == [["author_name"], ["ai_author"]] def test_only_email(self, monkeypatch: pytest.MonkeyPatch) -> None: calls: list[list[str]] = [] @@ -323,8 +323,8 @@ def fake_run_checks(check_names, context, config): result = server._validate_author(email="bob@example.com") assert result["status"] == "pass" - assert len(result["checks"]) == 1 - assert calls == [["author_email"]] + assert len(result["checks"]) == 2 + assert calls == [["author_email"], ["ai_author"]] def test_neither_name_nor_email(self, monkeypatch: pytest.MonkeyPatch) -> None: calls: list[list[str]] = [] @@ -1584,3 +1584,80 @@ def test_the_disclose_policy_reaches_the_agent(self) -> None: assert by_id["CC015"]["status"] == "fail" assert by_id["CC016"]["status"] == "pass" assert by_id["CC014"]["fix"] == "feat: add caching\n\nAssisted-by: Claude Opus 5" + + +class TestAiAuthorJudgesThePassedAuthor: + """CC017 (commit-check 2.20) judges the author passed in, not the server's own. + + The rule reads the author git would record for a message not yet + committed. Through this server that was the server's git identity, so an + agent asking about "Claude " under forbid was told + it passed, and a person's name could fail for the server's AI identity. + """ + + FORBID = {"commit": {"ai_attribution": "forbid"}} + + @pytest.fixture(autouse=True) + def _requires_cc017(self) -> None: + from commit_check.rules_catalog import MESSAGE_CHECKS + + if "ai_author" not in MESSAGE_CHECKS: + pytest.skip("commit-check before 2.20 has no CC017") + + @pytest.fixture + def server_author(self, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("GIT_AUTHOR_NAME", "Server Identity") + monkeypatch.setenv("GIT_AUTHOR_EMAIL", "server@example.com") + + @staticmethod + def _cc017(result: dict) -> list[dict]: + return [c for c in result["checks"] if c["rule_id"] == "CC017"] + + @pytest.mark.usefixtures("server_author") + def test_author_info_fails_an_ai_author(self) -> None: + result = server.validate_author_info( + author_name="Claude", author_email="noreply@anthropic.com", config=self.FORBID + ) + (cc017,) = self._cc017(result) + assert result["status"] == "fail" + assert cc017["status"] == "fail" + assert cc017["value"] == "Claude " + + def test_author_info_passes_a_person(self, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("GIT_AUTHOR_NAME", "Claude") + monkeypatch.setenv("GIT_AUTHOR_EMAIL", "noreply@anthropic.com") + result = server.validate_author_info( + author_name="Alice", author_email="alice@example.com", config=self.FORBID + ) + (cc017,) = self._cc017(result) + assert result["status"] == "pass" + assert cc017["value"] == "Alice " + # The server's own author is back once the call returns. + assert os.environ["GIT_AUTHOR_NAME"] == "Claude" + + @pytest.mark.usefixtures("server_author") + def test_commit_context_runs_cc017_once_against_the_passed_author(self) -> None: + result = server.validate_commit_context( + message="feat: add caching", + author_name="Claude", + author_email="noreply@anthropic.com", + config=self.FORBID, + ) + (cc017,) = self._cc017(result) + assert cc017["status"] == "fail" + assert cc017["value"] == "Claude " + + @pytest.mark.usefixtures("server_author") + def test_message_alone_still_uses_the_pending_author(self) -> None: + result = server.validate_commit_context(message="feat: add caching", config=self.FORBID) + (cc017,) = self._cc017(result) + assert cc017["value"] == "Server Identity " + + def test_no_cc017_under_the_default_policy(self) -> None: + result = server.validate_author_info(author_name="Claude") + assert self._cc017(result) == [] + + def test_environment_is_restored_when_unset(self, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.delenv("GIT_AUTHOR_NAME", raising=False) + server.validate_author_info(author_name="Claude", config=self.FORBID) + assert "GIT_AUTHOR_NAME" not in os.environ