diff --git a/src/git/README.md b/src/git/README.md index 70b00974e6..34ee68899e 100644 --- a/src/git/README.md +++ b/src/git/README.md @@ -53,8 +53,8 @@ Please note that mcp-server-git is currently in early development. The functiona - Adds file contents to the staging area - Inputs: - `repo_path` (string): Path to Git repository - - `files` (string[]): Array of file paths to stage - - Returns: Confirmation of staged files + - `files` (string[]): Non-empty array of repository-relative paths to stage; use `["."]` to stage all files + - Returns: `Files staged successfully` when the index changes, or `No changes were staged` when the request is a no-op 7. `git_reset` - Unstages all staged changes diff --git a/src/git/src/mcp_server_git/server.py b/src/git/src/mcp_server_git/server.py index b94af84661..12781cc5e1 100644 --- a/src/git/src/mcp_server_git/server.py +++ b/src/git/src/mcp_server_git/server.py @@ -42,7 +42,11 @@ class GitCommit(BaseModel): class GitAdd(BaseModel): repo_path: str - files: list[str] + files: list[str] = Field( + ..., + min_length=1, + description="Non-empty list of repository-relative paths to stage; use [\".\"] to stage all files" + ) class GitReset(BaseModel): repo_path: str @@ -129,10 +133,15 @@ def git_commit(repo: git.Repo, message: str) -> str: commit = repo.index.commit(message) return f"Changes committed successfully with hash {commit.hexsha}" +def _staged_entries(repo: git.Repo, files: list[str]) -> str: + # Mode, blob and stage of index entries matching the pathspecs, conflict stages included. + return repo.git.ls_files("--stage", "--", *files) + def git_add(repo: git.Repo, files: list[str]) -> str: - if files == ["."]: - repo.git.add(".") - else: + if not files: + raise ValueError("files must contain at least one path") + + if files != ["."]: # Defense in depth: validate each path resolves within the repository # working tree to prevent path traversal (e.g. '../../etc/passwd' or an # absolute path) from staging files outside repository boundaries. @@ -148,8 +157,12 @@ def git_add(repo: git.Repo, files: list[str]) -> str: raise ValueError( f"Path '{f}' is outside the repository '{repo_root}'" ) - # Use '--' to prevent files starting with '-' from being interpreted as options - repo.git.add("--", *files) + + before_entries = _staged_entries(repo, files) + # Use '--' to prevent files starting with '-' from being interpreted as options + repo.git.add("--", *files) + if _staged_entries(repo, files) == before_entries: + return "No changes were staged" return "Files staged successfully" def git_reset(repo: git.Repo) -> str: @@ -361,7 +374,7 @@ async def list_tools() -> list[Tool]: ), Tool( name=GitTools.ADD, - description="Adds file contents to the staging area", + description="Adds file contents to the staging area. Returns 'Files staged successfully' when the index changes, or 'No changes were staged' when the requested files were already staged", inputSchema=GitAdd.model_json_schema(), annotations=ToolAnnotations( readOnlyHint=False, diff --git a/src/git/tests/test_server.py b/src/git/tests/test_server.py index 05d5931466..f4511f5486 100644 --- a/src/git/tests/test_server.py +++ b/src/git/tests/test_server.py @@ -25,6 +25,10 @@ def test_repository(tmp_path: Path): repo_path = tmp_path / "temp_test_repo" test_repo = git.Repo.init(repo_path) + # CLI merges require an identity even when they stop with conflicts. + with test_repo.config_writer() as config: + config.set_value("user", "name", "Test User") + config.set_value("user", "email", "test@example.com") Path(repo_path / "test.txt").write_text("test") test_repo.index.add(["test.txt"]) @@ -111,6 +115,82 @@ def test_git_add_specific_files(test_repository): assert "file2.txt" not in staged_files assert result == "Files staged successfully" +def test_git_add_rejects_empty_files(test_repository): + with mock.patch.object(git.Git, "execute") as execute: + with pytest.raises(ValueError, match="files must contain at least one path"): + git_add(test_repository, []) + execute.assert_not_called() + + assert list(test_repository.index.diff("HEAD")) == [] + +def test_git_add_clean_tree_reports_no_changes(test_repository): + result = git_add(test_repository, ["."]) + + assert result == "No changes were staged" + assert list(test_repository.index.diff("HEAD")) == [] + +def test_git_add_reports_no_changes_when_file_is_already_staged(test_repository): + file_path = Path(test_repository.working_dir) / "already_staged.txt" + file_path.write_text("already staged") + test_repository.index.add(["already_staged.txt"]) + + result = git_add(test_repository, ["already_staged.txt"]) + + assert result == "No changes were staged" + +def test_git_add_no_op_ignores_unrelated_index_changes(test_repository): + staged_path = Path(test_repository.working_dir) / "staged.txt" + staged_path.write_text("staged") + test_repository.index.add(["staged.txt"]) + (Path(test_repository.working_dir) / "other.txt").write_text("other") + git_add(test_repository, ["other.txt"]) + + result = git_add(test_repository, ["staged.txt"]) + + assert result == "No changes were staged" + +def test_git_add_glob_pathspec_reports_staged_changes(test_repository): + (Path(test_repository.working_dir) / "new.md").write_text("new") + + result = git_add(test_repository, ["*.md"]) + + assert result == "Files staged successfully" + assert "new.md" in [item.a_path for item in test_repository.index.diff("HEAD")] + +def test_git_add_nonexistent_path_still_raises(test_repository): + with pytest.raises(git.GitCommandError): + git_add(test_repository, ["nope.txt"]) + +@pytest.mark.parametrize("leave_conflict", [False, True]) +def test_git_add_stages_resolved_merge_conflict(test_repository, leave_conflict): + original_branch = test_repository.active_branch.name + file_path = Path(test_repository.working_dir) / "test.txt" + other_path = Path(test_repository.working_dir) / "other.txt" + test_repository.git.checkout("-b", "conflicting") + file_path.write_text("branch content") + other_path.write_text("branch content") + test_repository.index.add(["test.txt", "other.txt"]) + test_repository.index.commit("branch change") + test_repository.git.checkout(original_branch) + file_path.write_text("main content") + other_path.write_text("main content") + test_repository.index.add(["test.txt", "other.txt"]) + test_repository.index.commit("main change") + with pytest.raises(git.GitCommandError, match="CONFLICT"): + test_repository.git.merge("conflicting") + assert test_repository.index.unmerged_blobs() + file_path.write_text("resolved content") + if not leave_conflict: + other_path.write_text("resolved content") + + result = git_add(test_repository, ["test.txt"] if leave_conflict else ["."]) + + assert result == "Files staged successfully" + assert set(test_repository.index.unmerged_blobs()) == ( + {"other.txt"} if leave_conflict else set() + ) + assert test_repository.git.show(":test.txt") == "resolved content" + 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