Skip to content

fix+docs(git): consistent error idiom for git_branch + expanded tool descriptions #4048

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
yakuphanycl wants to merge 1 commit into from
+74 −13
Closed

fix+docs(git): consistent error idiom for git_branch + expanded tool descriptions #4048

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
77 changes: 64 additions & 13 deletions src/git/src/mcp_server_git/server.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,10 @@ def git_branch(repo: git.Repo, branch_type: str, contains: str | None = None, no
case 'all':
b_type = "-a"
case _:
return f"Invalid branch type: {branch_type}"
# Surface invalid input via exception, consistent with the rest
# of this module (git_diff, git_checkout, git_show, git_create_branch,
# git_log all raise on invalid input rather than returning a string).
raise ValueError(f"Invalid branch type: '{branch_type}' - must be 'local', 'remote', or 'all'")

# None value will be auto deleted by GitPython
branch_info = repo.git.branch(b_type, *contains_sha, *not_contains_sha)
Expand All @@ -308,7 +311,11 @@ async def list_tools() -> list[Tool]:
return [
Tool(
name=GitTools.STATUS,
description="Shows the working tree status",
description=(
"Show the working tree status (modified, staged, and untracked files). "
"Use before staging or committing to see what has changed. "
"Returns the git status output as text."
),
inputSchema=GitStatus.model_json_schema(),
annotations=ToolAnnotations(
readOnlyHint=True,
Expand All @@ -319,7 +326,11 @@ async def list_tools() -> list[Tool]:
),
Tool(
name=GitTools.DIFF_UNSTAGED,
description="Shows changes in the working directory that are not yet staged",
description=(
"Show changes in the working directory that are not yet staged. "
"Use to review modifications before running git_add. "
"Returns a unified diff as text (empty string if there are no unstaged changes)."
),
inputSchema=GitDiffUnstaged.model_json_schema(),
annotations=ToolAnnotations(
readOnlyHint=True,
Expand All @@ -330,7 +341,11 @@ async def list_tools() -> list[Tool]:
),
Tool(
name=GitTools.DIFF_STAGED,
description="Shows changes that are staged for commit",
description=(
"Show changes that are staged for commit. "
"Use to review what git_commit would record before committing. "
"Returns a unified diff as text (empty string if nothing is staged)."
),
inputSchema=GitDiffStaged.model_json_schema(),
annotations=ToolAnnotations(
readOnlyHint=True,
Expand All @@ -341,7 +356,11 @@ async def list_tools() -> list[Tool]:
),
Tool(
name=GitTools.DIFF,
description="Shows differences between branches or commits",
description=(
"Show differences between the working tree and a target ref (branch, tag, or commit hash). "
"Use to compare the current state against another branch or a specific commit. "
"Returns a unified diff as text. Refs starting with '-' are rejected (flag-injection guard)."
),
inputSchema=GitDiff.model_json_schema(),
annotations=ToolAnnotations(
readOnlyHint=True,
Expand All @@ -352,7 +371,11 @@ async def list_tools() -> list[Tool]:
),
Tool(
name=GitTools.COMMIT,
description="Records changes to the repository",
description=(
"Record currently staged changes as a new commit with the given message. "
"Use after git_add to finalise a snapshot. "
"Returns a confirmation string including the new commit hash."
),
inputSchema=GitCommit.model_json_schema(),
annotations=ToolAnnotations(
readOnlyHint=False,
Expand All @@ -363,7 +386,11 @@ async def list_tools() -> list[Tool]:
),
Tool(
name=GitTools.ADD,
description="Adds file contents to the staging area",
description=(
"Stage file contents from the working directory for the next commit. "
"Pass [\".\"] to stage everything, or specific paths to stage individual files. "
"Use before git_commit. Returns a confirmation that files were staged."
),
inputSchema=GitAdd.model_json_schema(),
annotations=ToolAnnotations(
readOnlyHint=False,
Expand All @@ -374,7 +401,11 @@ async def list_tools() -> list[Tool]:
),
Tool(
name=GitTools.RESET,
description="Unstages all staged changes",
description=(
"Unstage all currently staged changes (working-tree files are left untouched). "
"Use to undo a git_add before committing. "
"Returns a confirmation that staging was reset."
),
inputSchema=GitReset.model_json_schema(),
annotations=ToolAnnotations(
readOnlyHint=False,
Expand All @@ -385,7 +416,11 @@ async def list_tools() -> list[Tool]:
),
Tool(
name=GitTools.LOG,
description="Shows the commit logs",
description=(
"List recent commits, optionally filtered by date range. "
"Use to inspect history, find recent activity, or scope commits to a time window. "
"Returns a list of commit entries each containing hash, author, date, and message."
),
inputSchema=GitLog.model_json_schema(),
annotations=ToolAnnotations(
readOnlyHint=True,
Expand All @@ -396,7 +431,11 @@ async def list_tools() -> list[Tool]:
),
Tool(
name=GitTools.CREATE_BRANCH,
description="Creates a new branch from an optional base branch",
description=(
"Create a new branch from an optional base branch (defaults to the currently checked-out branch). "
"Use to start work on a new feature without checking out the new branch immediately. "
"Returns a confirmation including the source branch name."
),
inputSchema=GitCreateBranch.model_json_schema(),
annotations=ToolAnnotations(
readOnlyHint=False,
Expand All @@ -407,7 +446,11 @@ async def list_tools() -> list[Tool]:
),
Tool(
name=GitTools.CHECKOUT,
description="Switches branches",
description=(
"Switch the working tree to an existing branch by name. "
"Use to move between in-progress work on different branches. "
"Returns a confirmation of the now-active branch. Branch names starting with '-' are rejected."
),
inputSchema=GitCheckout.model_json_schema(),
annotations=ToolAnnotations(
readOnlyHint=False,
Expand All @@ -418,7 +461,11 @@ async def list_tools() -> list[Tool]:
),
Tool(
name=GitTools.SHOW,
description="Shows the contents of a commit",
description=(
"Show the contents of a specific commit (header metadata plus per-file diffs against the parent). "
"Use to inspect a single commit in depth, e.g. to understand what a hash introduced. "
"Returns the commit header followed by unified diffs as text. Revisions starting with '-' are rejected."
),
inputSchema=GitShow.model_json_schema(),
annotations=ToolAnnotations(
readOnlyHint=True,
Expand All @@ -429,7 +476,11 @@ async def list_tools() -> list[Tool]:
),
Tool(
name=GitTools.BRANCH,
description="List Git branches",
description=(
"List git branches scoped to local, remote, or all, optionally filtered by which commit they "
"(do not) contain. Use to enumerate branches matching a specific commit or to inspect remote "
"branches. Returns the branch list as text."
),
inputSchema=GitBranch.model_json_schema(),
annotations=ToolAnnotations(
readOnlyHint=True,
Expand Down
10 changes: 10 additions & 0 deletions src/git/tests/test_server.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -482,3 +482,13 @@ def test_git_branch_rejects_contains_flag_injection(test_repository):

with pytest.raises(BadName):
git_branch(test_repository, "local", not_contains="--exec=evil")


def test_git_branch_rejects_invalid_branch_type(test_repository):
"""git_branch should raise ValueError for invalid branch_type values, consistent
with how the other helpers in this module surface invalid input (raise rather
than return an in-band error string)."""
with pytest.raises(ValueError) as exc_info:
git_branch(test_repository, "invalid-type")
assert "Invalid branch type" in str(exc_info.value)
assert "'invalid-type'" in str(exc_info.value)
Loading