mirror of
https://github.com/HKUDS/nanobot.git
synced 2026-08-18 18:16:38 +03:00
fix(webui): surface MCP runtime connection failures (#5331)
This commit is contained in:
@@ -6,7 +6,7 @@ import asyncio
|
||||
from contextlib import AsyncExitStack
|
||||
from types import SimpleNamespace
|
||||
from typing import Any
|
||||
from unittest.mock import MagicMock
|
||||
from unittest.mock import AsyncMock, MagicMock
|
||||
|
||||
import anyio
|
||||
import pytest
|
||||
@@ -155,6 +155,31 @@ async def test_connect_mcp_retries_when_no_servers_connect(tmp_path, monkeypatch
|
||||
|
||||
assert attempts == 2
|
||||
assert loop._mcp_stacks == {}
|
||||
assert loop.mcp_runtime_status() == {"test": "failed"}
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_connect_mcp_does_not_report_failure_before_oauth_authorization(
|
||||
tmp_path,
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
):
|
||||
cfg = MCPServerConfig(
|
||||
type="streamableHttp",
|
||||
auth="oauth",
|
||||
url="https://mcp.example.com/mcp",
|
||||
)
|
||||
loop = _make_loop(tmp_path, mcp_servers={"oauth-app": cfg})
|
||||
connect = AsyncMock()
|
||||
monkeypatch.setattr("nanobot.agent.tools.mcp.connect_mcp_servers", connect)
|
||||
monkeypatch.setattr(
|
||||
"nanobot.agent.tools.mcp_oauth.mcp_oauth_has_credentials",
|
||||
lambda _name, _url: False,
|
||||
)
|
||||
|
||||
await loop._connect_mcp()
|
||||
|
||||
connect.assert_not_awaited()
|
||||
assert loop.mcp_runtime_status() == {}
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
|
||||
@@ -84,6 +84,10 @@ class _GatewayAgentContractStub:
|
||||
|
||||
tools = ToolRegistry()
|
||||
|
||||
@staticmethod
|
||||
def mcp_runtime_status() -> dict[str, str]:
|
||||
return {}
|
||||
|
||||
@staticmethod
|
||||
def pending_cron_job_ids_for_session(_session_key: str) -> set[str]:
|
||||
return set()
|
||||
|
||||
@@ -181,6 +181,72 @@ async def test_connect_missing_servers_propagates_external_cancellation(monkeypa
|
||||
assert state._mcp_connecting is False
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_saved_oauth_http_403_projects_failed_runtime_without_details(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
class OAuthAuth(httpx.Auth):
|
||||
def auth_flow(self, request: httpx.Request):
|
||||
request.headers["Authorization"] = "Bearer saved-oauth-secret"
|
||||
yield request
|
||||
|
||||
class AuthorizationRequiredError(RuntimeError):
|
||||
pass
|
||||
|
||||
async def create_auth(_name: str, _url: str, _handlers=None) -> httpx.Auth:
|
||||
return OAuthAuth()
|
||||
|
||||
@asynccontextmanager
|
||||
async def rejected_streamable_http(url: str, http_client=None):
|
||||
request = httpx.Request("POST", f"{url}?access_token=saved-oauth-secret")
|
||||
response = httpx.Response(403, request=request)
|
||||
raise httpx.HTTPStatusError(
|
||||
"403 Forbidden for saved-oauth-secret",
|
||||
request=request,
|
||||
response=response,
|
||||
)
|
||||
yield object(), object(), object()
|
||||
|
||||
async def reachable(_url: str) -> bool:
|
||||
return True
|
||||
|
||||
oauth_mod = ModuleType("nanobot.agent.tools.mcp_oauth")
|
||||
oauth_mod.MCPAuthorizationRequiredError = AuthorizationRequiredError # type: ignore[attr-defined]
|
||||
oauth_mod.create_mcp_oauth_auth = create_auth # type: ignore[attr-defined]
|
||||
oauth_mod.mcp_oauth_has_credentials = lambda _name, _url: True # type: ignore[attr-defined]
|
||||
monkeypatch.setitem(sys.modules, "nanobot.agent.tools.mcp_oauth", oauth_mod)
|
||||
monkeypatch.setattr(mcp_mod, "validate_url_target", lambda _url: (True, ""))
|
||||
monkeypatch.setattr(mcp_mod, "_probe_http_url", reachable)
|
||||
monkeypatch.setattr(
|
||||
sys.modules["mcp.client.streamable_http"],
|
||||
"streamable_http_client",
|
||||
rejected_streamable_http,
|
||||
)
|
||||
|
||||
class State:
|
||||
pass
|
||||
|
||||
state = State()
|
||||
state._mcp_closing = False
|
||||
state._mcp_servers = {
|
||||
"xmind": MCPServerConfig(
|
||||
type="streamableHttp",
|
||||
auth="oauth",
|
||||
url="https://app.xmind.com/api/mcp",
|
||||
)
|
||||
}
|
||||
state._mcp_stacks = {}
|
||||
state._mcp_runtime_statuses = {}
|
||||
state._mcp_connecting = False
|
||||
|
||||
await mcp_mod.connect_missing_servers(state, ToolRegistry())
|
||||
|
||||
snapshot = mcp_mod.runtime_status(state)
|
||||
assert snapshot == {"xmind": "failed"}
|
||||
assert "saved-oauth-secret" not in str(snapshot)
|
||||
assert "app.xmind.com" not in str(snapshot)
|
||||
|
||||
|
||||
def test_wrapper_preserves_non_nullable_unions() -> None:
|
||||
tool_def = SimpleNamespace(
|
||||
name="demo",
|
||||
|
||||
@@ -172,10 +172,72 @@ async def test_oauth_preset_is_one_click_configured_after_token_storage(
|
||||
assert row["configured"] is True
|
||||
assert row["status"] == "configured"
|
||||
|
||||
failed = mcp_presets_payload(runtime_status={"xmind": "failed"})
|
||||
row = next(item for item in failed["presets"] if item["name"] == "xmind")
|
||||
assert row["configured"] is True
|
||||
assert row["status"] == "configured"
|
||||
assert row["runtime_status"] == "failed"
|
||||
assert "secret" not in str(row)
|
||||
|
||||
healthy = mcp_presets_payload(runtime_status={"xmind": "connected"})
|
||||
row = next(item for item in healthy["presets"] if item["name"] == "xmind")
|
||||
assert row["runtime_status"] == "connected"
|
||||
|
||||
mcp_presets_action("remove", {"name": ["xmind"]})
|
||||
assert await MCPOAuthStorage("xmind", cfg.url).get_tokens() is None
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_settings_list_projects_runtime_snapshot_and_reconnects_custom_server(
|
||||
tmp_path,
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
_use_config(tmp_path, monkeypatch)
|
||||
custom_mcp_action(
|
||||
"custom",
|
||||
{
|
||||
"name": ["team-docs"],
|
||||
"transport": ["streamableHttp"],
|
||||
"url": ["https://mcp.example.com/mcp"],
|
||||
},
|
||||
)
|
||||
statuses = {"team-docs": "failed"}
|
||||
reload_calls = 0
|
||||
|
||||
async def reload_mcp() -> dict[str, object]:
|
||||
nonlocal reload_calls
|
||||
reload_calls += 1
|
||||
statuses["team-docs"] = "connected"
|
||||
return {
|
||||
"ok": True,
|
||||
"connected": ["team-docs"],
|
||||
"failed": [],
|
||||
"requires_restart": False,
|
||||
}
|
||||
|
||||
listed = await mcp_presets_settings_action(
|
||||
None,
|
||||
{},
|
||||
reload_mcp=reload_mcp,
|
||||
mcp_runtime_status=lambda: statuses,
|
||||
)
|
||||
row = next(item for item in listed["presets"] if item["name"] == "team-docs")
|
||||
assert row["configured"] is True
|
||||
assert row["runtime_status"] == "failed"
|
||||
assert reload_calls == 0
|
||||
|
||||
reconnected = await mcp_presets_settings_action(
|
||||
"reconnect",
|
||||
{"name": ["team-docs"]},
|
||||
reload_mcp=reload_mcp,
|
||||
mcp_runtime_status=lambda: statuses,
|
||||
)
|
||||
row = next(item for item in reconnected["presets"] if item["name"] == "team-docs")
|
||||
assert row["runtime_status"] == "connected"
|
||||
assert reconnected["requires_restart"] is False
|
||||
assert reload_calls == 1
|
||||
|
||||
|
||||
def test_enable_browserbase_writes_scrubbed_config_payload(
|
||||
tmp_path,
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
|
||||
@@ -1,6 +1,8 @@
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
from collections.abc import Callable, Mapping
|
||||
from pathlib import Path
|
||||
from types import SimpleNamespace
|
||||
from unittest.mock import ANY, AsyncMock, MagicMock
|
||||
from urllib.parse import parse_qs, urlsplit
|
||||
@@ -10,13 +12,19 @@ from websockets.datastructures import Headers
|
||||
|
||||
from nanobot.config.loader import get_config_path
|
||||
from nanobot.webui.http_utils import http_json_response
|
||||
from nanobot.webui.mcp_presets_api import custom_mcp_action
|
||||
from nanobot.webui.settings_routes import WebUISettingsRouter
|
||||
from nanobot.webui.settings_services import WebUISettingsServices
|
||||
|
||||
|
||||
def _router(*, authorized: bool = True) -> WebUISettingsRouter:
|
||||
def _router(
|
||||
*,
|
||||
authorized: bool = True,
|
||||
config_path: Path | None = None,
|
||||
mcp_runtime_status: Callable[[], Mapping[str, str]] | None = None,
|
||||
) -> WebUISettingsRouter:
|
||||
return WebUISettingsRouter(
|
||||
settings=WebUISettingsServices.create(get_config_path()),
|
||||
settings=WebUISettingsServices.create(config_path or get_config_path()),
|
||||
bus=SimpleNamespace(),
|
||||
logger=SimpleNamespace(exception=lambda *_args: None),
|
||||
check_api_token=lambda _request: authorized,
|
||||
@@ -28,6 +36,7 @@ def _router(*, authorized: bool = True) -> WebUISettingsRouter:
|
||||
),
|
||||
runtime_surface="browser",
|
||||
runtime_capabilities={},
|
||||
mcp_runtime_status=mcp_runtime_status,
|
||||
mcp_oauth_redirect_uri=lambda _request: "https://gateway.example/auth/mcp/callback",
|
||||
)
|
||||
|
||||
@@ -40,6 +49,46 @@ def _mutation_request(path: str, payload: dict[str, object]) -> SimpleNamespace:
|
||||
return request
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_mcp_list_serializes_local_runtime_failure_snapshot(tmp_path) -> None:
|
||||
config_path = tmp_path / "config.json"
|
||||
custom_mcp_action(
|
||||
"custom",
|
||||
{
|
||||
"name": ["team-docs"],
|
||||
"transport": ["streamableHttp"],
|
||||
"url": ["https://mcp.example.com/mcp"],
|
||||
},
|
||||
config_path=config_path,
|
||||
)
|
||||
snapshot_calls = 0
|
||||
|
||||
def runtime_snapshot() -> Mapping[str, str]:
|
||||
nonlocal snapshot_calls
|
||||
snapshot_calls += 1
|
||||
return {"team-docs": "failed"}
|
||||
|
||||
router = _router(
|
||||
config_path=config_path,
|
||||
mcp_runtime_status=runtime_snapshot,
|
||||
)
|
||||
request = SimpleNamespace(
|
||||
path="/api/settings/mcp-presets",
|
||||
headers=Headers(),
|
||||
)
|
||||
|
||||
response = await router.dispatch(None, request, "/api/settings/mcp-presets")
|
||||
|
||||
assert response is not None
|
||||
assert response.status_code == 200
|
||||
payload = json.loads(response.body)
|
||||
row = next(item for item in payload["presets"] if item["name"] == "team-docs")
|
||||
assert row["status"] == "configured"
|
||||
assert row["runtime_status"] == "failed"
|
||||
assert b'"runtime_status": "failed"' in response.body
|
||||
assert snapshot_calls == 1
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_mcp_oauth_start_uses_gateway_callback_and_requires_api_auth(monkeypatch) -> None:
|
||||
config = SimpleNamespace(
|
||||
|
||||
Reference in New Issue
Block a user