Skip to content

Commit f8feca9

Browse files
shenxianpengshenxianpeng
authored
fix: judge CC017 by the author passed to the author tools (#48)
commit-check 2.20 added CC017, which fails a commit whose author is an AI tool. For a message not yet committed it reads the author git would record, so through this server it judged the server's own git identity: validate_author_info never ran it, and validate_commit_context ignored the author_name/author_email it was given. Run CC017 with the passed author as GIT_AUTHOR_NAME/GIT_AUTHOR_EMAIL, inside the working-directory lock, and restore the environment after.
1 parent 33b7d5e commit f8feca9

3 files changed

Lines changed: 149 additions & 53 deletions

File tree

‎README.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ mcpServers:
8383
|---|---|---|
8484
| `validate_commit_message` | A commit message: Conventional Commits, subject length and case, body, sign-off, WIP and fixup markers, AI attribution | `message` |
8585
| `validate_branch_name` | A branch name, or the checked-out one, and its rebase target when configured | `branch?` |
86-
| `validate_author_info` | Author name and email; read from the repository's git config when omitted | `author_name?` `author_email?` |
86+
| `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?` |
8787
| `validate_commit_context` | Message, branch and author in one call, for whichever you pass | `message?` `branch?` `author_name?` `author_email?` |
8888
| `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 |
8989
| `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`) |

‎src/commit_check_mcp/server.py‎

Lines changed: 65 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -468,6 +468,58 @@ def _validate_push(
468468
return result
469469

470470

471+
@contextmanager
472+
def _pending_author(name: str | None, email: str | None):
473+
"""Make git report the given name/email as the author of the next commit.
474+
475+
commit-check judges a message not yet committed (CC017, ``ai_author``) by
476+
the author ``git var GIT_AUTHOR_IDENT`` reports, which honours
477+
``GIT_AUTHOR_NAME``/``GIT_AUTHOR_EMAIL`` as ``git commit`` does. Setting
478+
them lets that rule judge the author the caller passed instead of the
479+
server's own git identity. Use it only inside :func:`_working_directory`,
480+
whose lock keeps other calls from seeing the change.
481+
"""
482+
overrides = {
483+
key: value
484+
for key, value in (("GIT_AUTHOR_NAME", name), ("GIT_AUTHOR_EMAIL", email))
485+
if value is not None
486+
}
487+
saved = {key: os.environ.get(key) for key in overrides}
488+
os.environ.update(overrides)
489+
try:
490+
yield
491+
finally:
492+
for key, value in saved.items():
493+
if value is None:
494+
os.environ.pop(key, None)
495+
else:
496+
os.environ[key] = value
497+
498+
499+
def _author_checks(
500+
name: str | None, email: str | None, cfg: dict[str, Any]
501+
) -> list[dict[str, Any]]:
502+
"""Check the author the caller passed: name and email patterns, then CC017.
503+
504+
CC017 (``ai_author``) only exists when ``[commit] ai_attribution`` is
505+
``forbid`` or ``disclose``, so with the default policy it adds nothing.
506+
"""
507+
checks: list[dict[str, Any]] = []
508+
if name is not None:
509+
checks += _run_checks(["author_name"], ValidationContext(stdin_text=name, config=cfg), cfg)[
510+
"checks"
511+
]
512+
if email is not None:
513+
checks += _run_checks(
514+
["author_email"], ValidationContext(stdin_text=email, config=cfg), cfg
515+
)["checks"]
516+
with _pending_author(name, email):
517+
checks += _run_checks(["ai_author"], ValidationContext(stdin_text="", config=cfg), cfg)[
518+
"checks"
519+
]
520+
return checks
521+
522+
471523
def _validate_author(
472524
name: str | None = None,
473525
email: str | None = None,
@@ -480,31 +532,11 @@ def _validate_author(
480532
cfg = _merge_config(config, repo_path=repo_path, config_path=config_path)
481533

482534
with _working_directory(repo_path):
483-
if name is not None and email is not None:
484-
name_result = _run_checks(
485-
["author_name"],
486-
ValidationContext(stdin_text=name, config=cfg),
487-
cfg,
488-
)
489-
email_result = _run_checks(
490-
["author_email"],
491-
ValidationContext(stdin_text=email, config=cfg),
492-
cfg,
535+
if name is None and email is None:
536+
return _run_checks(
537+
["author_name", "author_email"], ValidationContext(stdin_text=None, config=cfg), cfg
493538
)
494-
return _summarize(name_result["checks"] + email_result["checks"])
495-
496-
check_names: list[str] = []
497-
stdin = None
498-
if name is not None:
499-
check_names.append("author_name")
500-
stdin = name
501-
if email is not None:
502-
check_names.append("author_email")
503-
stdin = email
504-
if not check_names:
505-
check_names = ["author_name", "author_email"]
506-
507-
return _run_checks(check_names, ValidationContext(stdin_text=stdin, config=cfg), cfg)
539+
return _summarize(_author_checks(name, email, cfg))
508540

509541

510542
def _validate_all(
@@ -521,11 +553,14 @@ def _validate_all(
521553
cfg = _merge_config(config, repo_path=repo_path, config_path=config_path)
522554
checks: list[dict[str, Any]] = []
523555

556+
has_author = author_name is not None or author_email is not None
524557
with _working_directory(repo_path):
525558
if message is not None:
559+
# With an author passed, CC017 runs below against that author.
560+
message_checks = MESSAGE_CHECKS - {"ai_author"} if has_author else MESSAGE_CHECKS
526561
checks.extend(
527562
_run_checks(
528-
MESSAGE_CHECKS,
563+
message_checks,
529564
ValidationContext(stdin_text=message, config=cfg),
530565
cfg,
531566
)["checks"]
@@ -538,23 +573,8 @@ def _validate_all(
538573
cfg,
539574
)["checks"]
540575
)
541-
if author_name is not None or author_email is not None:
542-
if author_name is not None:
543-
checks.extend(
544-
_run_checks(
545-
["author_name"],
546-
ValidationContext(stdin_text=author_name, config=cfg),
547-
cfg,
548-
)["checks"]
549-
)
550-
if author_email is not None:
551-
checks.extend(
552-
_run_checks(
553-
["author_email"],
554-
ValidationContext(stdin_text=author_email, config=cfg),
555-
cfg,
556-
)["checks"]
557-
)
576+
if has_author:
577+
checks.extend(_author_checks(author_name, author_email, cfg))
558578

559579
return _summarize(checks)
560580

@@ -640,7 +660,8 @@ def validate_author_info(
640660
config_path: ConfigPathParam = None,
641661
) -> dict[str, Any]:
642662
"""Validate a commit author's name and/or email against the configured rules (e.g. allowed
643-
email domains, name patterns). Read-only.
663+
email domains, name patterns, and under ai_attribution forbid/disclose that the author is not
664+
an AI tool). Read-only.
644665
645666
{result_shape}
646667
@@ -888,9 +909,7 @@ def describe_validation_rules(
888909
"commit_check_version": commit_check_version,
889910
"config": merged_config,
890911
"supported_checks": list(
891-
dict.fromkeys(
892-
entry.check for entry in COMMIT_RULES + BRANCH_RULES + PUSH_RULES
893-
)
912+
dict.fromkeys(entry.check for entry in COMMIT_RULES + BRANCH_RULES + PUSH_RULES)
894913
),
895914
"enabled_rules": rules,
896915
}

‎tests/test_server.py‎

Lines changed: 83 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -283,8 +283,8 @@ def fake_run_checks(check_names, context, config):
283283

284284
result = server._validate_author("Alice", "alice@example.com")
285285
assert result["status"] == "pass"
286-
assert len(result["checks"]) == 2
287-
assert calls == [["author_name"], ["author_email"]]
286+
assert len(result["checks"]) == 3
287+
assert calls == [["author_name"], ["author_email"], ["ai_author"]]
288288

289289
def test_only_name(self, monkeypatch: pytest.MonkeyPatch) -> None:
290290
calls: list[list[str]] = []
@@ -303,8 +303,8 @@ def fake_run_checks(check_names, context, config):
303303

304304
result = server._validate_author(name="Bob")
305305
assert result["status"] == "pass"
306-
assert len(result["checks"]) == 1
307-
assert calls == [["author_name"]]
306+
assert len(result["checks"]) == 2
307+
assert calls == [["author_name"], ["ai_author"]]
308308

309309
def test_only_email(self, monkeypatch: pytest.MonkeyPatch) -> None:
310310
calls: list[list[str]] = []
@@ -323,8 +323,8 @@ def fake_run_checks(check_names, context, config):
323323

324324
result = server._validate_author(email="bob@example.com")
325325
assert result["status"] == "pass"
326-
assert len(result["checks"]) == 1
327-
assert calls == [["author_email"]]
326+
assert len(result["checks"]) == 2
327+
assert calls == [["author_email"], ["ai_author"]]
328328

329329
def test_neither_name_nor_email(self, monkeypatch: pytest.MonkeyPatch) -> None:
330330
calls: list[list[str]] = []
@@ -1584,3 +1584,80 @@ def test_the_disclose_policy_reaches_the_agent(self) -> None:
15841584
assert by_id["CC015"]["status"] == "fail"
15851585
assert by_id["CC016"]["status"] == "pass"
15861586
assert by_id["CC014"]["fix"] == "feat: add caching\n\nAssisted-by: Claude Opus 5"
1587+
1588+
1589+
class TestAiAuthorJudgesThePassedAuthor:
1590+
"""CC017 (commit-check 2.20) judges the author passed in, not the server's own.
1591+
1592+
The rule reads the author git would record for a message not yet
1593+
committed. Through this server that was the server's git identity, so an
1594+
agent asking about "Claude <noreply@anthropic.com>" under forbid was told
1595+
it passed, and a person's name could fail for the server's AI identity.
1596+
"""
1597+
1598+
FORBID = {"commit": {"ai_attribution": "forbid"}}
1599+
1600+
@pytest.fixture(autouse=True)
1601+
def _requires_cc017(self) -> None:
1602+
from commit_check.rules_catalog import MESSAGE_CHECKS
1603+
1604+
if "ai_author" not in MESSAGE_CHECKS:
1605+
pytest.skip("commit-check before 2.20 has no CC017")
1606+
1607+
@pytest.fixture
1608+
def server_author(self, monkeypatch: pytest.MonkeyPatch) -> None:
1609+
monkeypatch.setenv("GIT_AUTHOR_NAME", "Server Identity")
1610+
monkeypatch.setenv("GIT_AUTHOR_EMAIL", "server@example.com")
1611+
1612+
@staticmethod
1613+
def _cc017(result: dict) -> list[dict]:
1614+
return [c for c in result["checks"] if c["rule_id"] == "CC017"]
1615+
1616+
@pytest.mark.usefixtures("server_author")
1617+
def test_author_info_fails_an_ai_author(self) -> None:
1618+
result = server.validate_author_info(
1619+
author_name="Claude", author_email="noreply@anthropic.com", config=self.FORBID
1620+
)
1621+
(cc017,) = self._cc017(result)
1622+
assert result["status"] == "fail"
1623+
assert cc017["status"] == "fail"
1624+
assert cc017["value"] == "Claude <noreply@anthropic.com>"
1625+
1626+
def test_author_info_passes_a_person(self, monkeypatch: pytest.MonkeyPatch) -> None:
1627+
monkeypatch.setenv("GIT_AUTHOR_NAME", "Claude")
1628+
monkeypatch.setenv("GIT_AUTHOR_EMAIL", "noreply@anthropic.com")
1629+
result = server.validate_author_info(
1630+
author_name="Alice", author_email="alice@example.com", config=self.FORBID
1631+
)
1632+
(cc017,) = self._cc017(result)
1633+
assert result["status"] == "pass"
1634+
assert cc017["value"] == "Alice <alice@example.com>"
1635+
# The server's own author is back once the call returns.
1636+
assert os.environ["GIT_AUTHOR_NAME"] == "Claude"
1637+
1638+
@pytest.mark.usefixtures("server_author")
1639+
def test_commit_context_runs_cc017_once_against_the_passed_author(self) -> None:
1640+
result = server.validate_commit_context(
1641+
message="feat: add caching",
1642+
author_name="Claude",
1643+
author_email="noreply@anthropic.com",
1644+
config=self.FORBID,
1645+
)
1646+
(cc017,) = self._cc017(result)
1647+
assert cc017["status"] == "fail"
1648+
assert cc017["value"] == "Claude <noreply@anthropic.com>"
1649+
1650+
@pytest.mark.usefixtures("server_author")
1651+
def test_message_alone_still_uses_the_pending_author(self) -> None:
1652+
result = server.validate_commit_context(message="feat: add caching", config=self.FORBID)
1653+
(cc017,) = self._cc017(result)
1654+
assert cc017["value"] == "Server Identity <server@example.com>"
1655+
1656+
def test_no_cc017_under_the_default_policy(self) -> None:
1657+
result = server.validate_author_info(author_name="Claude")
1658+
assert self._cc017(result) == []
1659+
1660+
def test_environment_is_restored_when_unset(self, monkeypatch: pytest.MonkeyPatch) -> None:
1661+
monkeypatch.delenv("GIT_AUTHOR_NAME", raising=False)
1662+
server.validate_author_info(author_name="Claude", config=self.FORBID)
1663+
assert "GIT_AUTHOR_NAME" not in os.environ

0 commit comments

Comments
 (0)