diff --git a/README.md b/README.md index 2b21570..f053535 100644 --- a/README.md +++ b/README.md @@ -19,7 +19,7 @@ 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_push_safety` — validates that a push is not a force push (force pushes are always rejected by this tool) - `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, author state, and optional push safety for a repo @@ -33,17 +33,22 @@ All validation tools return the same structured commit-check result shape: "warnings": 0, "checks": [ { + "rule_id": "CC001", "check": "message", "status": "pass|fail|warn|skip", "value": "...", "error": "...", "suggest": "...", - "fix": "..." + "fix": "...", + "docs_url": "https://commit-check.com/rules/#cc001" } ] } ``` +`rule_id` is the stable id of the rule that produced the check and `docs_url` +links to its documentation. + 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 @@ -59,9 +64,14 @@ a non-empty `fix` as it stands and fall back to `suggest` when it is 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 state (see the `repo_path` note under [Tool Usage](#tool-usage)), a malformed or rejected -commit-check config — is returned as an MCP tool error (`is_error`) whose text -names the problem, for example `repo_path is not a git repository: /path/to/dir` -or `invalid commit-check config: ...`, rather than as a `pass`/`fail` result. +commit-check config, a `push_refs` SHA that is not a commit in `repo_path` even +after the force-push check tried to fetch it — is returned as an MCP tool error +(`is_error`) whose text names the problem, for example +`repo_path is not a git repository: /path/to/dir`, +`invalid commit-check config: ...` or +`push_refs: is not a commit in the repository; fetch it first, the force-push check cannot be judged`, +rather than as a `pass`/`fail` result. In particular a push whose SHAs cannot +be judged is never reported as a pass. ## Installation @@ -266,10 +276,26 @@ After the client starts the server, it will expose these tools: - `validate_repository_state(repo_path?, config?, config_path?, include_message?, include_branch?, include_author?, include_push?)` - `describe_validation_rules(config?, repo_path?, config_path?)` +Every parameter carries a description in the tool's JSON input schema, so an +MCP client (and the model behind it) can see what each one expects without +reading this file: for example `push_refs` documents the git pre-push line +format ` `. Each tool also has +a display `title` and MCP tool annotations: `destructiveHint: false` and +`idempotentHint: true` everywhere, `readOnlyHint: true` on the six tools that +only read, and `readOnlyHint: false` with `openWorldHint: true` on +`validate_push_safety` and `validate_repository_state`, because the force-push +check may run `git fetch` to resolve a SHA, which updates `FETCH_HEAD` and +remote-tracking refs (the working tree and commits are never touched). Clients +that gate tool calls on those hints can auto-approve the read-only six. The +server's `instructions` describe the intended +loop: validate first, read `status` (only `fail` rejects, `skip` is not +approval), apply a non-empty `fix` verbatim or follow `suggest`, then validate +again. + The common optional arguments are: -- `repo_path`: repository directory to validate against; it must be a git repository when the tool reads git state (branch, author, or push refs omitted, or `validate_repository_state`), and may be a plain directory holding a config file when every value is supplied -- `config_path`: explicit TOML config file; relative paths resolve from `repo_path` +- `repo_path`: repository directory to validate against; it must be a git repository when the tool reads git state (branch, author, or push refs omitted, `validate_repository_state`, or `push_refs` given, whose SHAs must resolve there), and may be a plain directory holding a config file when every other value is supplied +- `config_path`: explicit TOML config file, used instead of the repository's own `cchk.toml`/`commit-check.toml`; relative paths resolve from `repo_path` - `config`: ad-hoc config overrides merged on top of defaults and repo config ## Common Examples @@ -355,9 +381,8 @@ Example payload for a repository-wide validation: Config precedence is: 1. `commit-check` built-in defaults -2. repository config loaded from `repo_path` -3. `config_path` when explicitly provided -4. inline `config` overrides passed to the tool +2. repository config loaded from `repo_path`, or the file named by `config_path` when it is provided (it replaces the repository's own config file) +3. inline `config` overrides passed to the tool ## Published On diff --git a/src/commit_check_mcp/server.py b/src/commit_check_mcp/server.py index 60e5c4b..84ce307 100644 --- a/src/commit_check_mcp/server.py +++ b/src/commit_check_mcp/server.py @@ -2,12 +2,14 @@ from __future__ import annotations +from collections.abc import Callable from contextlib import contextmanager from importlib.metadata import version from pathlib import Path +import inspect import os import subprocess -from typing import Any +from typing import Annotated, Any, TypeVar from commit_check import __version__ as commit_check_version from commit_check.config_merger import deep_merge, get_default_config, load_toml_config @@ -22,17 +24,166 @@ from commit_check.rules_catalog import BRANCH_RULES, COMMIT_RULES, PUSH_RULES from mcp.server.mcpserver import MCPServer from mcp.server.mcpserver.exceptions import ToolError +from mcp.types import ToolAnnotations +from pydantic import Field from . import __version__ -mcp = MCPServer( - "commit-check-mcp", - instructions=( - "Use these tools to validate commit messages, branch names, author metadata, " - "and push safety with commit-check." - ), +INSTRUCTIONS = ( + "commit-check-mcp validates commit messages, branch names, author metadata and push safety " + "against commit-check rules. Every tool leaves your working tree and commits untouched; the " + "two push checks may run git fetch to resolve SHAs.\n" + "Workflow: (1) validate FIRST, before you commit or push, with the matching validate_* tool " + "(pass repo_path so the repository's own cchk.toml/commit-check.toml is used). " + "(2) Read the top-level status: only 'fail' is a rejection; 'skip' means nothing was validated " + "(do not treat it as approval); 'warn' checks are reported but do not fail the run. " + "(3) On 'fail', for each check with status 'fail': if its fix is non-empty, apply fix " + "verbatim; otherwise rewrite the value following suggest (rule_id and docs_url point at the " + "rule's docs). " + "(4) Validate AGAIN with the corrected value and repeat until status is 'pass'. " + "Use describe_validation_rules to see which rules are enabled before guessing at a format." +) + +mcp = MCPServer("commit-check-mcp", instructions=INSTRUCTIONS, version=__version__) + +# The result every validate_* tool returns, described once and spliced into each +# tool description where its docstring says ``{result_shape}``. +RESULT_SHAPE = ( + "Returns {status, warnings, checks[]}. status is 'pass', 'fail' or 'skip': only 'fail' is a " + "rejection; 'skip' means every check skipped, so nothing was validated and the result is not " + "approval. warnings is the number of checks with status 'warn'. Each check has: rule_id " + "(stable rule id, e.g. 'CC001'); check (rule name, e.g. 'message'); status 'pass' | 'fail' | " + "'warn' | 'skip' ('warn' = the config lists the check under warn, so the finding is reported " + "without failing the run; 'skip' = the rule did not run, e.g. the author is in ignore_authors " + "or there was nothing to check); value (what was checked); error (why it failed); suggest " + "(advice for a person); fix (the corrected value when the correction is unambiguous, else ''); " + "docs_url (documentation for the rule). On 'fail', apply a non-empty fix verbatim; when fix is " + "'', rewrite following suggest; then validate again." ) +_F = TypeVar("_F", bound=Callable[..., Any]) + + +def _tool(title: str, *, fetches: bool = False) -> Callable[[_F], _F]: + """Register a commit-check tool with the hints every one of them shares. + + No tool changes the working tree or the commits, and repeating a call has + no further effect, so every tool is idempotent and none is destructive. + ``fetches`` marks the two that run the force-push check: to resolve a SHA it + does not know locally it may run ``git fetch``, which reaches the network + and updates FETCH_HEAD and remote-tracking refs, so those two are neither + read-only nor closed-world. The function docstring becomes the tool + description, with ``{result_shape}`` replaced by :data:`RESULT_SHAPE`. + """ + annotations = ToolAnnotations( + readOnlyHint=not fetches, + destructiveHint=False, + idempotentHint=True, + openWorldHint=fetches, + ) + + def register(fn: _F) -> _F: + description = inspect.cleandoc(fn.__doc__ or "").replace("{result_shape}", RESULT_SHAPE) + return mcp.tool(title=title, description=description, annotations=annotations)(fn) + + return register + + +# Parameter types shared by the tools; the Field description reaches the tool's +# JSON schema, which is what an MCP client shows the model. +ConfigParam = Annotated[ + dict[str, Any] | None, + Field( + description=( + "Inline commit-check config overrides as a JSON object, merged on top of the built-in " + 'defaults and any config file, e.g. {"warn": ["message"]} or ' + '{"commit": {"require_body": true}}.' + ) + ), +] +RepoPathParam = Annotated[ + str | None, + Field( + description=( + "Path to the git repository to validate against. Its cchk.toml or commit-check.toml " + "(also looked up under .github/) is loaded, and a relative config_path is resolved " + "from it. Omit to use the server's working directory." + ) + ), +] +ConfigPathParam = Annotated[ + str | None, + Field( + description=( + "Path to a commit-check TOML config file, used instead of the repository's own " + "cchk.toml/commit-check.toml; a relative path is resolved from repo_path." + ) + ), +] +MessageParam = Annotated[ + str, + Field( + description=( + "Full commit message text to validate: subject line, optional blank line, body " + "(and trailers such as Signed-off-by). Must be non-empty." + ) + ), +] +OptionalMessageParam = Annotated[ + str | None, + Field( + description=( + "Commit message text to validate: subject line, optional blank line, body. Omit to " + "skip the message checks. Must be non-empty when provided." + ) + ), +] +BranchParam = Annotated[ + str | None, + Field( + description=( + "Branch name to validate, e.g. 'feature/login'. Omit to validate the branch currently " + "checked out in repo_path, which must then be a git repository. Must be non-empty when " + "provided." + ) + ), +] +AuthorNameParam = Annotated[ + str | None, + Field( + description=( + "Author name to validate, e.g. 'Alice Example'. Omit to read it from repo_path " + "(git config user.name, falling back to the latest commit's author). Must be " + "non-empty when provided." + ) + ), +] +AuthorEmailParam = Annotated[ + str | None, + Field( + description=( + "Author email to validate, e.g. 'alice@example.com'. Omit to read it from repo_path " + "(git config user.email, falling back to the latest commit's author). Must be " + "non-empty when provided." + ) + ), +] +PushRefsParam = Annotated[ + str | None, + Field( + description=( + "Refs about to be pushed, in git pre-push hook stdin format, one ref per line: " + "' ', e.g. " + "'refs/heads/main 1a2b3c... refs/heads/main 9f8e7d...'. A remote_sha of 40 zeros " + "means a new branch (never a force push). Every other SHA must resolve to a commit in " + "repo_path (the check runs git merge-base and may fetch the remote ref first); a SHA " + "that still cannot be resolved is a tool error, not a pass, so fetch it first. Omit to " + "check the current branch of repo_path against its upstream instead. Must be non-empty " + "when provided." + ) + ), +] + def _normalize_config(config: dict[str, Any] | None) -> dict[str, Any] | None: """Ensure tool config input is JSON-object-like.""" @@ -237,6 +388,43 @@ def _validate_branch( ) +ZERO_SHA = "0" * 40 + + +def _require_push_shas_resolvable(push_refs: str) -> None: + """Fail when a pushed SHA is not a commit in the current working directory. + + The force-push rule asks ``git merge-base`` whether the remote SHA is an + ancestor of the local one. When either SHA is unknown, git exits 128 and, + after commit-check's own attempt to fetch the remote ref, the rule falls + through to PASS. Nothing was judged in that case, so the pass must not + reach the caller. This runs after the rule, so a SHA the rule's fetch + brought in counts as resolved. The 40-zero placeholder for a new branch is + not a commit and is skipped. + """ + for line in push_refs.splitlines(): + parts = line.split() + if len(parts) < 4: + continue + for sha in (parts[1], parts[3]): + if sha == ZERO_SHA: + continue + try: + result = subprocess.run( + ["git", "cat-file", "-e", f"{sha}^{{commit}}"], + capture_output=True, + text=True, + check=False, + ) + except OSError as e: + raise ToolError(f"git is not available to inspect push_refs: {e}") from e + if result.returncode != 0: + raise ToolError( + f"push_refs: {sha} is not a commit in the repository; fetch it first, " + "the force-push check cannot be judged" + ) + + def _validate_push( push_refs: str | None = None, *, @@ -244,11 +432,17 @@ def _validate_push( repo_path: Path | None = None, config_path: str | None = None, ) -> dict[str, Any]: - """Validate push ref updates against commit-check force-push protection.""" + """Validate push ref updates against commit-check force-push protection. + + With explicit ``push_refs``, a pass is only returned once every SHA in + them has been confirmed to be a commit in the repository; the + upstream-fallback path (``push_refs`` is ``None``) reads HEAD and the + upstream ref, which always resolve. + """ 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( + result = _run_checks( ["no_force_push"], ValidationContext( stdin_text=push_refs, @@ -257,6 +451,9 @@ def _validate_push( ), cfg, ) + if push_refs and result["status"] != "fail": + _require_push_shas_resolvable(push_refs) + return result def _validate_author( @@ -364,9 +561,13 @@ def _validate_all( return _summarize(checks) -@mcp.tool() +@_tool("Server health") def server_health() -> dict[str, str]: - """Return server and dependency versions. Read-only, no side effects. Returns dict with server name, server version, commit-check version, and MCP SDK version. Useful as a first call to verify the server is running and check version compatibility.""" + """Return server and dependency versions. Read-only, no side effects. + + Returns {server, server_version, commit_check_version, mcp_sdk_version}. Useful as a first call + to verify the server is running and to check version compatibility. + """ return { "server": "commit-check-mcp", "server_version": __version__, @@ -375,22 +576,22 @@ def server_health() -> dict[str, str]: } -@mcp.tool() +@_tool("Validate commit message") def validate_commit_message( - message: str, - config: dict[str, Any] | None = None, - repo_path: str | None = None, - config_path: str | None = None, + message: MessageParam, + config: ConfigParam = None, + repo_path: RepoPathParam = None, + config_path: ConfigPathParam = 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), 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. + """Validate a commit message against commit-check rules (Conventional Commits type and format, + subject length and case, body, sign-off, WIP/fixup markers, AI attribution: whatever the + effective config enables). Read-only; touches no git state. - 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. + {result_shape} - Parameters: - - message (required): The commit message text to validate. - - config (optional): Inline JSON config overrides on top of any loaded config file. - - repo_path (optional): Path to the git repository for repo-relative config loading. - - config_path (optional): Path to a custom commit-check TOML config file. + Use this when you have one commit message string to check before committing. To check message, + branch and author in one call use validate_commit_context; to check the latest commit already + in a repository use validate_repository_state. """ if not isinstance(message, str) or not message.strip(): raise ToolError("message must be a non-empty string") @@ -403,22 +604,20 @@ def validate_commit_message( ) -@mcp.tool() +@_tool("Validate branch name") def validate_branch_name( - branch: str | None = None, - config: dict[str, Any] | None = None, - repo_path: str | None = None, - config_path: str | None = None, + branch: BranchParam = None, + config: ConfigParam = None, + repo_path: RepoPathParam = None, + config_path: ConfigPathParam = 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, and fix: the corrected value when unambiguous, else empty). + """Validate a branch name against the configured naming convention (e.g. feature/*, bugfix/*) + and, when configured, that the branch is based on the required merge base. Read-only. - 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. + {result_shape} - Parameters: - - branch (optional): The branch name to validate. If omitted, detected from the current repo. - - config (optional): Inline JSON config overrides. - - repo_path (optional): Path to the git repository. - - config_path (optional): Path to a custom commit-check TOML config file. + Use this before creating or pushing a branch. Omit branch to check the branch currently checked + out in repo_path. For combined message+branch+author validation use validate_commit_context. """ normalized_branch = branch.strip() if isinstance(branch, str) else None if isinstance(branch, str) and not normalized_branch: @@ -434,24 +633,22 @@ def validate_branch_name( ) -@mcp.tool() +@_tool("Validate author info") def validate_author_info( - author_name: str | None = None, - author_email: str | None = None, - config: dict[str, Any] | None = None, - repo_path: str | None = None, - config_path: str | None = None, + author_name: AuthorNameParam = None, + author_email: AuthorEmailParam = None, + config: ConfigParam = None, + repo_path: RepoPathParam = None, + config_path: ConfigPathParam = 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, and fix: the corrected value when unambiguous, else empty). + """Validate a commit author's name and/or email against the configured rules (e.g. allowed + email domains, name patterns). Read-only. - 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. + {result_shape} - Parameters: - - author_name (optional): The author name to validate. - - author_email (optional): The author email to validate. - - config (optional): Inline JSON config overrides. - - repo_path (optional): Path to the git repository. - - config_path (optional): Path to a custom commit-check TOML config file. + Use this to check author metadata before committing. Only the values you pass are checked; if + neither is given, both are read from repo_path's git config (falling back to the latest commit) + and repo_path must be a git repository. For combined validation use validate_commit_context. """ normalized_name = author_name.strip() if isinstance(author_name, str) else None normalized_email = author_email.strip() if isinstance(author_email, str) else None @@ -473,22 +670,25 @@ def validate_author_info( ) -@mcp.tool() +@_tool("Validate push safety", fetches=True) 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, + push_refs: PushRefsParam = None, + config: ConfigParam = None, + repo_path: RepoPathParam = None, + config_path: ConfigPathParam = 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, 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. - - Parameters: - - push_refs (optional): The push ref specification to validate. If omitted, checks upstream fallback state. Must be non-empty when provided. - - config (optional): Inline JSON config overrides. - - repo_path (optional): Path to the git repository. - - config_path (optional): Path to a custom commit-check TOML config file. + """Check that a pending push is not a force push (rule CC301, no_force_push): fails when a + remote_sha is not an ancestor of its local_sha, i.e. the push would rewrite remote history. + Leaves your working tree and commits untouched, but may run `git fetch ` to + resolve SHAs, which updates FETCH_HEAD and remote-tracking refs. A SHA that cannot be resolved + even then is a tool error, never a pass. Force pushes are always rejected by this tool; + push.allow_force_push in config cannot re-enable them here. + + {result_shape} + + Call this before `git push`. Only the no_force_push rule runs. When it fails there is no + automatic fix (fix is ''): follow suggest, i.e. push without --force/--force-with-lease or + rebase onto the remote first, then validate again. """ normalized_push_refs = push_refs.strip() if isinstance(push_refs, str) else None if isinstance(push_refs, str) and not normalized_push_refs: @@ -504,28 +704,25 @@ def validate_push_safety( ) -@mcp.tool() +@_tool("Validate commit context") def validate_commit_context( - message: str | None = None, - branch: str | None = None, - author_name: str | None = None, - author_email: str | None = None, - config: dict[str, Any] | None = None, - repo_path: str | None = None, - config_path: str | None = None, + message: OptionalMessageParam = None, + branch: BranchParam = None, + author_name: AuthorNameParam = None, + author_email: AuthorEmailParam = None, + config: ConfigParam = None, + repo_path: RepoPathParam = None, + config_path: ConfigPathParam = 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, 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. - - Parameters: - - message (optional): Commit message text to validate. - - branch (optional): Branch name to validate. - - author_name (optional): Author name to validate. - - author_email (optional): Author email to validate. - - config (optional): Inline JSON config overrides on top of any loaded config file. - - repo_path (optional): Path to the git repository for repo-relative config loading. - - config_path (optional): Path to a custom commit-check TOML config file. + """Run the commit message, branch name and author checks together in one call, for whichever of + message, branch, author_name and author_email you pass. Read-only. + + {result_shape} + + Use this to validate several aspects of a commit you are about to make with a single call. At + least one of message, branch, author_name or author_email is required; omitted aspects are not + checked. For one aspect use validate_commit_message, validate_branch_name or + validate_author_info; for the commit already at HEAD use validate_repository_state. """ normalized_message = message.strip() if isinstance(message, str) else None normalized_branch = branch.strip() if isinstance(branch, str) else None @@ -558,28 +755,59 @@ def validate_commit_context( ) -@mcp.tool() +@_tool("Validate repository state", fetches=True) def validate_repository_state( - repo_path: str | None = None, - config: dict[str, Any] | None = None, - config_path: str | None = None, - include_message: bool = True, - include_branch: bool = True, - include_author: bool = True, - include_push: bool = False, + repo_path: Annotated[ + str | None, + Field( + description=( + "Path to the git repository to inspect; its cchk.toml or commit-check.toml is " + "loaded and a relative config_path is resolved from it. Omit to use the server's " + "working directory, which must then be a git repository." + ) + ), + ] = None, + config: ConfigParam = None, + config_path: ConfigPathParam = None, + include_message: Annotated[ + bool, + Field(description="Validate the message of the latest commit (HEAD). Default true."), + ] = True, + include_branch: Annotated[ + bool, + Field(description="Validate the name of the currently checked-out branch. Default true."), + ] = True, + include_author: Annotated[ + bool, + Field( + description=( + "Validate the author name and email of the latest commit (falling back to git " + "config user.name/user.email). Default true." + ) + ), + ] = True, + include_push: Annotated[ + bool, + Field( + description=( + "Also check that pushing the current branch to its upstream would not be a " + "force push; may run git fetch (updating FETCH_HEAD), and passes when the branch " + "has no upstream. Default false." + ) + ), + ] = False, ) -> dict[str, Any]: - """Validate the current repository state including latest commit message, active branch, author metadata, and optional push safety. Read-only validation. Reads git data (message, branch, author) from the local repository. Returns a structured result with overall status and per-check results. - - Use this to validate the entire state of a local git repository in one call — ideal for pre-commit or CI hooks. Controls which checks run via boolean include_* flags. For validating arbitrary (non-repo) values, use validate_commit_context or individual validation tools instead. - - Parameters: - - repo_path (optional): Path to the git repository. If omitted, uses current working directory. - - config (optional): Inline JSON config overrides on top of any loaded config file. - - config_path (optional): Path to a custom commit-check TOML config file. - - include_message (optional, default true): Whether to validate the latest commit message. - - include_branch (optional, default true): Whether to validate the current branch name. - - include_author (optional, default true): Whether to validate the latest commit author. - - include_push (optional, default false): Whether to validate push safety. + """Validate what is already in a local git repository: the latest commit's message and author, + the checked-out branch name and, optionally, whether pushing that branch to its upstream would + be a force push. Leaves the working tree and commits untouched; the push check may run + `git fetch`, which updates FETCH_HEAD and remote-tracking refs. + + {result_shape} + + Use this to check a repository's current state in one call, e.g. after committing and before + pushing, or in a hook. The include_* flags select the checks; at least one must be true. To + validate values that are not yet committed use validate_commit_context or the single-aspect + tools. """ if not any([include_message, include_branch, include_author, include_push]): raise ToolError("At least one validation target must be enabled") @@ -631,20 +859,22 @@ def validate_repository_state( return _summarize(checks) -@mcp.tool() +@_tool("Describe validation rules") def describe_validation_rules( - config: dict[str, Any] | None = None, - repo_path: str | None = None, - config_path: str | None = None, + config: ConfigParam = None, + repo_path: RepoPathParam = None, + config_path: ConfigPathParam = None, ) -> dict[str, Any]: - """Return enabled commit-check rules after merging defaults, repo config, and inline overrides. Read-only, no side effects. Returns a dict with commit_check_version, the full merged config, supported check types, and enabled rules (each with check name, config, and pattern details). + """Return the commit-check rules that are in effect after merging the built-in defaults, the + repository's config file (or config_path) and the inline config overrides. Read-only, no side + effects. - Use this to inspect which validation rules are currently active before running any validation. Helps debug rule configuration and check which checks will be applied. + Returns {commit_check_version, config (the merged config), supported_checks (every check name + commit-check knows), enabled_rules[]} where each enabled rule carries its check name, config + and pattern details. - Parameters: - - config (optional): Inline JSON config overrides on top of any loaded config file. - - repo_path (optional): Path to the git repository for repo-relative config loading. - - config_path (optional): Path to a custom commit-check TOML config file. + Use this before writing a commit message or branch name to learn the expected format instead + of guessing, and to debug why a validation failed or was skipped. """ normalized_repo_path = _normalize_repo_path(repo_path) normalized_config = _normalize_config(config) diff --git a/tests/test_server.py b/tests/test_server.py index fc8f9cb..83e2b82 100644 --- a/tests/test_server.py +++ b/tests/test_server.py @@ -208,11 +208,11 @@ def test_called_with_arguments(self) -> None: # --------------------------------------------------------------------------- class TestValidatePush: - def test_no_force_push_rule_is_enforced(self) -> None: - result = server._validate_push("refs/heads/main abc refs/heads/main def") - # The rule is enabled, but depending on context it may pass or fail - assert "status" in result - assert isinstance(result["checks"], list) + def test_unresolvable_shas_are_an_error_not_a_pass(self) -> None: + # git merge-base exits 128 on SHAs it does not know and the rule falls + # through to PASS; the server must not hand that pass to the caller. + with pytest.raises(ToolError, match="push_refs: abc is not a commit in the repository"): + server._validate_push("refs/heads/main abc refs/heads/main def") def test_with_push_refs_none_and_patch(self, monkeypatch: pytest.MonkeyPatch) -> None: captured: list[dict] = [] @@ -1194,3 +1194,177 @@ def test_blank_push_refs_raises(self) -> None: def test_blank_push_refs_via_tool_call(self) -> None: with pytest.raises(ToolError, match="push_refs cannot be empty when provided"): _call_tool("validate_push_safety", push_refs=" ") + + +# --------------------------------------------------------------------------- +# What an MCP client is told about the tools: titles, annotations, schema +# descriptions, server version and instructions +# --------------------------------------------------------------------------- + +FETCHING_TOOLS = {"validate_push_safety", "validate_repository_state"} + + +def _list_tools() -> list: + """List tools the way an MCP client does, through the server's list path.""" + return asyncio.run(server.mcp.list_tools()) + + +class TestToolMetadata: + def test_every_tool_has_a_title_and_non_destructive_annotations(self) -> None: + tools = _list_tools() + assert len(tools) == 8 + for tool in tools: + assert tool.title, tool.name + assert tool.annotations is not None, tool.name + assert tool.annotations.destructive_hint is False, tool.name + assert tool.annotations.idempotent_hint is True, tool.name + + def test_only_the_tools_that_may_fetch_are_not_read_only(self) -> None: + # The force-push check may run git fetch, which writes FETCH_HEAD and + # remote-tracking refs, so those two tools cannot claim to be read-only. + seen = set() + for tool in _list_tools(): + fetches = tool.name in FETCHING_TOOLS + assert tool.annotations.read_only_hint is (not fetches), tool.name + assert tool.annotations.open_world_hint is fetches, tool.name + if fetches: + seen.add(tool.name) + assert seen == FETCHING_TOOLS + + def test_every_parameter_has_a_description(self) -> None: + seen = 0 + for tool in _list_tools(): + for name, prop in tool.input_schema.get("properties", {}).items(): + seen += 1 + assert prop.get("description"), f"{tool.name}.{name} has no description" + assert seen > 0 + + def test_push_refs_description_explains_the_pre_push_line_format(self) -> None: + tool = next(t for t in _list_tools() if t.name == "validate_push_safety") + description = tool.input_schema["properties"]["push_refs"]["description"] + assert " " in description + assert "40 zeros" in description + assert "upstream" in description + assert "non-empty when provided" in description + assert "tool error, not a pass" in description + + def test_validation_tools_describe_the_result_shape(self) -> None: + validators = [t for t in _list_tools() if t.name.startswith("validate_")] + assert len(validators) == 6 + for tool in validators: + for term in ("rule_id", "docs_url", "'warn'", "'skip'", "fix", "suggest"): + assert term in tool.description, f"{tool.name} does not mention {term}" + + def test_push_safety_says_force_pushes_are_always_rejected(self) -> None: + tool = next(t for t in _list_tools() if t.name == "validate_push_safety") + assert "always rejected" in tool.description + assert "configure via" not in tool.description + + def test_server_reports_its_version(self) -> None: + assert server.mcp.version == server.__version__ + assert server.mcp.version != "" + + def test_instructions_describe_the_validate_fix_workflow(self) -> None: + instructions = server.mcp.instructions + assert instructions is not None + assert "validate" in instructions + assert "fix" in instructions + assert "describe_validation_rules" in instructions + assert "read-only" not in instructions + + +# --------------------------------------------------------------------------- +# validate_push_safety: a push whose SHAs cannot be judged is not a pass +# --------------------------------------------------------------------------- + +FAKE_SHA_A = "0123456789abcdef0123456789abcdef01234567" +FAKE_SHA_B = "fedcba9876543210fedcba9876543210fedcba98" +ZERO_SHA = "0" * 40 + + +def _rev(repo: Path, ref: str) -> str: + return subprocess.run( + ["git", "rev-parse", ref], cwd=repo, check=True, capture_output=True, text=True + ).stdout.strip() + + +def _repo_with_two_commits(root: Path) -> Path: + repo = _repo_with_commit(root, "feat: first") + (repo / "file.txt").write_text("more\n") + _git(repo, "commit", "-q", "-am", "feat: second") + return repo + + +class TestPushRefsMustResolve: + def test_fake_shas_are_a_tool_error(self, tmp_path: Path) -> None: + repo = _repo_with_two_commits(tmp_path) + with pytest.raises(ToolError, match=f"push_refs: {FAKE_SHA_A} is not a commit"): + _call_tool( + "validate_push_safety", + push_refs=f"refs/heads/main {FAKE_SHA_A} refs/heads/main {FAKE_SHA_B}", + repo_path=str(repo), + ) + + def test_fake_remote_sha_alone_is_a_tool_error(self, tmp_path: Path) -> None: + repo = _repo_with_two_commits(tmp_path) + head = _rev(repo, "HEAD") + with pytest.raises(ToolError, match=f"push_refs: {FAKE_SHA_B} is not a commit"): + server.validate_push_safety( + push_refs=f"refs/heads/main {head} refs/heads/main {FAKE_SHA_B}", + repo_path=str(repo), + ) + + def test_fast_forward_pair_passes(self, tmp_path: Path) -> None: + repo = _repo_with_two_commits(tmp_path) + head, parent = _rev(repo, "HEAD"), _rev(repo, "HEAD~1") + result = server.validate_push_safety( + push_refs=f"refs/heads/main {head} refs/heads/main {parent}", + repo_path=str(repo), + ) + assert result["status"] == "pass" + assert [c["check"] for c in result["checks"]] == ["no_force_push"] + + def test_rewriting_remote_history_fails(self, tmp_path: Path) -> None: + repo = _repo_with_two_commits(tmp_path) + head, parent = _rev(repo, "HEAD"), _rev(repo, "HEAD~1") + result = server.validate_push_safety( + push_refs=f"refs/heads/main {parent} refs/heads/main {head}", + repo_path=str(repo), + ) + assert result["status"] == "fail" + assert result["checks"][0]["check"] == "no_force_push" + + def test_new_branch_zero_remote_sha_passes(self, tmp_path: Path) -> None: + repo = _repo_with_two_commits(tmp_path) + head = _rev(repo, "HEAD") + result = server.validate_push_safety( + push_refs=f"refs/heads/topic {head} refs/heads/topic {ZERO_SHA}", + repo_path=str(repo), + ) + assert result["status"] == "pass" + + def test_engine_failure_wins_over_resolvability(self, tmp_path: Path) -> None: + # The first line is a real force push; the engine reports fail and the + # unresolvable SHA on the second line does not turn that into an error. + repo = _repo_with_two_commits(tmp_path) + head, parent = _rev(repo, "HEAD"), _rev(repo, "HEAD~1") + result = server.validate_push_safety( + push_refs=( + f"refs/heads/main {parent} refs/heads/main {head}\n" + f"refs/heads/other {FAKE_SHA_A} refs/heads/other {FAKE_SHA_B}" + ), + repo_path=str(repo), + ) + assert result["status"] == "fail" + + def test_repository_state_include_push_is_unaffected(self, tmp_path: Path) -> None: + repo = _repo_with_two_commits(tmp_path) + result = server.validate_repository_state( + repo_path=str(repo), + include_message=False, + include_branch=False, + include_author=False, + include_push=True, + ) + assert result["status"] == "pass" + assert [c["check"] for c in result["checks"]] == ["no_force_push"]