diff --git a/src/mcp_server_git/git/models.py b/src/mcp_server_git/git/models.py index a5895b79..eb9aba4e 100644 --- a/src/mcp_server_git/git/models.py +++ b/src/mcp_server_git/git/models.py @@ -335,6 +335,14 @@ class GitBranchUpdate(BaseModel): ) +class GitBranchDelete(BaseModel): + repo_path: str + branch_name: str = Field(description="Branch to delete") + force: bool = Field( + default=False, description="Force delete unmerged branch (-D)" + ) + + class GitWorktreeList(BaseModel): repo_path: str diff --git a/src/mcp_server_git/git/operations_extended.py b/src/mcp_server_git/git/operations_extended.py index 58d4d5b8..cecb81a7 100644 --- a/src/mcp_server_git/git/operations_extended.py +++ b/src/mcp_server_git/git/operations_extended.py @@ -16,6 +16,7 @@ __all__ = [ "git_restore", "git_branch_update", + "git_branch_delete", "git_worktree_list", "git_worktree_remove", "git_worktree_add", @@ -105,6 +106,18 @@ def git_branch_update( return f"❌ Error updating branch: {str(e)}" +def git_branch_delete( + repo: Repo, + branch_name: str, + force: bool = False, +) -> str: + """Delete a local git branch by delegating to git_branch_update. + + force=True uses -D (delete even if unmerged), else -d (safe delete). + """ + return git_branch_update(repo, branch_name, delete=True, force=force) + + def git_worktree_list(repo: Repo) -> str: """List all worktrees in the repository.""" try: diff --git a/src/mcp_server_git/lean/interface.py b/src/mcp_server_git/lean/interface.py index 09187bb2..d1f9abc8 100644 --- a/src/mcp_server_git/lean/interface.py +++ b/src/mcp_server_git/lean/interface.py @@ -20,6 +20,8 @@ from fastmcp import FastMCP +from mcp_server_git.config import config_manager + from .response_offloader import ResponseOffloader from .token_limiter import MCPTokenLimiter @@ -128,7 +130,13 @@ def __init__( "Issues: https://github.com/MementoRC/mcp-git/issues" ), ) - self.token_limiter = token_limiter or MCPTokenLimiter() + if token_limiter is None: + _settings = config_manager.get_current_settings() + token_limiter = MCPTokenLimiter( + default_limit=_settings.unknown_token_limit, + operation_limits=_settings.operation_limits, + ) + self.token_limiter = token_limiter self.response_offloader = ResponseOffloader(token_limiter=self.token_limiter) # Tool registry diff --git a/src/mcp_server_git/lean/registry_git.py b/src/mcp_server_git/lean/registry_git.py index 6d86462c..3d9457ae 100644 --- a/src/mcp_server_git/lean/registry_git.py +++ b/src/mcp_server_git/lean/registry_git.py @@ -8,6 +8,7 @@ GitAbort, GitAdd, GitBranchList, + GitBranchDelete, GitBranchUpdate, GitCheckout, GitCherryPick, @@ -44,6 +45,7 @@ GitWorktreeRemove, ) from ..git.operations_extended import ( + git_branch_delete, git_branch_update, git_merge_tree, git_restore, @@ -473,6 +475,14 @@ def wrapper(repo_path: str, **kwargs): domain="git", complexity="focused", ), + ToolDefinition( + name="git_branch_delete", + implementation=wrap_repo_op(git_branch_delete), + description="Delete a local git branch (use force=True for unmerged branches)", + schema=GitBranchDelete.model_json_schema(), + domain="git", + complexity="focused", + ), ToolDefinition( name="git_worktree_list", implementation=wrap_repo_op(git_worktree_list), diff --git a/tests/unit/git/test_git_branch_delete.py b/tests/unit/git/test_git_branch_delete.py new file mode 100644 index 00000000..ac607fab --- /dev/null +++ b/tests/unit/git/test_git_branch_delete.py @@ -0,0 +1,110 @@ +""" +Unit tests for git_branch_delete operation. + +These tests verify the git_branch_delete function that deletes a local branch +by delegating to git_branch_update. +""" + +from unittest.mock import Mock + +import pytest + +from mcp_server_git.git.operations_extended import git_branch_delete +from mcp_server_git.utils.git_import import GitCommandError + + +class TestGitBranchDeleteSuccess: + """Test successful git_branch_delete scenarios.""" + + def test_git_branch_delete_removes_branch_returns_success(self): + """Should delete branch and return success message.""" + mock_repo = Mock() + + result = git_branch_delete(mock_repo, branch_name="old-branch") + + assert "✅" in result + assert "old-branch" in result + assert "Deleted" in result + mock_repo.git.branch.assert_called_once_with("-d", "old-branch") + + def test_git_branch_delete_force_deletes_unmerged_branch_uses_capital_D(self): + """Should use -D flag when force=True (unmerged branch).""" + mock_repo = Mock() + + result = git_branch_delete(mock_repo, branch_name="unmerged-branch", force=True) + + assert "✅" in result + assert "unmerged-branch" in result + mock_repo.git.branch.assert_called_once_with("-D", "unmerged-branch") + + def test_git_branch_delete_force_false_uses_lowercase_d(self): + """Should use -d flag when force=False (default).""" + mock_repo = Mock() + + result = git_branch_delete(mock_repo, branch_name="feature") + + mock_repo.git.branch.assert_called_once_with("-d", "feature") + + +class TestGitBranchDeleteInputValidation: + """Test input validation for git_branch_delete.""" + + @pytest.mark.parametrize("dangerous_char", [";", "|", "&", "`", "$"]) + def test_git_branch_delete_rejects_dangerous_chars_in_branch_name(self, dangerous_char): + """Should reject dangerous characters in branch_name.""" + mock_repo = Mock() + malicious_name = f"feature{dangerous_char}rm -rf /" + + result = git_branch_delete(mock_repo, branch_name=malicious_name) + + assert "❌" in result + assert "Invalid characters detected in branch_name" in result + mock_repo.git.branch.assert_not_called() + + def test_git_branch_delete_accepts_valid_branch_names_with_slashes_and_hyphens(self): + """Should accept valid branch names with slashes and hyphens.""" + mock_repo = Mock() + + result = git_branch_delete(mock_repo, branch_name="feature/my-branch_v2") + + assert "✅" in result + mock_repo.git.branch.assert_called_once() + + +class TestGitBranchDeleteErrorHandling: + """Test error handling for git_branch_delete.""" + + def test_git_branch_delete_raises_error_when_unmerged_without_force(self): + """Should return error when deleting unmerged branch without force.""" + mock_repo = Mock() + mock_repo.git.branch.side_effect = GitCommandError( + "git branch", 1, b"", b"error: The branch 'feature' is not fully merged." + ) + + result = git_branch_delete(mock_repo, branch_name="feature", force=False) + + assert "❌" in result + assert "Branch update failed" in result + + def test_git_branch_delete_handles_git_command_error_bytes_stderr(self): + """Should handle GitCommandError with bytes stderr.""" + mock_repo = Mock() + mock_repo.git.branch.side_effect = GitCommandError( + "git branch", 1, b"", b"error: Cannot delete branch 'main' checked out" + ) + + result = git_branch_delete(mock_repo, branch_name="main") + + assert "❌" in result + assert "Branch update failed" in result + + def test_git_branch_delete_handles_general_exception(self): + """Should handle unexpected exceptions gracefully.""" + mock_repo = Mock() + mock_repo.git.branch.side_effect = Exception("Unexpected failure") + + result = git_branch_delete(mock_repo, branch_name="feature") + + assert "❌" in result + assert "Error updating branch" in result + assert "Unexpected failure" in result diff --git a/tests/unit/lean/test_offloader_token_limit.py b/tests/unit/lean/test_offloader_token_limit.py new file mode 100644 index 00000000..b4b529ab --- /dev/null +++ b/tests/unit/lean/test_offloader_token_limit.py @@ -0,0 +1,84 @@ +""" +Regression test: ResponseOffloader uses configured token limits, not the +MCPTokenLimiter hard-wired 2000-token default. + +Bug: GitLeanInterface.__init__ built MCPTokenLimiter() with default_limit=2000 +even when MCP_GIT_UNKNOWN_TOKEN_LIMIT was set to a larger value via env var. +""" + +import pytest + +import mcp_server_git.config.token_limits as _token_limits_mod +from mcp_server_git.lean.token_limiter import MCPTokenLimiter + +# A payload length that sits between the old 2000-token default and the +# configured 20000-token limit. The token estimator uses a ~4 chars/token +# ratio, so 12000 chars ≈ 3000 tokens — above 2000 but far below 20000. +_PAYLOAD_CHARS = 12_000 +_CONFIGURED_LIMIT = 20_000 + + +@pytest.fixture(autouse=True) +def _reset_config_manager(): + """Reset the global config_manager singleton before and after each test.""" + _token_limits_mod.config_manager._settings = None + yield + _token_limits_mod.config_manager._settings = None + + +class TestOffloaderUsesConfiguredLimit: + def test_default_limiter_respects_unknown_token_limit( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + """GitLeanInterface's default token_limiter must use the env-configured + unknown_token_limit, not the MCPTokenLimiter hard-wired 2000 default. + + A mid-sized payload (~3000 tokens) should NOT trigger would_truncate + when MCP_GIT_UNKNOWN_TOKEN_LIMIT=20000 is set. + """ + monkeypatch.setenv("MCP_GIT_UNKNOWN_TOKEN_LIMIT", str(_CONFIGURED_LIMIT)) + + # Force config_manager to re-read env (reset already done by fixture) + settings = _token_limits_mod.config_manager.get_current_settings() + assert settings.unknown_token_limit == _CONFIGURED_LIMIT + + limiter = MCPTokenLimiter( + default_limit=settings.unknown_token_limit, + operation_limits=settings.operation_limits, + ) + mid_payload = {"result": "x" * _PAYLOAD_CHARS} + assert not limiter.would_truncate(mid_payload, "git_diff"), ( + "A mid-sized payload should NOT be truncated when limit is " + f"{_CONFIGURED_LIMIT} tokens (old bug: limit was hard-wired to 2000)" + ) + + def test_old_default_2000_would_have_truncated_same_payload(self) -> None: + """Sanity-check: the same mid-sized payload *does* exceed 2000 tokens, + confirming that the original bare MCPTokenLimiter() would have triggered + offloading unnecessarily. + """ + old_limiter = MCPTokenLimiter(default_limit=2000) + mid_payload = {"result": "x" * _PAYLOAD_CHARS} + assert old_limiter.would_truncate(mid_payload, "git_diff"), ( + "Sanity check failed: mid-sized payload should exceed the old 2000-token default" + ) + + def test_git_lean_interface_token_limiter_uses_configured_limit( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + """End-to-end: GitLeanInterface built without an explicit token_limiter + must derive its default_limit from MCP_GIT_UNKNOWN_TOKEN_LIMIT, not 2000. + """ + monkeypatch.setenv("MCP_GIT_UNKNOWN_TOKEN_LIMIT", str(_CONFIGURED_LIMIT)) + + # Re-load settings so the env var is picked up + settings = _token_limits_mod.config_manager.get_current_settings() + assert settings.unknown_token_limit == _CONFIGURED_LIMIT + + from mcp_server_git.lean.interface import GitLeanInterface + + iface = GitLeanInterface(git_service=None, github_service=None, azure_service=None) + assert iface.token_limiter.default_limit == _CONFIGURED_LIMIT, ( + f"Expected default_limit={_CONFIGURED_LIMIT}, " + f"got {iface.token_limiter.default_limit} (hard-wired 2000 bug still present)" + )