From 0aced5f7ba73ba02c6ac2c4e959addf048edcd1b Mon Sep 17 00:00:00 2001 From: Connor Moss Date: Sun, 6 Sep 2026 00:56:37 -0400 Subject: [PATCH 1/2] fix(git): stop reporting staged files when nothing was staged git_add returned the constant "Files staged successfully" regardless of what the index took. `git add` exits 0 when it stages nothing, so two callers got a success they could not check: - files: [] runs `git add --` with no pathspec, a no-op - files: ["."] on a tree with no changes stages nothing Paired with git_commit, an agent could stage nothing, commit nothing, and report the work as committed. Rejects an empty list at the schema and in the function, and reads the outcome back from the index instead of assuming it from the exit status. The success string is unchanged when it is true. --- src/git/src/mcp_server_git/server.py | 23 ++++++++++++++++++++++- src/git/tests/test_server.py | 23 +++++++++++++++++++++++ 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/src/git/src/mcp_server_git/server.py b/src/git/src/mcp_server_git/server.py index b94af84661..8be4609ecc 100644 --- a/src/git/src/mcp_server_git/server.py +++ b/src/git/src/mcp_server_git/server.py @@ -42,7 +42,7 @@ class GitCommit(BaseModel): class GitAdd(BaseModel): repo_path: str - files: list[str] + files: list[str] = Field(..., min_length=1) class GitReset(BaseModel): repo_path: str @@ -125,11 +125,23 @@ def git_diff(repo: git.Repo, target: str, context_lines: int = DEFAULT_CONTEXT_L repo.rev_parse(target) # Validates target is a real git ref, throws BadName if not return repo.git.diff(f"--unified={context_lines}", target) +def _staged_paths(repo: git.Repo) -> list[str]: + """Paths the index currently holds as changes against HEAD.""" + if not repo.head.is_valid(): + # Unborn branch: everything in the index is staged for the first commit. + return [entry[0] for entry in repo.index.entries] + return [diff.a_path or diff.b_path for diff in repo.index.diff(repo.head.commit)] + def git_commit(repo: git.Repo, message: str) -> str: commit = repo.index.commit(message) return f"Changes committed successfully with hash {commit.hexsha}" def git_add(repo: git.Repo, files: list[str]) -> str: + if not files: + raise ValueError( + "No files provided to stage. Pass one or more paths, " + "or ['.'] to stage everything." + ) if files == ["."]: repo.git.add(".") else: @@ -150,6 +162,15 @@ def git_add(repo: git.Repo, files: list[str]) -> str: ) # Use '--' to prevent files starting with '-' from being interpreted as options repo.git.add("--", *files) + + # `git add` exits 0 when it stages nothing -- an empty pathspec, or '.' on a + # tree with no changes -- so the outcome has to be read back from the index + # rather than assumed from the exit status. + if not _staged_paths(repo): + return ( + "No files staged: the given paths matched no changes. " + "git_status shows what is modified or untracked." + ) return "Files staged successfully" def git_reset(repo: git.Repo) -> str: diff --git a/src/git/tests/test_server.py b/src/git/tests/test_server.py index 05d5931466..aace0374c8 100644 --- a/src/git/tests/test_server.py +++ b/src/git/tests/test_server.py @@ -20,6 +20,7 @@ ) import shutil import unittest.mock as mock +from pydantic import ValidationError @pytest.fixture def test_repository(tmp_path: Path): @@ -111,6 +112,28 @@ def test_git_add_specific_files(test_repository): assert "file2.txt" not in staged_files assert result == "Files staged successfully" +def test_git_add_reports_when_nothing_was_staged(test_repository): + """`git add` exits 0 when it stages nothing, so the old unconditional + "Files staged successfully" claimed work on an unchanged tree.""" + assert not test_repository.is_dirty(untracked_files=True) + + result = git_add(test_repository, ["."]) + + assert result != "Files staged successfully" + assert "No files staged" in result + assert not test_repository.index.diff(test_repository.head.commit) + +def test_git_add_rejects_an_empty_file_list(test_repository): + """`git add --` with no pathspec is a no-op that exits 0.""" + with pytest.raises(ValueError, match="No files provided to stage"): + git_add(test_repository, []) + +def test_git_add_schema_rejects_an_empty_file_list(): + from mcp_server_git.server import GitAdd + + with pytest.raises(ValidationError): + GitAdd(repo_path=".", files=[]) + def test_git_add_rejects_path_traversal(test_repository): # Security invariant (CVE-2026-27735): a relative path escaping the # repository must never be staged. Accept rejection from either the From 621a89988b17378bd15a8aa9c9aa3b2aa4a40cb0 Mon Sep 17 00:00:00 2001 From: Connor Moss Date: Sun, 6 Sep 2026 00:59:36 -0400 Subject: [PATCH 2/2] fix(git): satisfy pyright in _staged_paths Index entry keys are PathLike, and a diff entry can leave either path unset, so the helper returned list[PathLike] and list[str | None] where list[str] was declared. --- src/git/src/mcp_server_git/server.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/git/src/mcp_server_git/server.py b/src/git/src/mcp_server_git/server.py index 8be4609ecc..abd77aadac 100644 --- a/src/git/src/mcp_server_git/server.py +++ b/src/git/src/mcp_server_git/server.py @@ -129,8 +129,14 @@ def _staged_paths(repo: git.Repo) -> list[str]: """Paths the index currently holds as changes against HEAD.""" if not repo.head.is_valid(): # Unborn branch: everything in the index is staged for the first commit. - return [entry[0] for entry in repo.index.entries] - return [diff.a_path or diff.b_path for diff in repo.index.diff(repo.head.commit)] + return [str(path) for path, _stage in repo.index.entries] + # A rename or delete leaves one side of the diff unset, so take whichever + # path the entry does carry and drop any entry with neither. + return [ + path + for diff in repo.index.diff(repo.head.commit) + if (path := diff.a_path or diff.b_path) is not None + ] def git_commit(repo: git.Repo, message: str) -> str: commit = repo.index.commit(message)