Skip to content

feat: bootstrap MCP server framework (bootstrap_mcp_framework) - #145

Open
wryyyds7 wants to merge 2 commits into
antgroup:mainfrom
wryyyds7:feat/oss26-mcp-framework-129
Open

feat: bootstrap MCP server framework (bootstrap_mcp_framework)#145
wryyyds7 wants to merge 2 commits into
antgroup:mainfrom
wryyyds7:feat/oss26-mcp-framework-129

Conversation

@wryyyds7

Copy link
Copy Markdown

Implement the unified MCP Server framework as the runtime base for all upcoming YASA MCP tools (Issue #129).

Framework:

  • Dual transport: stdio (default) / streamable-http
  • Tool registration via @mcp_tool decorator + auto-discovery
  • Unified input validation (Pydantic), error handling, logging (stderr)
  • Health check endpoint GET /healthz in HTTP mode
  • Built-in demo tool ping returning server status

Modules:

  • yasa_mcp/config.py: CLI args + env var parsing
  • yasa_mcp/server.py: FastMCP lifecycle management
  • yasa_mcp/registry.py: decorator-based tool auto-registration
  • yasa_mcp/transport/http.py: streamable-http + /healthz
  • yasa_mcp/tools/ping.py: demo health-check tool
  • yasa_mcp/errors.py: unified error codes
  • yasa_mcp/logging_config.py: stderr logging with level control

Tests (44 passing):

  • test_registry.py: tool registration, auto-discovery, schema
  • test_validation.py: param validation, error handling
  • test_logging.py: stderr output, format, level filtering
  • test_config.py: CLI parsing, env vars, error exit

Docs:

  • docs/mcp/requirements.md: functional/non-functional requirements
  • docs/mcp/system-design.md: architecture, module design, data flow
  • docs/mcp/development-guide.md: step-by-step implementation guide
  • yasa_mcp/README.md: usage + Claude Desktop/Cline config examples

Implement the unified MCP Server framework as the runtime base for
all upcoming YASA MCP tools (Issue antgroup#129).

Framework:
- Dual transport: stdio (default) / streamable-http
- Tool registration via @mcp_tool decorator + auto-discovery
- Unified input validation (Pydantic), error handling, logging (stderr)
- Health check endpoint GET /healthz in HTTP mode
- Built-in demo tool ping returning server status

Modules:
- yasa_mcp/config.py: CLI args + env var parsing
- yasa_mcp/server.py: FastMCP lifecycle management
- yasa_mcp/registry.py: decorator-based tool auto-registration
- yasa_mcp/transport/http.py: streamable-http + /healthz
- yasa_mcp/tools/ping.py: demo health-check tool
- yasa_mcp/errors.py: unified error codes
- yasa_mcp/logging_config.py: stderr logging with level control

Tests (44 passing):
- test_registry.py: tool registration, auto-discovery, schema
- test_validation.py: param validation, error handling
- test_logging.py: stderr output, format, level filtering
- test_config.py: CLI parsing, env vars, error exit

Docs:
- docs/mcp/requirements.md: functional/non-functional requirements
- docs/mcp/system-design.md: architecture, module design, data flow
- docs/mcp/development-guide.md: step-by-step implementation guide
- yasa_mcp/README.md: usage + Claude Desktop/Cline config examples

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request bootstraps the YASA MCP Server framework, establishing a unified base for program analysis tools with support for both stdio and HTTP transport modes, a dynamic tool registration and discovery mechanism, and a comprehensive test suite. The reviewer provided valuable feedback to enhance robustness and code quality, including: preventing duplicate tool registration by validating module names; using Starlette's public add_route API instead of directly modifying app.routes; passing the configured CLI log level to the HTTP server; resolving a potential fallback bug with empty string messages in errors.py; converting repo_root to an absolute path to avoid path resolution issues; refining the logger namespace check; and cleaning up unused imports, deprecated event loop calls, and placeholder comments.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread yasa_mcp/registry.py Outdated
Comment on lines +92 to +94
for attr_name in dir(module): # 遍历模块中的所有属性
attr = getattr(module, attr_name)
if callable(attr) and hasattr(attr, _MCP_TOOL_META_ATTR): # 找到可调用且有mcptoolmeta的属性的对象

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

To prevent duplicate tool registration, ensure that the function being registered was actually defined in the module currently being scanned. If a tool module imports another tool (e.g., from yasa_mcp.tools.ping import ping), dir(module) will contain that imported tool, causing it to be registered multiple times. Checking getattr(attr, '__module__', None) == full_name resolves this issue.

        for attr_name in dir(module):  # 遍历模块中的所有属性
            attr = getattr(module, attr_name)
            if (
                callable(attr)
                and hasattr(attr, _MCP_TOOL_META_ATTR)
                and getattr(attr, "__module__", None) == full_name
            ):  # 找到在本模块中定义且有mcptoolmeta属性的对象

Comment thread yasa_mcp/transport/http.py Outdated

from fastmcp import FastMCP
from starlette.responses import JSONResponse
from starlette.routing import Route

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Remove the unused Route import since /healthz can be registered using app.add_route.

Comment thread yasa_mcp/transport/http.py Outdated
Comment on lines +53 to +55
# 追加 /healthz 路由
health_route = Route("/healthz", healthz, methods=["GET"])
app.routes.insert(0, health_route)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Directly manipulating app.routes is an implementation detail of Starlette. It is safer and cleaner to use the public add_route API of the Starlette application, which also allows removing the unused Route import from starlette.routing.

Suggested change
# 追加 /healthz 路由
health_route = Route("/healthz", healthz, methods=["GET"])
app.routes.insert(0, health_route)
# 追加 /healthz 路由
app.add_route("/healthz", healthz, methods=["GET"])

Comment thread yasa_mcp/transport/http.py Outdated
return app


async def run_http(mcp: FastMCP, port: int, host: str = "0.0.0.0") -> None:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Accept the log_level parameter in run_http so that the ASGI server can respect the user's CLI configuration.

Suggested change
async def run_http(mcp: FastMCP, port: int, host: str = "0.0.0.0") -> None:
async def run_http(mcp: FastMCP, port: int, host: str = "0.0.0.0", log_level: str = "info") -> None:

Comment on lines +75 to +80
config = uvicorn.Config(
app,
host=host,
port=port,
log_level="info",
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Pass the log_level parameter to uvicorn.Config instead of hardcoding it to 'info'.

Suggested change
config = uvicorn.Config(
app,
host=host,
port=port,
log_level="info",
)
config = uvicorn.Config(
app,
host=host,
port=port,
log_level=log_level,
)

Comment thread yasa_mcp/errors.py
code = "TOOL_NOT_FOUND"
message = "Requested tool is not registered"

# 还可以添加 No newline at end of file

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Remove the trailing placeholder comment # 还可以添加 to keep the production codebase clean.

Comment thread tests/mcp/test_registry.py Outdated
async def my_tool(x: int) -> int:
return x * 2

result = asyncio.get_event_loop().run_until_complete(my_tool(5))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

In Python 3.10+, asyncio.get_event_loop() can raise deprecation warnings or runtime errors if there is no running event loop. Use asyncio.run() instead, which is the modern and standard way to run coroutines in synchronous contexts.

Suggested change
result = asyncio.get_event_loop().run_until_complete(my_tool(5))
result = asyncio.run(my_tool(5))

Comment thread tests/mcp/test_validation.py Outdated
Comment on lines +17 to +20
from pydantic import ValidationError

from yasa_mcp.registry import mcp_tool, auto_discover_tools
from yasa_mcp.errors import ValidationError as YasaValidationError

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The imports ValidationError from pydantic and ValidationError as YasaValidationError from yasa_mcp.errors are unused in this test file. They should be removed to keep the code clean.

Suggested change
from pydantic import ValidationError
from yasa_mcp.registry import mcp_tool, auto_discover_tools
from yasa_mcp.errors import ValidationError as YasaValidationError
from yasa_mcp.registry import mcp_tool, auto_discover_tools

Comment thread yasa_mcp/config.py
)

args = parser.parse_args(argv)
repo_root = os.environ.get("YASA_MCP_REPO_ROOT", "")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Convert repo_root to an absolute path using os.path.abspath. If a relative path is provided via the YASA_MCP_REPO_ROOT environment variable, it can lead to bugs if the application changes its working directory or spawns subprocesses in different contexts.

    repo_root = os.environ.get("YASA_MCP_REPO_ROOT", "")
    if repo_root:
        repo_root = os.path.abspath(repo_root)

Comment thread yasa_mcp/logging_config.py Outdated
Comment on lines +42 to +43
if not name.startswith("yasa_mcp"):
name = f"yasa_mcp.{name}"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The check not name.startswith("yasa_mcp") is too loose. For example, a logger named "yasa_mcp_helper" would match this check and not be prefixed, even though it is not part of the "yasa_mcp" hierarchical namespace (which should be "yasa_mcp.helper"). Use a more precise check to ensure proper hierarchy.

Suggested change
if not name.startswith("yasa_mcp"):
name = f"yasa_mcp.{name}"
if name != "yasa_mcp" and not name.startswith("yasa_mcp."):
name = f"yasa_mcp.{name}"

@wryyyds7

Copy link
Copy Markdown
Author

wait,I don't finish it.

@wryyyds7

Copy link
Copy Markdown
Author

#129

- registry.py: prevent duplicate tool registration via __module__ check
- http.py: use app.add_route() instead of direct app.routes manipulation
- http.py: remove unused Route import
- http.py + server.py: pass log_level to uvicorn instead of hardcoding
- errors.py: fix empty string message fallback bug
- test_registry.py: use asyncio.run() instead of deprecated get_event_loop()
- test_validation.py: remove unused imports
- config.py: convert repo_root to absolute path
- logging_config.py: precise logger namespace check
- pyproject.toml: fix build-backend to setuptools.build_meta
- .gitignore: add .venv/ and Python project ignore rules
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant