Skip to content

fix: preserve ModelScope MCP transport #8270

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
he-yufeng wants to merge 1 commit into
base: master
Choose a base branch
from
+39 −2
Open

fix: preserve ModelScope MCP transport #8270

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 21 additions & 1 deletion astrbot/core/provider/func_tool_manager.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,24 @@ def _resolve_timeout(
FuncTool = FunctionTool


def _modelscope_mcp_transport_from_url_info(url_info: Mapping[str, Any]) -> str:
for key in ("transport", "transport_type", "type", "protocol"):
raw_value = url_info.get(key)
if isinstance(raw_value, Mapping):
raw_value = raw_value.get("type") or raw_value.get("name")
if isinstance(raw_value, str):
value = raw_value.strip().lower().replace("-", "_").replace(" ", "_")
if value in {"streamable_http", "streamablehttp", "http"}:
return "streamable_http"
if value in {"sse", "server_sent_events"}:
return "sse"

path = urllib.parse.urlparse(str(url_info.get("url", ""))).path.rstrip("/").lower()
if path.endswith("/sse"):
return "sse"
return "streamable_http"


def _prepare_config(config: dict) -> dict:
"""准备配置，处理嵌套格式"""
if config.get("mcpServers"):
Expand Down Expand Up @@ -949,7 +967,9 @@ async def sync_modelscope_mcp_servers(self, access_token: str) -> None:
# 添加到配置中(同名会覆盖)
local_mcp_config["mcpServers"][server_name] = {
"url": server_url,
"transport": "sse",
"transport": _modelscope_mcp_transport_from_url_info(
url_info
),
"active": True,
"provider": "modelscope",
}
Expand Down
19 changes: 18 additions & 1 deletion tests/unit/test_func_tool_manager.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,10 @@
import pytest

from astrbot.core import sp
from astrbot.core.provider.func_tool_manager import FunctionToolManager
from astrbot.core.provider.func_tool_manager import (
FunctionToolManager,
_modelscope_mcp_transport_from_url_info,
)
from astrbot.core.tools.computer_tools.shell import ExecuteShellTool
from astrbot.core.tools.message_tools import SendMessageToUserTool
from astrbot.core.tools.web_search_tools import (
Expand Down Expand Up @@ -293,6 +296,20 @@ def test_is_self_detached_command_handles_quotes_and_comments(command, expected)
assert _is_self_detached_command(command) is expected


@pytest.mark.parametrize(
("url_info", "expected"),
[
({"transport": "Streamable HTTP", "url": "https://example.com/mcp"}, "streamable_http"),
({"transport_type": "sse", "url": "https://example.com/mcp"}, "sse"),
({"type": "streamable_http", "url": "https://example.com/mcp"}, "streamable_http"),
({"url": "https://example.com/mcp/sse"}, "sse"),
({"url": "https://example.com/mcp"}, "streamable_http"),
],
Comment on lines +301 to +307
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot May 20, 2026

Choose a reason for hiding this comment

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

medium

The test coverage for _modelscope_mcp_transport_from_url_info is good, but it's missing cases for when the transport value is a nested dictionary. The implementation at astrbot/core/provider/func_tool_manager.py:152-153 handles this scenario, so it would be beneficial to add test cases for it to prevent future regressions. Specifically, the logic handling Mapping types (checking for type or name keys) is not covered. Please consider adding test cases to cover this path.

    [
        ({"transport": "Streamable HTTP", "url": "https://example.com/mcp"}, "streamable_http"),
        ({"transport_type": "sse", "url": "https://example.com/mcp"}, "sse"),
        ({"type": "streamable_http", "url": "https://example.com/mcp"}, "streamable_http"),
        ({"transport": {"type": "sse"}, "url": "https://example.com/mcp"}, "sse"),
        ({"protocol": {"name": "Streamable HTTP"}, "url": "https://example.com/mcp"}, "streamable_http"),
        ({"url": "https://example.com/mcp/sse"}, "sse"),
        ({"url": "https://example.com/mcp"}, "streamable_http"),
    ],
References
  1. New functionality should be accompanied by corresponding unit tests.

)
def test_modelscope_mcp_transport_from_url_info(url_info, expected):
assert _modelscope_mcp_transport_from_url_info(url_info) == expected


@pytest.mark.asyncio
async def test_execute_shell_reports_blank_exception_type(monkeypatch):
from astrbot.core.tools.computer_tools import shell as shell_tools
Expand Down
Loading