feat(DATAGO-118651): Add MCP Server Gateway Adapter plugin #76
Conversation
- Implemented `sanitize_tool_name` to create valid MCP tool names. - Added `format_agent_skill_description` for generating human-readable skill descriptions. - Created `truncate_text` to limit text length with ellipsis. - Developed `create_session_id` for generating unique session IDs. - Introduced `extract_agent_skill_from_tool_name` to parse tool names into agent and skill components. - Added `should_include_tool` to filter tools based on include/exclude patterns. - Implemented helper functions `_split_patterns`, `_matches_exact_any`, and `_matches_regex_any` for pattern matching.
…er cleanup methods
cyrus2281
changed the title
There was a problem hiding this comment.
Pull request overview
This PR introduces a new MCP (Model Context Protocol) Server Gateway Adapter plugin that enables MCP-compatible clients to interact with SAM agents through a standardized interface. The adapter dynamically exposes SAM agents as MCP tools, supporting both HTTP and stdio transports with real-time response streaming.
Key changes:
- Implementation of FastMCP-based gateway adapter with dynamic agent/skill discovery
- OAuth 2.0 authentication flow with PKCE support for secure client access
- Intelligent file handling system that returns content inline or as resources based on type/size
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| sam-mcp-server-gateway-adapter/src/sam_mcp_server_gateway_adapter/adapter.py | Core adapter implementing MCP server, OAuth flows, tool registration, and response handling |
| sam-mcp-server-gateway-adapter/src/sam_mcp_server_gateway_adapter/utils.py | Utility functions for tool name sanitization, filtering, validation, and access control |
| sam-mcp-server-gateway-adapter/src/sam_mcp_server_gateway_adapter/init.py | Package initialization exporting McpAdapter class |
| sam-mcp-server-gateway-adapter/pyproject.toml | Package metadata and dependencies (fastmcp, starlette) |
| sam-mcp-server-gateway-adapter/config.yaml | Example configuration with transport, auth, filtering, and artifact settings |
| sam-mcp-server-gateway-adapter/README.md | Comprehensive documentation covering architecture, configuration, and usage patterns |
Comments suppressed due to low confidence (1)
sam-mcp-server-gateway-adapter/src/sam_mcp_server_gateway_adapter/adapter.py:1
- Missing finally block implementation. The code shows line 1299 with 'finally:' but the actual cleanup code is not visible in the diff. If cleanup is needed (as suggested by line 1108's comment), ensure
_cleanup_task(task_id)is called in the finally block to prevent resource leaks when exceptions occur.
"""
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Signal consumer to stop | ||
| try: | ||
| await stream_queue.put(STREAM_COMPLETE) | ||
| except: |
There was a problem hiding this comment.
Bare except clause catches all exceptions including system-exiting exceptions like KeyboardInterrupt and SystemExit. Replace with a specific exception type or at minimum use except Exception:.
| # Create Future and Queue for this task | ||
| task_future = asyncio.Future() | ||
| stream_queue = asyncio.Queue() | ||
| task_id = None # Initialize to None so finally block can check it |
There was a problem hiding this comment.
The comment mentions a 'finally block' but there is no finally block in this function. Either add the finally block for proper cleanup or remove this misleading comment.
|
|
||
| # Validate PKCE (required when require_pkce is enabled) | ||
| code_challenge = code_data.get("code_challenge") | ||
| config: McpAdapterConfig = self.context.adapter_config |
There was a problem hiding this comment.
The config is retrieved multiple times within the same function (lines 808, 1103). Consider retrieving it once at the beginning of the function and reusing the variable to improve readability and avoid redundant lookups.
- Added McpAdapterAuthHandler to manage OAuth 2.0 authorization, callback, and token exchange. - Introduced McpAdapterConfig for configuration management, including OAuth settings. - Created ListingFilterMiddleware to filter tools based on user permissions and access. - Implemented dynamic client registration and metadata endpoints for OAuth. - Added cleanup mechanisms for expired OAuth codes and states to prevent memory leaks. - Enhanced logging for better traceability of OAuth operations and user access filtering.
1 participant
What is the purpose of this change?
This PR adds a new MCP (Model Context Protocol) Server Gateway Adapter for SAM, allowing any MCP-compatible client (like Claude Desktop, IDEs, etc.) to interact with SAM agents through a standardized interface.
How is this accomplished?
The adapter:
Anything reviews should focus on/be aware of?
Review the authentication flow implementation to ensure security best practices are followed. The tool filtering logic is somewhat complex with both regex and exact matching; confirm it works as expected across edge cases.