Skip to content

Commit 02f3fe1

Browse files
committed
fix(server): Fix missing FastMCP v2 migration, and add more non-regression tests
1 parent 78ef7ac commit 02f3fe1

File tree

3 files changed

+197
-8
lines changed

3 files changed

+197
-8
lines changed

DEVELOPMENT.md

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -22,18 +22,22 @@ This document provides instructions for developers who want to contribute to the
2222

2323
### Pre-commit Hooks
2424

25-
This project uses [pre-commit](https://pre-commit.com/) to ensure code quality and security standards. The hooks are configured in `.pre-commit-config.yaml` and include:
25+
This project uses [pre-commit](https://pre-commit.com/) to ensure code quality and security standards. The hooks are
26+
configured in `.pre-commit-config.yaml` and include:
2627

2728
**On every commit:**
29+
2830
- **Ruff** - Automatically lints and formats Python code
2931
- **Commitizen** - Validates commit message format
3032
- **GitGuardian ggshield** - Scans for secrets in staged files
3133

3234
**On every push:**
35+
3336
- **Commitizen-branch** - Validates branch naming conventions
3437
- **GitGuardian ggshield-push** - Scans all commits being pushed for secrets
3538

36-
The hooks will automatically run before commits/pushes and will block the operation if any issues are found. You can also run the hooks manually:
39+
The hooks will automatically run before commits/pushes and will block the operation if any issues are found. You can
40+
also run the hooks manually:
3741

3842
```bash
3943
# Run all hooks on all files
@@ -79,6 +83,7 @@ To add a new tool to the MCP server:
7983
from fastmcp import Request, Response, Tool
8084
from typing import Dict, Any
8185

86+
8287
class ExampleTool(Tool):
8388
"""Example tool implementation."""
8489

@@ -114,6 +119,7 @@ class ExampleTool(Tool):
114119
data={"result": result}
115120
)
116121

122+
117123
# List of tools to be exported
118124
tools = [ExampleTool()]
119125
```
@@ -126,7 +132,7 @@ from example.tools import tools as example_tools
126132

127133
# Register the tools
128134
for tool in example_tools:
129-
mcp.add_tool(tool)
135+
mcp.tool(tool)
130136
```
131137

132138
## Testing
@@ -153,7 +159,8 @@ Create test files in the `tests/` directory that match the pattern `test_*.py`.
153159

154160
## Code Style
155161

156-
This project uses `ruff` for linting and formatting. While pre-commit hooks will automatically run ruff on your staged files, you can also run it manually:
162+
This project uses `ruff` for linting and formatting. While pre-commit hooks will automatically run ruff on your staged
163+
files, you can also run it manually:
157164

158165
```bash
159166
# Check for linting issues
@@ -166,13 +173,15 @@ ruff check --fix src tests
166173
ruff format src tests
167174
```
168175

169-
**Note:** Pre-commit hooks will automatically run ruff on your staged files when you commit, so you usually don't need to run it manually.
176+
**Note:** Pre-commit hooks will automatically run ruff on your staged files when you commit, so you usually don't need
177+
to run it manually.
170178

171179
## Cursor Rules
172180

173181
This project includes Cursor IDE rules in the `.cursor/rules` directory that enforce coding standards:
174182

175-
1. **Don't use uvicorn or fastapi with MCP** - MCP has its own server implementation, external web servers are not needed
183+
1. **Don't use uvicorn or fastapi with MCP** - MCP has its own server implementation, external web servers are not
184+
needed
176185
2. **Use pyproject.toml with uv** - Modern Python projects should use pyproject.toml with uv for dependency management
177186

178187
These rules help maintain consistent code quality and follow best practices for MCP development.
@@ -198,7 +207,8 @@ When adding a new tool, please document it in the README.md following the same s
198207
5. Push your changes (pre-push hooks will scan for secrets and validate branch names)
199208
6. Submit a pull request with a clear description of your changes
200209

201-
**Note:** The pre-commit and pre-push hooks will automatically check your code quality, commit messages, and scan for secrets before allowing commits and pushes.
210+
**Note:** The pre-commit and pre-push hooks will automatically check your code quality, commit messages, and scan for
211+
secrets before allowing commits and pushes.
202212

203213
## Releasing
204214

packages/secops_mcp_server/src/secops_mcp_server/server.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -197,7 +197,7 @@ async def get_current_token_info() -> dict[str, Any]:
197197
required_scopes=["incidents:write"],
198198
)
199199

200-
mcp.add_tool(
200+
mcp.tool(
201201
create_code_fix_request,
202202
description="Create code fix requests for multiple secret incidents with their locations. This will generate pull requests to automatically remediate the detected secrets.",
203203
required_scopes=["incidents:write"],

tests/test_server_profiles.py

Lines changed: 179 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,179 @@
1+
"""Test that server modules can be imported and initialized for both profiles.
2+
3+
This test ensures that both developer and secops server modules can be
4+
successfully imported and that their tool registration doesn't have syntax errors
5+
(like using mcp.add_tool instead of mcp.tool).
6+
"""
7+
8+
import sys
9+
from unittest.mock import AsyncMock, MagicMock, patch
10+
11+
import pytest
12+
13+
14+
@pytest.fixture
15+
def mock_env_no_http():
16+
"""Mock environment variables to prevent HTTP server from starting."""
17+
with patch.dict("os.environ", {"MCP_PORT": ""}, clear=False):
18+
yield
19+
20+
21+
@pytest.fixture
22+
def mock_gitguardian_modules():
23+
"""Mock GitGuardian API modules to avoid actual API calls during import."""
24+
with (
25+
patch("gg_api_core.mcp_server.get_client") as mock_get_client,
26+
patch("gg_api_core.scopes.set_developer_scopes") as mock_set_dev_scopes,
27+
patch("gg_api_core.scopes.set_secops_scopes") as mock_set_secops_scopes,
28+
):
29+
# Mock client
30+
mock_client = MagicMock()
31+
mock_client.get_current_token_info = AsyncMock(return_value={"scopes": ["scan"]})
32+
mock_get_client.return_value = mock_client
33+
34+
yield {
35+
"get_client": mock_get_client,
36+
"set_dev_scopes": mock_set_dev_scopes,
37+
"set_secops_scopes": mock_set_secops_scopes,
38+
}
39+
40+
41+
def clean_module_imports(module_name: str):
42+
"""Remove a module and its submodules from sys.modules."""
43+
modules_to_remove = [key for key in sys.modules if key.startswith(module_name)]
44+
for module in modules_to_remove:
45+
del sys.modules[module]
46+
47+
48+
class TestServerProfiles:
49+
"""Test that both server profiles can be initialized successfully."""
50+
51+
def test_developer_server_imports_successfully(self, mock_gitguardian_modules, mock_env_no_http):
52+
"""Test that the developer server module can be imported without errors.
53+
54+
This test would catch issues like:
55+
- Using mcp.add_tool instead of mcp.tool
56+
- Syntax errors in tool registration
57+
- Missing imports
58+
"""
59+
# Clean any previous imports
60+
clean_module_imports("developer_mcp_server")
61+
62+
try:
63+
# Import the developer server module
64+
import developer_mcp_server.server as dev_server
65+
66+
# Verify the server was created
67+
assert hasattr(dev_server, "mcp")
68+
assert dev_server.mcp is not None
69+
70+
# Verify it's a GitGuardianFastMCP instance
71+
from gg_api_core.mcp_server import GitGuardianFastMCP
72+
73+
assert isinstance(dev_server.mcp, GitGuardianFastMCP)
74+
75+
# Verify the server has the expected name
76+
assert dev_server.mcp.name == "GitGuardian Developer"
77+
78+
except AttributeError as e:
79+
if "add_tool" in str(e):
80+
pytest.fail(f"Developer server is using mcp.add_tool instead of mcp.tool: {e}")
81+
raise
82+
except Exception as e:
83+
pytest.fail(f"Failed to import developer server: {e}")
84+
85+
def test_secops_server_imports_successfully(self, mock_gitguardian_modules, mock_env_no_http):
86+
"""Test that the secops server module can be imported without errors.
87+
88+
This test would catch issues like:
89+
- Using mcp.add_tool instead of mcp.tool
90+
- Syntax errors in tool registration
91+
- Missing imports
92+
"""
93+
# Clean any previous imports
94+
clean_module_imports("secops_mcp_server")
95+
96+
try:
97+
# Import the secops server module
98+
import secops_mcp_server.server as secops_server
99+
100+
# Verify the server was created
101+
assert hasattr(secops_server, "mcp")
102+
assert secops_server.mcp is not None
103+
104+
# Verify it's a GitGuardianFastMCP instance
105+
from gg_api_core.mcp_server import GitGuardianFastMCP
106+
107+
assert isinstance(secops_server.mcp, GitGuardianFastMCP)
108+
109+
# Verify the server has the expected name
110+
assert secops_server.mcp.name == "GitGuardian SecOps"
111+
112+
except AttributeError as e:
113+
if "add_tool" in str(e):
114+
pytest.fail(f"SecOps server is using mcp.add_tool instead of mcp.tool: {e}")
115+
raise
116+
except Exception as e:
117+
pytest.fail(f"Failed to import secops server: {e}")
118+
119+
@pytest.mark.asyncio
120+
async def test_developer_server_tools_registered(self, mock_gitguardian_modules, mock_env_no_http):
121+
"""Test that developer server has tools registered properly."""
122+
# Clean any previous imports
123+
clean_module_imports("developer_mcp_server")
124+
125+
import developer_mcp_server.server as dev_server
126+
127+
# Mock the _fetch_token_scopes to avoid actual API calls
128+
dev_server.mcp._fetch_token_scopes = AsyncMock()
129+
dev_server.mcp._token_scopes = {"scan", "incidents:read"}
130+
131+
# List tools - this would fail if any tool was registered incorrectly
132+
tools = await dev_server.mcp.list_tools()
133+
134+
# Verify we have some tools registered
135+
assert len(tools) > 0, "Developer server should have tools registered"
136+
137+
@pytest.mark.asyncio
138+
async def test_secops_server_tools_registered(self, mock_gitguardian_modules, mock_env_no_http):
139+
"""Test that secops server has tools registered properly."""
140+
# Clean any previous imports
141+
clean_module_imports("secops_mcp_server")
142+
143+
import secops_mcp_server.server as secops_server
144+
145+
# Mock the _fetch_token_scopes to avoid actual API calls
146+
secops_server.mcp._fetch_token_scopes = AsyncMock()
147+
secops_server.mcp._token_scopes = {"scan", "incidents:read", "incidents:write"}
148+
149+
# List tools - this would fail if any tool was registered incorrectly
150+
tools = await secops_server.mcp.list_tools()
151+
152+
# Verify we have some tools registered
153+
assert len(tools) > 0, "SecOps server should have tools registered"
154+
155+
# Verify some expected secops-specific tools are present
156+
tool_names = [tool.name for tool in tools]
157+
# Check for a secops-specific tool that should be registered
158+
assert "assign_incident" in tool_names, "SecOps server should have assign_incident tool"
159+
assert "create_code_fix_request" in tool_names, "SecOps server should have create_code_fix_request tool"
160+
161+
def test_both_servers_can_coexist(self, mock_gitguardian_modules, mock_env_no_http):
162+
"""Test that both server modules can be imported in the same test session.
163+
164+
This ensures there are no naming conflicts or import issues.
165+
"""
166+
# Clean any previous imports
167+
clean_module_imports("developer_mcp_server")
168+
clean_module_imports("secops_mcp_server")
169+
170+
try:
171+
import developer_mcp_server.server as dev_server
172+
import secops_mcp_server.server as secops_server
173+
174+
# Both servers should be distinct instances
175+
assert dev_server.mcp is not secops_server.mcp
176+
assert dev_server.mcp.name != secops_server.mcp.name
177+
178+
except Exception as e:
179+
pytest.fail(f"Failed to import both servers: {e}")

0 commit comments

Comments
 (0)