From 0b819edceb307ce2f8ba6d58b37a86329b7d6ec0 Mon Sep 17 00:00:00 2001 From: Dawid Rycerz Date: Wed, 28 May 2025 15:27:57 +0200 Subject: feat(gitlab_glab): add merge request diff functionality with large diff mitigation - Add get_mr_diff method to GitLabServer class with support for all glab mr diff options - Implement smart large diff handling that saves diffs > 100KB to temporary files - Add comprehensive test coverage (7 new tests) achieving 95% coverage - Support for MR ID/branch, color options, raw format, and repository selection - Include proper error handling and logging - Update README with usage examples and documentation - Follow project coding standards with type hints and docstrings Resolves large diff handling for LLM consumption while preserving full diff access. --- servers/gitlab_glab/tests/test_integration.py | 3 +- servers/gitlab_glab/tests/test_server.py | 177 ++++++++++++++++++++++++++ 2 files changed, 179 insertions(+), 1 deletion(-) (limited to 'servers/gitlab_glab/tests') diff --git a/servers/gitlab_glab/tests/test_integration.py b/servers/gitlab_glab/tests/test_integration.py index 5d7eed5..b7d1005 100644 --- a/servers/gitlab_glab/tests/test_integration.py +++ b/servers/gitlab_glab/tests/test_integration.py @@ -28,7 +28,8 @@ class TestIntegration: # - find_project # - search_issues # - create_issue - assert mock_server.tool.call_count == 4 + # - get_mr_diff + assert mock_server.tool.call_count == 5 # Verify that the tool decorator was called with functions that have # working_directory parameter. We can't directly access the decorated functions diff --git a/servers/gitlab_glab/tests/test_server.py b/servers/gitlab_glab/tests/test_server.py index ab93026..a7e74a4 100644 --- a/servers/gitlab_glab/tests/test_server.py +++ b/servers/gitlab_glab/tests/test_server.py @@ -585,3 +585,180 @@ class TestGitLabServer: # Verify the number of calls assert mock_run.call_count == len(working_dirs) + + @patch.object(GitLabServer, "execute_glab_command") + def test_get_mr_diff_success_small(self, mock_execute: MagicMock) -> None: + """Test successful MR diff retrieval with small diff.""" + # Mock successful command execution with small diff + diff_content = """diff --git a/file.txt b/file.txt +index 1234567..abcdefg 100644 +--- a/file.txt ++++ b/file.txt +@@ -1,3 +1,4 @@ + line 1 + line 2 ++new line + line 3""" + mock_execute.return_value = (True, diff_content) + + server = GitLabServer() + working_dir = "/test/directory" + result = server.get_mr_diff( + working_directory=working_dir, + mr_id="123", + color="never", + raw=False, + repo="group/project", + ) + + assert "diff" in result + assert result["diff"] == diff_content + assert result["size_kb"] < 1 + assert result["temp_file_path"] is None + + # Verify command was called with correct arguments + mock_execute.assert_called_once_with( + ["mr", "diff", "123", "--color", "never", "-R", "group/project"], + working_dir, + ) + + @patch.object(GitLabServer, "execute_glab_command") + def test_get_mr_diff_success_current_branch(self, mock_execute: MagicMock) -> None: + """Test successful MR diff retrieval for current branch.""" + diff_content = "diff content" + mock_execute.return_value = (True, diff_content) + + server = GitLabServer() + working_dir = "/test/directory" + result = server.get_mr_diff(working_directory=working_dir) + + assert "diff" in result + assert result["diff"] == diff_content + assert result["temp_file_path"] is None + + # Verify command was called without MR ID + mock_execute.assert_called_once_with( + ["mr", "diff", "--color", "never"], + working_dir, + ) + + @patch.object(GitLabServer, "execute_glab_command") + def test_get_mr_diff_with_raw_option(self, mock_execute: MagicMock) -> None: + """Test MR diff retrieval with raw option.""" + diff_content = "raw diff content" + mock_execute.return_value = (True, diff_content) + + server = GitLabServer() + working_dir = "/test/directory" + result = server.get_mr_diff( + working_directory=working_dir, + mr_id="branch-name", + color="auto", + raw=True, + ) + + assert "diff" in result + assert result["diff"] == diff_content + + # Verify command was called with raw option + mock_execute.assert_called_once_with( + ["mr", "diff", "branch-name", "--color", "auto", "--raw"], + working_dir, + ) + + @patch("tempfile.NamedTemporaryFile") + @patch.object(GitLabServer, "execute_glab_command") + def test_get_mr_diff_large_diff_temp_file( + self, mock_execute: MagicMock, mock_temp_file: MagicMock + ) -> None: + """Test MR diff retrieval with large diff that gets saved to temp file.""" + # Create a large diff content (over 100KB) + large_diff = "x" * (101 * 1024) # 101 KB + mock_execute.return_value = (True, large_diff) + + # Mock temporary file + mock_file = MagicMock() + mock_file.name = "/tmp/mr_diff_12345.diff" + mock_temp_file.return_value.__enter__.return_value = mock_file + + server = GitLabServer() + working_dir = "/test/directory" + result = server.get_mr_diff( + working_directory=working_dir, + mr_id="123", + max_size_kb=100, + ) + + assert result["diff_too_large"] is True + assert result["size_kb"] > 100 + assert result["max_size_kb"] == 100 + assert result["temp_file_path"] == "/tmp/mr_diff_12345.diff" + assert "message" in result + + # Verify temp file was created with correct parameters + mock_temp_file.assert_called_once_with( + mode='w', + suffix='.diff', + prefix='mr_diff_', + delete=False, + encoding='utf-8' + ) + mock_file.write.assert_called_once_with(large_diff) + + @patch("tempfile.NamedTemporaryFile") + @patch.object(GitLabServer, "execute_glab_command") + def test_get_mr_diff_large_diff_temp_file_error( + self, mock_execute: MagicMock, mock_temp_file: MagicMock + ) -> None: + """Test MR diff retrieval with large diff and temp file creation error.""" + # Create a large diff content + large_diff = "x" * (101 * 1024) # 101 KB + mock_execute.return_value = (True, large_diff) + + # Mock temporary file creation error + mock_temp_file.side_effect = Exception("Permission denied") + + server = GitLabServer() + working_dir = "/test/directory" + result = server.get_mr_diff( + working_directory=working_dir, + mr_id="123", + max_size_kb=100, + ) + + assert "error" in result + assert "too large" in result["error"] + assert "Permission denied" in result["error"] + + @patch.object(GitLabServer, "execute_glab_command") + def test_get_mr_diff_command_failure(self, mock_execute: MagicMock) -> None: + """Test MR diff retrieval with command failure.""" + mock_execute.return_value = (False, {"error": "MR not found"}) + + server = GitLabServer() + working_dir = "/test/directory" + result = server.get_mr_diff(working_directory=working_dir, mr_id="999") + + assert "error" in result + assert result["error"] == "MR not found" + + @patch.object(GitLabServer, "execute_glab_command") + def test_get_mr_diff_invalid_color_option(self, mock_execute: MagicMock) -> None: + """Test MR diff with invalid color option skips color parameter.""" + diff_content = "diff content" + mock_execute.return_value = (True, diff_content) + + server = GitLabServer() + working_dir = "/test/directory" + result = server.get_mr_diff( + working_directory=working_dir, + mr_id="123", + color="invalid", # Invalid color option + ) + + assert "diff" in result + # Invalid color options should be filtered out, only valid ones are added + mock_execute.assert_called_once_with( + ["mr", "diff", "123"], + working_dir, + ) -- cgit v1.2.3 From 54b521f19e00bea52304f7379ca59de0c2e20962 Mon Sep 17 00:00:00 2001 From: Dawid Rycerz Date: Wed, 28 May 2025 15:44:40 +0200 Subject: feat(gitlab_glab): add CI pipeline functionality with glab ci run - Add new run_ci_pipeline method to GitLabServer class - Support all glab ci run options including variables, branch, and web mode - Auto-detect current branch using git branch --show-current when -b is missing - Implement web mode that overrides CI_PIPELINE_SOURCE=web - Add comprehensive test coverage with 9 new test cases - Update README.md with complete documentation and usage examples - Maintain 95% test coverage and pass all 53 tests - Follow project standards with proper error handling and type hints Closes: Add CI job runner functionality as requested --- servers/gitlab_glab/README.md | 35 +++ .../src/mcp_server_gitlab_glab/server.py | 144 ++++++++++++ servers/gitlab_glab/tests/test_integration.py | 3 +- servers/gitlab_glab/tests/test_server.py | 258 +++++++++++++++++++++ 4 files changed, 439 insertions(+), 1 deletion(-) (limited to 'servers/gitlab_glab/tests') diff --git a/servers/gitlab_glab/README.md b/servers/gitlab_glab/README.md index a2bb677..19b6cd7 100644 --- a/servers/gitlab_glab/README.md +++ b/servers/gitlab_glab/README.md @@ -9,6 +9,7 @@ This MCP server provides integration with GitLab through the GitLab CLI (`glab`) - Search for GitLab issues with various filters - Create new GitLab issues - Get merge request diffs with automatic handling of large diffs +- Run CI/CD pipelines with support for variables and web mode - More features to be added in the future ## Prerequisites @@ -179,6 +180,40 @@ The function returns a dictionary containing: **Note on Large Diffs**: To prevent overwhelming LLMs with extremely large diffs, this tool automatically saves diffs larger than `max_size_kb` (default: 100KB) to a temporary file and returns the file path instead of the content. This allows you to process large merge request diffs without hitting token limits. +### run_ci_pipeline + +Run a CI/CD pipeline on GitLab using `glab ci run`. + +```python +result = use_mcp_tool( + server_name="gitlab_glab", + tool_name="run_ci_pipeline", + arguments={ + "working_directory": "/path/to/current/directory", + # Optional parameters + "branch": "main", # Branch/ref to run pipeline on (if None, uses current branch) + "variables": ["VAR1:value1", "VAR2:value2"], # Variables in key:value format + "variables_env": ["ENV1:envval1"], # Environment variables in key:value format + "variables_file": ["FILE1:file1.txt"], # File variables in key:filename format + "variables_from": "/path/to/vars.json", # JSON file containing variables + "web_mode": True, # Enable web mode (sets CI_PIPELINE_SOURCE=web) + "repo": "group/project" # Project path with namespace + } +) +``` + +The function returns a dictionary containing: +- `success`: Boolean indicating if the pipeline was created successfully +- `output`: The full command output from glab +- `branch`: The branch the pipeline was created on +- `web_mode`: Boolean indicating if web mode was used +- `pipeline_url`: The URL of the created pipeline (if found in output) +- `error`: Error message if the operation failed + +**Branch Detection**: If no `branch` is specified, the tool will automatically detect the current git branch using `git branch --show-current`. If branch detection fails, the pipeline will be created without specifying a branch (uses GitLab's default behavior). + +**Web Mode**: When `web_mode` is set to `True`, the tool adds `CI_PIPELINE_SOURCE:web` as an environment variable, which allows the pipeline to run with web-triggered behavior and access to manual pipeline features. + ## Working Directory Context All tools require a `working_directory` parameter that specifies the directory context in which the GitLab CLI commands should be executed. This ensures that commands are run in the same directory as the agent, maintaining proper context for repository operations. diff --git a/servers/gitlab_glab/src/mcp_server_gitlab_glab/server.py b/servers/gitlab_glab/src/mcp_server_gitlab_glab/server.py index 881228f..b661255 100644 --- a/servers/gitlab_glab/src/mcp_server_gitlab_glab/server.py +++ b/servers/gitlab_glab/src/mcp_server_gitlab_glab/server.py @@ -402,7 +402,115 @@ class GitLabServer: "temp_file_path": None } + def run_ci_pipeline( + self, + working_directory: str, + branch: str | None = None, + variables: list[str] | None = None, + variables_env: list[str] | None = None, + variables_file: list[str] | None = None, + variables_from: str | None = None, + web_mode: bool = False, + repo: str | None = None, + ) -> dict[str, Any]: + """Run a CI/CD pipeline on GitLab. + Args: + working_directory: The directory to execute the command in. + branch: Create pipeline on branch/ref. If None, uses current branch. + variables: Pass variables to pipeline in format key:value. + variables_env: Pass environment variables to pipeline in format key:value. + variables_file: Pass file contents as file variables in format key:filename. + variables_from: JSON file containing variables for pipeline execution. + web_mode: Run pipeline in web mode (overrides CI_PIPELINE_SOURCE to 'web'). + repo: Select another repository (OWNER/REPO or GROUP/NAMESPACE/REPO format). + + Returns: + A dictionary containing the pipeline information or an error message. + """ + # Build command arguments + args = ["ci", "run"] + + # Handle branch - if not provided, get current branch + if branch is None: + try: + # Get current branch using git command + result = subprocess.run( + ["git", "branch", "--show-current"], + capture_output=True, + text=True, + check=False, + cwd=working_directory, + ) + if result.returncode == 0 and result.stdout.strip(): + branch = result.stdout.strip() + logger.info(f"Using current branch: {branch}") + else: + logger.warning( + "Could not determine current branch, proceeding without -b flag" + ) + except Exception as e: + logger.warning(f"Could not determine current branch: {str(e)}") + + # Add branch if available + if branch: + args.extend(["-b", branch]) + + # Add variables + if variables: + for var in variables: + args.extend(["--variables", var]) + + # Add environment variables + if variables_env: + for var in variables_env: + args.extend(["--variables-env", var]) + + # Add file variables + if variables_file: + for var in variables_file: + args.extend(["--variables-file", var]) + + # Add variables from file + if variables_from: + args.extend(["-f", variables_from]) + + # Add web mode - override CI_PIPELINE_SOURCE to 'web' + if web_mode: + args.extend(["--variables-env", "CI_PIPELINE_SOURCE:web"]) + + # Add repo + if repo: + args.extend(["-R", repo]) + + # Execute the command + success, result = self.execute_glab_command(args, working_directory) + + if not success: + return result + + # Parse the output to extract pipeline information + # The output typically contains pipeline URL and ID + output_lines = result.strip().split('\n') if isinstance(result, str) else [] + + pipeline_info = { + "success": True, + "output": result, + "branch": branch, + "web_mode": web_mode + } + + # Try to extract pipeline URL if present + for line in output_lines: + if "https://" in line and "/pipelines/" in line: + # Extract just the URL from the line + import re + url_match = re.search(r'https://[^\s]+/pipelines/\d+', line) + if url_match: + pipeline_info["pipeline_url"] = url_match.group(0) + break + + return pipeline_info def create_server(host: str = "127.0.0.1", port: int = 8080) -> FastMCP: """Create and configure the FastMCP server. @@ -581,6 +689,42 @@ def create_server(host: str = "127.0.0.1", port: int = 8080) -> FastMCP: max_size_kb=max_size_kb, ) + @mcp.tool() + def run_ci_pipeline( + working_directory: str, + branch: str | None = None, + variables: list[str] | None = None, + variables_env: list[str] | None = None, + variables_file: list[str] | None = None, + variables_from: str | None = None, + web_mode: bool = False, + repo: str | None = None, + ) -> dict[str, Any]: + """Run a CI/CD pipeline on GitLab. + + Args: + working_directory: The directory to execute the command in. + branch: Create pipeline on branch/ref. If None, uses current branch. + variables: Pass variables to pipeline in format key:value. + variables_env: Pass environment variables to pipeline in format key:value. + variables_file: Pass file contents as file variables in format key:filename. + variables_from: JSON file containing variables for pipeline execution. + web_mode: Run pipeline in web mode (overrides CI_PIPELINE_SOURCE to 'web'). + repo: Select another repository (OWNER/REPO or GROUP/NAMESPACE/REPO format). + + Returns: + A dictionary containing the pipeline information or an error message. + """ + return gitlab.run_ci_pipeline( + working_directory=working_directory, + branch=branch, + variables=variables, + variables_env=variables_env, + variables_file=variables_file, + variables_from=variables_from, + web_mode=web_mode, + repo=repo, + ) return mcp diff --git a/servers/gitlab_glab/tests/test_integration.py b/servers/gitlab_glab/tests/test_integration.py index b7d1005..1e55b46 100644 --- a/servers/gitlab_glab/tests/test_integration.py +++ b/servers/gitlab_glab/tests/test_integration.py @@ -29,7 +29,8 @@ class TestIntegration: # - search_issues # - create_issue # - get_mr_diff - assert mock_server.tool.call_count == 5 + # - run_ci_pipeline + assert mock_server.tool.call_count == 6 # Verify that the tool decorator was called with functions that have # working_directory parameter. We can't directly access the decorated functions diff --git a/servers/gitlab_glab/tests/test_server.py b/servers/gitlab_glab/tests/test_server.py index a7e74a4..867b2c0 100644 --- a/servers/gitlab_glab/tests/test_server.py +++ b/servers/gitlab_glab/tests/test_server.py @@ -762,3 +762,261 @@ index 1234567..abcdefg 100644 ["mr", "diff", "123"], working_dir, ) + + # Tests for run_ci_pipeline method + + @patch.object(GitLabServer, "execute_glab_command") + @patch("subprocess.run") + def test_run_ci_pipeline_success_with_branch( + self, mock_subprocess: MagicMock, mock_execute: MagicMock + ) -> None: + """Test successful pipeline run with specified branch.""" + # Mock successful glab command execution + mock_execute.return_value = (True, "Pipeline created: https://gitlab.example.com/project/-/pipelines/123") + + server = GitLabServer() + working_dir = "/test/directory" + result = server.run_ci_pipeline( + working_directory=working_dir, + branch="main", + ) + + assert result["success"] is True + assert result["branch"] == "main" + assert result["web_mode"] is False + assert "pipeline_url" in result + assert result["pipeline_url"] == "https://gitlab.example.com/project/-/pipelines/123" + mock_execute.assert_called_once_with( + ["ci", "run", "-b", "main"], + working_dir, + ) + # subprocess.run should not be called when branch is specified + mock_subprocess.assert_not_called() + + @patch.object(GitLabServer, "execute_glab_command") + @patch("subprocess.run") + def test_run_ci_pipeline_success_current_branch( + self, mock_subprocess: MagicMock, mock_execute: MagicMock + ) -> None: + """Test successful pipeline run using current branch.""" + # Mock git branch --show-current command + mock_git_process = MagicMock() + mock_git_process.returncode = 0 + mock_git_process.stdout = "feature-branch" + mock_subprocess.return_value = mock_git_process + + # Mock successful glab command execution + mock_execute.return_value = (True, "Pipeline created: https://gitlab.example.com/project/-/pipelines/456") + + server = GitLabServer() + working_dir = "/test/directory" + result = server.run_ci_pipeline( + working_directory=working_dir, + ) + + assert result["success"] is True + assert result["branch"] == "feature-branch" + assert result["web_mode"] is False + assert "pipeline_url" in result + assert result["pipeline_url"] == "https://gitlab.example.com/project/-/pipelines/456" + + mock_subprocess.assert_called_once_with( + ["git", "branch", "--show-current"], + capture_output=True, + text=True, + check=False, + cwd=working_dir, + ) + mock_execute.assert_called_once_with( + ["ci", "run", "-b", "feature-branch"], + working_dir, + ) + + @patch.object(GitLabServer, "execute_glab_command") + @patch("subprocess.run") + def test_run_ci_pipeline_git_branch_failure( + self, mock_subprocess: MagicMock, mock_execute: MagicMock + ) -> None: + """Test pipeline run when git branch --show-current fails.""" + # Mock git branch --show-current command failure + mock_git_process = MagicMock() + mock_git_process.returncode = 1 + mock_git_process.stdout = "" + mock_subprocess.return_value = mock_git_process + + # Mock successful glab command execution (without branch) + mock_execute.return_value = (True, "Pipeline created: https://gitlab.example.com/project/-/pipelines/789") + + server = GitLabServer() + working_dir = "/test/directory" + result = server.run_ci_pipeline( + working_directory=working_dir, + ) + + assert result["success"] is True + assert result["branch"] is None + assert result["web_mode"] is False + + mock_subprocess.assert_called_once_with( + ["git", "branch", "--show-current"], + capture_output=True, + text=True, + check=False, + cwd=working_dir, + ) + # Should run without -b flag when branch detection fails + mock_execute.assert_called_once_with( + ["ci", "run"], + working_dir, + ) + + @patch.object(GitLabServer, "execute_glab_command") + def test_run_ci_pipeline_with_web_mode(self, mock_execute: MagicMock) -> None: + """Test pipeline run with web mode enabled.""" + # Mock successful glab command execution + mock_execute.return_value = (True, "Pipeline created in web mode") + + server = GitLabServer() + working_dir = "/test/directory" + result = server.run_ci_pipeline( + working_directory=working_dir, + branch="main", + web_mode=True, + ) + + assert result["success"] is True + assert result["branch"] == "main" + assert result["web_mode"] is True + + mock_execute.assert_called_once_with( + ["ci", "run", "-b", "main", "--variables-env", "CI_PIPELINE_SOURCE:web"], + working_dir, + ) + + @patch.object(GitLabServer, "execute_glab_command") + def test_run_ci_pipeline_with_variables(self, mock_execute: MagicMock) -> None: + """Test pipeline run with various variable types.""" + # Mock successful glab command execution + mock_execute.return_value = (True, "Pipeline created with variables") + + server = GitLabServer() + working_dir = "/test/directory" + result = server.run_ci_pipeline( + working_directory=working_dir, + branch="develop", + variables=["VAR1:value1", "VAR2:value2"], + variables_env=["ENV1:envvalue1"], + variables_file=["FILE1:file1.txt"], + variables_from="/path/to/vars.json", + repo="group/project", + ) + + assert result["success"] is True + assert result["branch"] == "develop" + + expected_args = [ + "ci", "run", + "-b", "develop", + "--variables", "VAR1:value1", + "--variables", "VAR2:value2", + "--variables-env", "ENV1:envvalue1", + "--variables-file", "FILE1:file1.txt", + "-f", "/path/to/vars.json", + "-R", "group/project" + ] + mock_execute.assert_called_once_with(expected_args, working_dir) + + @patch.object(GitLabServer, "execute_glab_command") + def test_run_ci_pipeline_with_web_mode_and_variables( + self, mock_execute: MagicMock + ) -> None: + """Test pipeline run with web mode and additional variables.""" + # Mock successful glab command execution + mock_execute.return_value = (True, "Pipeline created") + + server = GitLabServer() + working_dir = "/test/directory" + result = server.run_ci_pipeline( + working_directory=working_dir, + branch="main", + variables_env=["CUSTOM_VAR:value"], + web_mode=True, + ) + + assert result["success"] is True + assert result["web_mode"] is True + + expected_args = [ + "ci", "run", + "-b", "main", + "--variables-env", "CUSTOM_VAR:value", + "--variables-env", "CI_PIPELINE_SOURCE:web" + ] + mock_execute.assert_called_once_with(expected_args, working_dir) + + @patch.object(GitLabServer, "execute_glab_command") + def test_run_ci_pipeline_glab_failure(self, mock_execute: MagicMock) -> None: + """Test pipeline run when glab command fails.""" + # Mock glab command failure + mock_execute.return_value = (False, {"error": "Authentication required"}) + + server = GitLabServer() + working_dir = "/test/directory" + result = server.run_ci_pipeline( + working_directory=working_dir, + branch="main", + ) + + assert result == {"error": "Authentication required"} + mock_execute.assert_called_once_with( + ["ci", "run", "-b", "main"], + working_dir, + ) + + @patch.object(GitLabServer, "execute_glab_command") + def test_run_ci_pipeline_no_url_in_output(self, mock_execute: MagicMock) -> None: + """Test pipeline run when output doesn't contain pipeline URL.""" + # Mock successful glab command execution without URL + mock_execute.return_value = ( + True, + "Pipeline created successfully\nCheck status later" + ) + + server = GitLabServer() + working_dir = "/test/directory" + result = server.run_ci_pipeline( + working_directory=working_dir, + branch="main", + ) + + assert result["success"] is True + assert result["branch"] == "main" + assert "pipeline_url" not in result + assert "Pipeline created successfully" in result["output"] + + @patch.object(GitLabServer, "execute_glab_command") + @patch("subprocess.run") + def test_run_ci_pipeline_git_exception( + self, mock_subprocess: MagicMock, mock_execute: MagicMock + ) -> None: + """Test pipeline run when git command raises an exception.""" + # Mock git branch --show-current command exception + mock_subprocess.side_effect = Exception("Git command failed") + + # Mock successful glab command execution (without branch) + mock_execute.return_value = (True, "Pipeline created") + + server = GitLabServer() + working_dir = "/test/directory" + result = server.run_ci_pipeline( + working_directory=working_dir, + ) + + assert result["success"] is True + assert result["branch"] is None + + # Should run without -b flag when git command fails + mock_execute.assert_called_once_with( + ["ci", "run"], + working_dir, + ) -- cgit v1.2.3 From 126ce542dca21e62b38049f67c7f4c951284a2c2 Mon Sep 17 00:00:00 2001 From: Dawid Rycerz Date: Wed, 28 May 2025 15:58:05 +0200 Subject: feat(gitlab_glab): add diff content filtering to get_mr_diff - Add filter_diff_content method to filter out unwanted file extensions - Default filtering excludes .lock and .log files to reduce noise - Smart lock file detection handles package-lock.json, yarn.lock, etc. - Customizable filtering via filter_extensions parameter - Filtering can be disabled by passing empty list - Add comprehensive tests for all filtering scenarios - Update README.md with filtering documentation - Maintain backward compatibility with existing API --- servers/gitlab_glab/README.md | 13 +- .../src/mcp_server_gitlab_glab/server.py | 72 +++++- servers/gitlab_glab/tests/test_server.py | 287 +++++++++++++++++++++ 3 files changed, 369 insertions(+), 3 deletions(-) (limited to 'servers/gitlab_glab/tests') diff --git a/servers/gitlab_glab/README.md b/servers/gitlab_glab/README.md index 19b6cd7..cef74c2 100644 --- a/servers/gitlab_glab/README.md +++ b/servers/gitlab_glab/README.md @@ -151,7 +151,7 @@ The function returns a dictionary containing: ### get_mr_diff -Get the diff for a merge request using `glab mr diff`. +Get the diff for a merge request using `glab mr diff` with optional content filtering. ```python result = use_mcp_tool( @@ -164,12 +164,14 @@ result = use_mcp_tool( "color": "never", # Use color in diff output: always, never, auto (default: never) "raw": True, # Use raw diff format (default: False) "repo": "group/project", # Project path with namespace - "max_size_kb": 100 # Maximum size in KB before saving to temp file (default: 100) + "max_size_kb": 100, # Maximum size in KB before saving to temp file (default: 100) + "filter_extensions": [".lock", ".log"] # File extensions to exclude from diff (default: [".lock", ".log"]) } ) ``` The function returns a dictionary containing: + - `diff`: The diff content (if small enough) - `size_kb`: The size of the diff in KB - `temp_file_path`: Path to temporary file if diff is too large (None otherwise) @@ -178,6 +180,13 @@ The function returns a dictionary containing: - `message`: Human-readable message about temp file creation (if applicable) - `error`: Error message if the operation failed +**Content Filtering**: By default, this tool filters out changes to files with `.lock` and `.log` extensions to reduce noise in diffs. This includes: + +- Lock files: `package-lock.json`, `yarn.lock`, `composer.lock`, etc. +- Log files: `debug.log`, `error.log`, `application.log`, etc. + +You can customize the filtering by providing a `filter_extensions` list, or disable filtering entirely by passing an empty list `[]`. + **Note on Large Diffs**: To prevent overwhelming LLMs with extremely large diffs, this tool automatically saves diffs larger than `max_size_kb` (default: 100KB) to a temporary file and returns the file path instead of the content. This allows you to process large merge request diffs without hitting token limits. ### run_ci_pipeline diff --git a/servers/gitlab_glab/src/mcp_server_gitlab_glab/server.py b/servers/gitlab_glab/src/mcp_server_gitlab_glab/server.py index b661255..f239246 100644 --- a/servers/gitlab_glab/src/mcp_server_gitlab_glab/server.py +++ b/servers/gitlab_glab/src/mcp_server_gitlab_glab/server.py @@ -310,6 +310,63 @@ class GitLabServer: logger.error(f"Failed to extract issue URL from output: {result}") return {"error": "Failed to extract issue URL from command output"} + def filter_diff_content( + self, + diff_content: str, + exclude_extensions: list[str], + ) -> str: + """Filter out files with specified extensions from diff content. + + Args: + diff_content: The original diff content. + exclude_extensions: List of file extensions to exclude + (e.g., [".lock", ".log"]). + + Returns: + Filtered diff content with excluded files removed. + """ + if not exclude_extensions: + return diff_content + + lines = diff_content.split('\n') + filtered_lines = [] + skip_block = False + + for line in lines: + if line.startswith("diff --git a/"): + parts = line.split() + if len(parts) >= 4: + file_a = parts[2] + file_b = parts[3] + # Check if either file should be excluded + should_exclude = False + for ext in exclude_extensions: + # Handle both exact extension matches and lock file patterns + if ext == ".lock": + # Special handling for lock files + # (package-lock.json, yarn.lock, etc.) + if (file_a.endswith('.lock') or + file_b.endswith('.lock') or + 'lock.' in file_a or 'lock.' in file_b or + file_a.endswith('-lock.json') or + file_b.endswith('-lock.json')): + should_exclude = True + break + else: + # Standard extension matching + if file_a.endswith(ext) or file_b.endswith(ext): + should_exclude = True + break + + skip_block = should_exclude + else: + skip_block = False + + if not skip_block: + filtered_lines.append(line) + + return '\n'.join(filtered_lines) + def get_mr_diff( self, working_directory: str, @@ -318,6 +375,7 @@ class GitLabServer: raw: bool = False, repo: str | None = None, max_size_kb: int = 100, + filter_extensions: list[str] | None = None, ) -> dict[str, Any]: """Get the diff for a merge request. @@ -329,11 +387,17 @@ class GitLabServer: repo: Select another repository (OWNER/REPO or GROUP/NAMESPACE/REPO format). max_size_kb: Maximum size in KB before saving to temporary file (default: 100). + filter_extensions: List of file extensions to exclude from diff + (default: [".lock", ".log"]). Returns: A dictionary containing the diff content or a path to temporary file if too large. """ + # Set default filter extensions if not provided + if filter_extensions is None: + filter_extensions = [".lock", ".log"] + # Build command arguments args = ["mr", "diff"] @@ -359,8 +423,10 @@ class GitLabServer: if not success: return result + # Apply filtering to remove unwanted file extensions + diff_content = self.filter_diff_content(result, filter_extensions) + # Check if the diff is too large - diff_content = result diff_size_kb = len(diff_content.encode('utf-8')) / 1024 if diff_size_kb > max_size_kb: @@ -664,6 +730,7 @@ def create_server(host: str = "127.0.0.1", port: int = 8080) -> FastMCP: raw: bool = False, repo: str | None = None, max_size_kb: int = 100, + filter_extensions: list[str] | None = None, ) -> dict[str, Any]: """Get the diff for a merge request. @@ -675,6 +742,8 @@ def create_server(host: str = "127.0.0.1", port: int = 8080) -> FastMCP: repo: Select another repository (OWNER/REPO or GROUP/NAMESPACE/REPO format). max_size_kb: Maximum size in KB before saving to temporary file (default: 100). + filter_extensions: List of file extensions to exclude from diff + (default: [".lock", ".log"]). Returns: A dictionary containing the diff content or a path to temporary file @@ -687,6 +756,7 @@ def create_server(host: str = "127.0.0.1", port: int = 8080) -> FastMCP: raw=raw, repo=repo, max_size_kb=max_size_kb, + filter_extensions=filter_extensions, ) @mcp.tool() diff --git a/servers/gitlab_glab/tests/test_server.py b/servers/gitlab_glab/tests/test_server.py index 867b2c0..13f90af 100644 --- a/servers/gitlab_glab/tests/test_server.py +++ b/servers/gitlab_glab/tests/test_server.py @@ -1020,3 +1020,290 @@ index 1234567..abcdefg 100644 ["ci", "run"], working_dir, ) + + @patch.object(GitLabServer, "execute_glab_command") + def test_get_mr_diff_with_default_filtering(self, mock_execute: MagicMock) -> None: + """Test MR diff with default filtering (.lock and .log files).""" + # Mock diff content with .lock and .log files + diff_content = """diff --git a/package-lock.json b/package-lock.json +index 1234567..abcdefg 100644 +--- a/package-lock.json ++++ b/package-lock.json +@@ -1,3 +1,4 @@ + { + "name": "test", ++ "version": "1.0.0", + "lockfileVersion": 1 +} +diff --git a/debug.log b/debug.log +index 1234567..abcdefg 100644 +--- a/debug.log ++++ b/debug.log +@@ -1,2 +1,3 @@ + [INFO] Starting application ++[INFO] Loading configuration + [INFO] Application ready +diff --git a/src/main.py b/src/main.py +index 1234567..abcdefg 100644 +--- a/src/main.py ++++ b/src/main.py +@@ -1,3 +1,4 @@ + def main(): + print("Hello") ++ print("World") + return 0""" + mock_execute.return_value = (True, diff_content) + + server = GitLabServer() + working_dir = "/test/directory" + result = server.get_mr_diff(working_directory=working_dir) + + # Check that .lock and .log files are filtered out + filtered_diff = result["diff"] + assert "package-lock.json" not in filtered_diff + assert "debug.log" not in filtered_diff + assert "src/main.py" in filtered_diff + assert 'print("World")' in filtered_diff + + @patch.object(GitLabServer, "execute_glab_command") + def test_get_mr_diff_with_custom_filtering(self, mock_execute: MagicMock) -> None: + """Test MR diff with custom filtering extensions.""" + # Mock diff content with various file types + diff_content = """diff --git a/test.txt b/test.txt +index 1234567..abcdefg 100644 +--- a/test.txt ++++ b/test.txt +@@ -1,2 +1,3 @@ + line 1 ++new line + line 2 +diff --git a/config.json b/config.json +index 1234567..abcdefg 100644 +--- a/config.json ++++ b/config.json +@@ -1,3 +1,4 @@ + { + "setting": "value" ++ "new_setting": "new_value" + } +diff --git a/temp.tmp b/temp.tmp +index 1234567..abcdefg 100644 +--- a/temp.tmp ++++ b/temp.tmp +@@ -1,2 +1,3 @@ + temp data ++more temp data + end""" + mock_execute.return_value = (True, diff_content) + + server = GitLabServer() + working_dir = "/test/directory" + result = server.get_mr_diff( + working_directory=working_dir, + filter_extensions=[".tmp", ".json"] + ) + + # Check that .tmp and .json files are filtered out + filtered_diff = result["diff"] + assert "temp.tmp" not in filtered_diff + assert "config.json" not in filtered_diff + assert "test.txt" in filtered_diff + assert "new line" in filtered_diff + + @patch.object(GitLabServer, "execute_glab_command") + def test_get_mr_diff_with_no_filtering(self, mock_execute: MagicMock) -> None: + """Test MR diff with filtering disabled (empty list).""" + # Mock diff content with .lock file + diff_content = """diff --git a/package-lock.json b/package-lock.json +index 1234567..abcdefg 100644 +--- a/package-lock.json ++++ b/package-lock.json +@@ -1,3 +1,4 @@ + { + "name": "test", ++ "version": "1.0.0", + "lockfileVersion": 1 +}""" + mock_execute.return_value = (True, diff_content) + + server = GitLabServer() + working_dir = "/test/directory" + result = server.get_mr_diff( + working_directory=working_dir, + filter_extensions=[] + ) + + # Check that no filtering occurred + filtered_diff = result["diff"] + assert "package-lock.json" in filtered_diff + assert '"version": "1.0.0"' in filtered_diff + + def test_filter_diff_content_basic(self) -> None: + """Test basic diff content filtering.""" + server = GitLabServer() + + diff_content = """diff --git a/src/main.py b/src/main.py +index 1234567..abcdefg 100644 +--- a/src/main.py ++++ b/src/main.py +@@ -1,3 +1,4 @@ + def main(): + print("Hello") ++ print("World") + return 0 +diff --git a/package-lock.json b/package-lock.json +index 1234567..abcdefg 100644 +--- a/package-lock.json ++++ b/package-lock.json +@@ -1,3 +1,4 @@ + { + "name": "test", ++ "version": "1.0.0", + "lockfileVersion": 1 +}""" + + filtered = server.filter_diff_content(diff_content, [".lock"]) + + assert "src/main.py" in filtered + assert 'print("World")' in filtered + assert "package-lock.json" not in filtered + assert '"version": "1.0.0"' not in filtered + + def test_filter_diff_content_multiple_extensions(self) -> None: + """Test filtering with multiple extensions.""" + server = GitLabServer() + + diff_content = """diff --git a/app.py b/app.py +index 1234567..abcdefg 100644 +--- a/app.py ++++ b/app.py +@@ -1,2 +1,3 @@ + import logging ++logging.basicConfig() + app = Flask(__name__) +diff --git a/debug.log b/debug.log +index 1234567..abcdefg 100644 +--- a/debug.log ++++ b/debug.log +@@ -1,2 +1,3 @@ + [INFO] Starting ++[DEBUG] Debug message + [INFO] Ready +diff --git a/yarn.lock b/yarn.lock +index 1234567..abcdefg 100644 +--- a/yarn.lock ++++ b/yarn.lock +@@ -1,3 +1,4 @@ + # THIS IS AN AUTOGENERATED FILE ++# Dependencies + package@1.0.0: + version "1.0.0""" + + filtered = server.filter_diff_content(diff_content, [".log", ".lock"]) + + assert "app.py" in filtered + assert "logging.basicConfig()" in filtered + assert "debug.log" not in filtered + assert "yarn.lock" not in filtered + assert "[DEBUG] Debug message" not in filtered + assert "# Dependencies" not in filtered + + def test_filter_diff_content_empty_extensions(self) -> None: + """Test filtering with empty extensions list.""" + server = GitLabServer() + + diff_content = """diff --git a/package-lock.json b/package-lock.json +index 1234567..abcdefg 100644 +--- a/package-lock.json ++++ b/package-lock.json +@@ -1,2 +1,3 @@ + { ++ "name": "test", + "lockfileVersion": 1 +}""" + + filtered = server.filter_diff_content(diff_content, []) + + # Should return original content when no extensions to filter + assert filtered == diff_content + + def test_filter_diff_content_malformed_diff_header(self) -> None: + """Test filtering with malformed diff headers.""" + server = GitLabServer() + + diff_content = """diff --git a/file.py +index 1234567..abcdefg 100644 +--- a/file.py ++++ b/file.py +@@ -1,2 +1,3 @@ + line 1 ++new line + line 2 +diff --git a/package-lock.json b/package-lock.json +index 1234567..abcdefg 100644 +--- a/package-lock.json ++++ b/package-lock.json +@@ -1,2 +1,3 @@ + { ++ "version": "1.0.0", + "lockfileVersion": 1 +}""" + + filtered = server.filter_diff_content(diff_content, [".lock"]) + + # Malformed header should not be filtered + assert "diff --git a/file.py" in filtered + assert "new line" in filtered + # Properly formed header with .lock extension should be filtered + assert "package-lock.json" not in filtered + assert '"version": "1.0.0"' not in filtered + + @patch.object(GitLabServer, "execute_glab_command") + def test_get_mr_diff_filtering_with_large_file(self, mock_execute: MagicMock) -> None: + """Test that filtering works correctly even when creating temp files.""" + # Create a large diff that will still be large after filtering + large_diff_base = """diff --git a/src/main.py b/src/main.py +index 1234567..abcdefg 100644 +--- a/src/main.py ++++ b/src/main.py +@@ -1,3 +1,4 @@ + def main(): + print("Hello") ++ print("World") + return 0 +""" + # Add a lot of content to main.py to make it large + large_diff = large_diff_base + "+ # " + "Large comment line\n" * 3000 + + # Add package-lock.json content that should be filtered out + large_diff += """diff --git a/package-lock.json b/package-lock.json +index 1234567..abcdefg 100644 +--- a/package-lock.json ++++ b/package-lock.json +@@ -1,500 +1,501 @@ + { + "name": "test", ++ "version": "1.0.0", +""" + " \"dependency\": \"^1.0.0\",\n" * 1000 + "}\n" + + mock_execute.return_value = (True, large_diff) + + server = GitLabServer() + working_dir = "/test/directory" + + with patch("tempfile.NamedTemporaryFile") as mock_temp: + mock_file = MagicMock() + mock_file.name = "/tmp/test_diff.diff" + mock_temp.return_value.__enter__.return_value = mock_file + + result = server.get_mr_diff( + working_directory=working_dir, + max_size_kb=1 # Force temp file creation + ) + + # Should still filter before saving to temp file + assert result["diff_too_large"] is True + # The filtered content should be written to temp file + written_content = mock_file.write.call_args[0][0] + assert "src/main.py" in written_content + assert "package-lock.json" not in written_content -- cgit v1.2.3