fix ToolServerBase transport param#20
Conversation
WalkthroughAdds import-time detection for whether ToolServerBase accepts a Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant App as Application
participant Mod as youtrack_mcp.server
participant SDK as mcp_sdk.ToolServerBase
participant FMC as mcp.server.fastmcp.ToolServerBase
Note over Mod: Import time detection
Mod->>SDK: try import ToolServerBase (preferred)
alt mcp_sdk present and inspected ok
Mod->>Mod: _TOOLSERVER_ACCEPTS_TRANSPORT = true
else fallback or error
Mod->>FMC: import legacy ToolServerBase
Mod->>Mod: _TOOLSERVER_ACCEPTS_TRANSPORT = false
end
Note over App,Mod: YouTrackMCPServer.__init__
App->>Mod: instantiate YouTrackMCPServer(transport=?)
Mod->>Mod: build server_kwargs (transport included only if flag true)
Mod->>SDK: ToolServerBase(**server_kwargs)
alt TypeError mentioning "transport"
Mod->>Mod: remove transport from server_kwargs
Mod->>SDK: ToolServerBase(**server_kwargs) -- retry without transport
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
youtrack_mcp/server.py (2)
13-23: Harden detection: introspect ToolServerBase.init for 'transport' instead of assuming by package namePackage presence ≠ signature guarantee. Use inspect.signature to check for a 'transport' kwarg after selecting the class; this avoids breakage across mcp_sdk/FastMCP versions.
try: - # Try importing from mcp_sdk (new package name) - from mcp_sdk.server import ToolServerBase - - _TOOLSERVER_ACCEPTS_TRANSPORT = True + # Try importing from mcp_sdk (new package name) + from mcp_sdk.server import ToolServerBase except ImportError: # Fall back to mcp (old package name) from mcp.server.fastmcp import FastMCP as ToolServerBase - _TOOLSERVER_ACCEPTS_TRANSPORT = False + +# Determine if ToolServerBase.__init__ accepts a 'transport' kwarg +try: + _TOOLSERVER_ACCEPTS_TRANSPORT = ( + "transport" in inspect.signature(ToolServerBase.__init__).parameters + ) +except Exception: + _TOOLSERVER_ACCEPTS_TRANSPORT = FalseIf you keep the heuristic, please verify against the versions you ship with:
- mcp_sdk ToolServerBase.init has 'transport' kwarg
- mcp.server.fastmcp.FastMCP.init does not
56-67: De-duplicate instantiation and optionally add a call-time fallbackBuild kwargs once and add 'transport' conditionally; optionally guard with a TypeError fallback for maximum resilience.
- if _TOOLSERVER_ACCEPTS_TRANSPORT: - self.server = ToolServerBase( - name=config.MCP_SERVER_NAME, - instructions=config.MCP_SERVER_DESCRIPTION, - transport=transport, - ) - else: - # Legacy FastMCP does not accept 'transport' - self.server = ToolServerBase( - name=config.MCP_SERVER_NAME, - instructions=config.MCP_SERVER_DESCRIPTION, - ) + server_kwargs = { + "name": config.MCP_SERVER_NAME, + "instructions": config.MCP_SERVER_DESCRIPTION, + } + if _TOOLSERVER_ACCEPTS_TRANSPORT: + server_kwargs["transport"] = transport + + # Simple path + try: + self.server = ToolServerBase(**server_kwargs) + except TypeError as e: + # Safety net if detection was wrong or runtime changes + if "transport" in server_kwargs and "transport" in str(e): + server_kwargs.pop("transport", None) + self.server = ToolServerBase(**server_kwargs) + else: + raise
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
youtrack_mcp/server.py(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
youtrack_mcp/server.py (1)
src/bin/youtrack-mcp.js (1)
server(95-95)
waiil
force-pushed
the
feature/fix-transport-param
branch
from
64689af to
3e6d76a
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
youtrack_mcp/server.py (1)
20-27: Narrow exception; detect VAR_KEYWORD to avoid false negatives.Catching Exception trips Ruff BLE001 and can hide real errors. Also consider accepting transport when init has **kwargs.
-# Determine if ToolServerBase.__init__ accepts a 'transport' kwarg -try: - _TOOLSERVER_ACCEPTS_TRANSPORT = ( - "transport" in inspect.signature(ToolServerBase.__init__).parameters - ) -except Exception: - _TOOLSERVER_ACCEPTS_TRANSPORT = False +# Determine if ToolServerBase.__init__ accepts a 'transport' kwarg +try: + init_params = inspect.signature(ToolServerBase.__init__).parameters + _TOOLSERVER_ACCEPTS_TRANSPORT = ( + "transport" in init_params + or any(p.kind == inspect.Parameter.VAR_KEYWORD for p in init_params.values()) + ) +except (ValueError, TypeError): + _TOOLSERVER_ACCEPTS_TRANSPORT = False
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
youtrack_mcp/server.py(2 hunks)
🧰 Additional context used
🪛 Ruff (0.12.2)
youtrack_mcp/server.py
25-25: Do not catch blind exception: Exception
(BLE001)
🔇 Additional comments (1)
youtrack_mcp/server.py (1)
58-74: Verify symmetric transport fallback and add logging
Sandbox imports ofToolServerBasecouldn’t be introspected, so please confirm in your runtime that:
- When detection misses a required
transport, you retry by copyingserver_kwargs, addingtransport, and logging a warning.- When detection falsely includes
transport, you retry by copyingserver_kwargs, removingtransport, and logging a warning.
Ensure you never mutate the originalserver_kwargsmap.
1 participant
Summary by CodeRabbit