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
2 changes: 1 addition & 1 deletion .github/dependabot.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,4 @@ updates:
groups:
pip:
patterns:
- "*"
- "*"
21 changes: 17 additions & 4 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

Model Context Protocol (MCP) server for [commit-check](https://github.com/commit-check/commit-check).

`commit-check-mcp` exposes `commit-check` as local MCP tools so an MCP client can validate commit messages, branch names, author info, and repository state.
`commit-check-mcp` exposes `commit-check` as local MCP tools so an MCP client can validate commit messages, branch names, author info, push safety, and repository state.

## Features

Expand All @@ -17,9 +17,10 @@ This MCP server exposes commit-check validations as MCP tools:
- `server_health` — returns server/sdk versions
- `validate_commit_message` — validates a commit message
- `validate_branch_name` — validates a branch name or the current repo branch
- `validate_push_safety` — validates that a push is not a force push
- `validate_author_info` — validates author name/email or the repo's git author config
- `validate_commit_context` — runs combined checks in one call
- `validate_repository_state` — validates latest commit, current branch, and author state for a repo
- `validate_repository_state` — validates latest commit, current branch, author state, and optional push safety for a repo
- `describe_validation_rules` — returns the effective config and enabled rules after merging defaults and repo config

All validation tools return the same structured commit-check result shape:
Expand Down Expand Up @@ -106,9 +107,10 @@ After the client starts the server, it will expose these tools:
- `server_health`: returns server, SDK, and dependency versions
- `validate_commit_message(message, config?, repo_path?, config_path?)`
- `validate_branch_name(branch?, config?, repo_path?, config_path?)`
- `validate_push_safety(push_refs?, config?, repo_path?, config_path?)`
- `validate_author_info(author_name?, author_email?, config?, repo_path?, config_path?)`
- `validate_commit_context(message?, branch?, author_name?, author_email?, config?, repo_path?, config_path?)`
- `validate_repository_state(repo_path?, config?, config_path?, include_message?, include_branch?, include_author?)`
- `validate_repository_state(repo_path?, config?, config_path?, include_message?, include_branch?, include_author?, include_push?)`
- `describe_validation_rules(config?, repo_path?, config_path?)`

The common optional arguments are:
Expand Down Expand Up @@ -148,6 +150,15 @@ Validate the full repository state:
}
```

Validate push safety from git pre-push hook ref metadata:

```json
{
"repo_path": "/path/to/repo",
"push_refs": "refs/heads/main abc123 refs/heads/main def456"
}
```

Inspect the final merged rules that will be applied:

```json
Expand All @@ -173,6 +184,7 @@ Typical patterns:

- Validate an explicit message with a repository's rules
- Validate the current repository state without passing message/branch/author values manually
- Validate push safety using pre-push ref metadata, or check the current branch against its upstream
- Inspect which rules are actually enabled after config merging

Example payload for a repository-wide validation:
Expand All @@ -182,7 +194,8 @@ Example payload for a repository-wide validation:
"repo_path": "/path/to/repo",
"include_message": true,
"include_branch": true,
"include_author": true
"include_author": true,
"include_push": true
}
```

Expand Down
64 changes: 58 additions & 6 deletions src/commit_check_mcp/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,16 +14,16 @@
from commit_check.config_merger import deep_merge, get_default_config, load_toml_config
from commit_check.engine import ValidationContext, ValidationEngine, ValidationResult
from commit_check.rule_builder import RuleBuilder, ValidationRule
from commit_check.rules_catalog import BRANCH_RULES, COMMIT_RULES
from commit_check.rules_catalog import BRANCH_RULES, COMMIT_RULES, PUSH_RULES
from mcp.server.fastmcp import FastMCP

from . import __version__

mcp = FastMCP(
"commit-check-mcp",
instructions=(
"Use these tools to validate commit messages, branch names, and author metadata "
"with commit-check."
"Use these tools to validate commit messages, branch names, author metadata, "
"and push safety with commit-check."
),
)

Expand Down Expand Up @@ -185,6 +185,28 @@ def _validate_branch(
)


def _validate_push(
push_refs: str | None = None,
*,
config: dict[str, Any] | None = None,
repo_path: Path | None = None,
config_path: str | None = None,
) -> dict[str, Any]:
"""Validate push ref updates against commit-check force-push protection."""
cfg = _merge_config(config, repo_path=repo_path, config_path=config_path)
cfg.setdefault("push", {})["allow_force_push"] = False
with _working_directory(repo_path):
return _run_checks(
["no_force_push"],
ValidationContext(
stdin_text=push_refs,
config=cfg,
push_upstream_fallback=push_refs is None,
),
cfg,
)


def _validate_author(
name: str | None = None,
email: str | None = None,
Expand Down Expand Up @@ -373,6 +395,24 @@ def validate_author_info(
)


@mcp.tool()
def validate_push_safety(
push_refs: str | None = None,
config: dict[str, Any] | None = None,
repo_path: str | None = None,
config_path: str | None = None,
) -> dict[str, Any]:
"""Validate that a push is not a force push."""
normalized_push_refs = push_refs.strip() if isinstance(push_refs, str) else None
normalized_repo_path = _normalize_repo_path(repo_path)
return _validate_push(
normalized_push_refs,
Comment on lines +406 to +409
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Reject whitespace-only push_refs input to avoid fallback misrouting.

On Line 406, whitespace-only input is normalized to "" and forwarded. That makes _validate_push treat it as explicit refs (push_upstream_fallback=False) instead of upstream fallback. Please reject empty strings (or coerce them to None) after normalization.

Suggested fix
 def validate_push_safety(
@@
 ) -> dict[str, Any]:
     """Validate that a push is not a force push."""
     normalized_push_refs = push_refs.strip() if isinstance(push_refs, str) else None
+    if isinstance(push_refs, str) and not normalized_push_refs:
+        raise ValueError("push_refs cannot be empty when provided")
     normalized_repo_path = _normalize_repo_path(repo_path)
     return _validate_push(
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
normalized_push_refs = push_refs.strip() if isinstance(push_refs, str) else None
normalized_repo_path = _normalize_repo_path(repo_path)
return _validate_push(
normalized_push_refs,
normalized_push_refs = push_refs.strip() if isinstance(push_refs, str) else None
if isinstance(push_refs, str) and not normalized_push_refs:
raise ValueError("push_refs cannot be empty when provided")
normalized_repo_path = _normalize_repo_path(repo_path)
return _validate_push(
normalized_push_refs,
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/commit_check_mcp/server.py` around lines 406 - 409, The current
normalization sets normalized_push_refs = push_refs.strip() which turns
whitespace-only strings into "" and causes _validate_push to treat them as
explicit refs; change the normalization so that after stripping any string that
becomes empty is coerced to None (e.g., use push_refs.strip() or None) so
normalized_push_refs is None for empty/whitespace-only input before calling
_validate_push; update the assignment that creates normalized_push_refs (and
keep using _normalize_repo_path and the _validate_push call) so downstream logic
uses upstream fallback correctly.

config=_normalize_config(config),
repo_path=normalized_repo_path,
config_path=_normalize_config_path(config_path, normalized_repo_path),
)


@mcp.tool()
def validate_commit_context(
message: str | None = None,
Expand Down Expand Up @@ -423,9 +463,10 @@ def validate_repository_state(
include_message: bool = True,
include_branch: bool = True,
include_author: bool = True,
include_push: bool = False,
) -> dict[str, Any]:
"""Validate the latest commit, current branch, and git author state of a repository."""
if not any([include_message, include_branch, include_author]):
"""Validate the latest commit, branch, author, and optional push safety state."""
if not any([include_message, include_branch, include_author, include_push]):
raise ValueError("At least one validation target must be enabled")

normalized_repo_path = _normalize_repo_path(repo_path)
Expand Down Expand Up @@ -461,6 +502,15 @@ def validate_repository_state(
config_path=normalized_config_path,
)["checks"]
)
if include_push:
checks.extend(
_validate_push(
None,
config=normalized_config,
repo_path=normalized_repo_path,
config_path=normalized_config_path,
)["checks"]
)

return {
"status": "fail" if any(c["status"] == "fail" for c in checks) else "pass",
Expand Down Expand Up @@ -489,7 +539,9 @@ 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)
dict.fromkeys(
entry.check for entry in COMMIT_RULES + BRANCH_RULES + PUSH_RULES
)
),
"enabled_rules": rules,
}
Expand Down
96 changes: 95 additions & 1 deletion tests/test_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,83 @@ def test_validate_commit_context_requires_at_least_one_field() -> None:
server.validate_commit_context()


def test_validate_push_safety_forwards_normalized_values(
monkeypatch: pytest.MonkeyPatch,
) -> None:
captured: dict[str, object] = {}

def fake_validate_push(
push_refs: str | None,
*,
config: dict | None,
repo_path: Path | None,
config_path: str | None,
):
captured["push_refs"] = push_refs
captured["config"] = config
captured["repo_path"] = repo_path
captured["config_path"] = config_path
return {"status": "pass", "checks": []}

monkeypatch.setattr(server, "_validate_push", fake_validate_push)

result = server.validate_push_safety(
" refs/heads/main abc refs/heads/main def ",
{"push": {"allow_force_push": True}},
repo_path=".",
)

assert result["status"] == "pass"
assert captured == {
"push_refs": "refs/heads/main abc refs/heads/main def",
"config": {"push": {"allow_force_push": True}},
"repo_path": Path.cwd().resolve(),
"config_path": None,
}


def test_validate_push_forces_no_force_push_rule(
monkeypatch: pytest.MonkeyPatch,
) -> None:
captured: list[dict[str, object]] = []

def fake_run_checks(check_names, context, config):
captured.append(
{
"check_names": check_names,
"stdin_text": context.stdin_text,
"push_upstream_fallback": context.push_upstream_fallback,
"allow_force_push": config["push"]["allow_force_push"],
}
)
return {"status": "pass", "checks": []}

monkeypatch.setattr(server, "_run_checks", fake_run_checks)

result = server._validate_push(None, config={"push": {"allow_force_push": True}})
empty_refs_result = server._validate_push(
"",
config={"push": {"allow_force_push": True}},
)

assert result["status"] == "pass"
assert empty_refs_result["status"] == "pass"
assert captured == [
{
"check_names": ["no_force_push"],
"stdin_text": None,
"push_upstream_fallback": True,
"allow_force_push": False,
},
{
"check_names": ["no_force_push"],
"stdin_text": "",
"push_upstream_fallback": False,
"allow_force_push": False,
},
]


def test_validate_author_info_forwards_normalized_values(
monkeypatch: pytest.MonkeyPatch,
) -> None:
Expand Down Expand Up @@ -132,6 +209,7 @@ def test_validate_repository_state_requires_one_enabled_target() -> None:
include_message=False,
include_branch=False,
include_author=False,
include_push=False,
)


Expand All @@ -158,16 +236,31 @@ def fake_validate_branch(
assert branch is None
return {"status": "pass", "checks": [{"check": "branch", "status": "pass"}]}

def fake_validate_push(
push_refs: str | None,
*,
config: dict | None,
repo_path: Path | None,
config_path: str | None,
):
assert push_refs is None
return {
"status": "pass",
"checks": [{"check": "no_force_push", "status": "pass"}],
}

monkeypatch.setattr(server, "_validate_message", fake_validate_message)
monkeypatch.setattr(server, "_validate_branch", fake_validate_branch)
monkeypatch.setattr(server, "_validate_push", fake_validate_push)

result = server.validate_repository_state(include_author=False)
result = server.validate_repository_state(include_author=False, include_push=True)

assert result == {
"status": "pass",
"checks": [
{"check": "message", "status": "pass"},
{"check": "branch", "status": "pass"},
{"check": "no_force_push", "status": "pass"},
],
}

Expand All @@ -192,5 +285,6 @@ def test_describe_validation_rules_includes_loaded_config(tmp_path: Path) -> Non
assert result["config"]["commit"]["require_body"] is True
assert result["config"]["branch"]["allow_branch_names"] == ["develop"]
assert "message" in result["supported_checks"]
assert "no_force_push" in result["supported_checks"]
assert result["supported_checks"].count("ignore_authors") == 1
assert any(rule["check"] == "require_body" for rule in result["enabled_rules"])
6 changes: 3 additions & 3 deletions uv.lock

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