squash some bugs
This commit is contained in:
@@ -124,6 +124,17 @@ Each upstream MCP connection is a source, for example `everything.default` or
|
|||||||
Connection sources are the only sources that represent upstream MCP server
|
Connection sources are the only sources that represent upstream MCP server
|
||||||
snapshots. System sources are local broker capabilities.
|
snapshots. System sources are local broker capabilities.
|
||||||
|
|
||||||
|
A configured connection should still appear as a connection source even when no
|
||||||
|
catalog snapshot has been loaded yet. In that state the source can have zero
|
||||||
|
owned capabilities and a description such as `No catalog loaded for ...`; the
|
||||||
|
absence of discovered capabilities must not make the configured connection
|
||||||
|
disappear from source inventory.
|
||||||
|
|
||||||
|
Connection discovery treats `tools/list` as the required workflow-facing family.
|
||||||
|
Optional MCP families such as `resources/list` and `prompts/list` may be absent;
|
||||||
|
servers that return MCP `Method not found` for those methods still refresh as
|
||||||
|
tool-only sources instead of failing the whole catalog refresh.
|
||||||
|
|
||||||
## Projections
|
## Projections
|
||||||
|
|
||||||
### Planner Catalog
|
### Planner Catalog
|
||||||
|
|||||||
@@ -1,17 +1,22 @@
|
|||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
from collections.abc import Callable
|
from collections.abc import Awaitable, Callable
|
||||||
from dataclasses import dataclass, field
|
from dataclasses import dataclass, field
|
||||||
from typing import Any
|
from typing import Any, TypeVar
|
||||||
|
|
||||||
|
from mcp import McpError
|
||||||
|
from mcp.types import METHOD_NOT_FOUND
|
||||||
|
|
||||||
from wf_authoring import NodeSpec
|
from wf_authoring import NodeSpec
|
||||||
|
|
||||||
from ..capabilities import DiscoveredPrompt, DiscoveredResource, DiscoveredTool
|
from ..capabilities import DiscoveredPrompt, DiscoveredResource, DiscoveredTool
|
||||||
from ..models import AuthRecord, ConnectionConfig
|
from ..models import AuthRecord, ConnectionConfig
|
||||||
from ..sdk import BackendAdapter
|
from ..sdk import BackendAdapter
|
||||||
|
from ..shared import root_exception
|
||||||
from ..workflow import wrap_discovered_tool
|
from ..workflow import wrap_discovered_tool
|
||||||
from .events import McpEvent
|
from .events import McpEvent
|
||||||
|
|
||||||
|
_CapabilityT = TypeVar("_CapabilityT")
|
||||||
|
|
||||||
@dataclass(slots=True)
|
@dataclass(slots=True)
|
||||||
class DiscoveredConnectionCapabilities:
|
class DiscoveredConnectionCapabilities:
|
||||||
@@ -28,8 +33,12 @@ async def discover_connection_capabilities(
|
|||||||
adapter: BackendAdapter,
|
adapter: BackendAdapter,
|
||||||
) -> DiscoveredConnectionCapabilities:
|
) -> DiscoveredConnectionCapabilities:
|
||||||
tools = await adapter.list_tools(connection, auth)
|
tools = await adapter.list_tools(connection, auth)
|
||||||
resources = await adapter.list_resources(connection, auth)
|
resources = await _list_optional_capabilities(
|
||||||
prompts = await adapter.list_prompts(connection, auth)
|
lambda: adapter.list_resources(connection, auth)
|
||||||
|
)
|
||||||
|
prompts = await _list_optional_capabilities(
|
||||||
|
lambda: adapter.list_prompts(connection, auth)
|
||||||
|
)
|
||||||
metadata = await adapter.get_connection_metadata(connection, auth)
|
metadata = await adapter.get_connection_metadata(connection, auth)
|
||||||
return DiscoveredConnectionCapabilities(
|
return DiscoveredConnectionCapabilities(
|
||||||
tools=tools,
|
tools=tools,
|
||||||
@@ -39,6 +48,25 @@ async def discover_connection_capabilities(
|
|||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
async def _list_optional_capabilities(
|
||||||
|
load: Callable[[], Awaitable[list[_CapabilityT]]],
|
||||||
|
) -> list[_CapabilityT]:
|
||||||
|
"""Treat unsupported optional MCP capability families as empty lists.
|
||||||
|
|
||||||
|
Some SDK transports raise ``METHOD_NOT_FOUND`` from inside an
|
||||||
|
``ExceptionGroup`` because the request ran through a task group. Resources
|
||||||
|
and prompts are optional families, so only that exact root error means "not
|
||||||
|
supported"; every other failure still needs to surface.
|
||||||
|
"""
|
||||||
|
try:
|
||||||
|
return await load()
|
||||||
|
except Exception as exc:
|
||||||
|
root = root_exception(exc)
|
||||||
|
if isinstance(root, McpError) and root.error.code == METHOD_NOT_FOUND:
|
||||||
|
return []
|
||||||
|
raise
|
||||||
|
|
||||||
|
|
||||||
def specs_from_discovered_tools(
|
def specs_from_discovered_tools(
|
||||||
*,
|
*,
|
||||||
connection: ConnectionConfig,
|
connection: ConnectionConfig,
|
||||||
|
|||||||
@@ -644,18 +644,20 @@ class WfMcpService:
|
|||||||
self,
|
self,
|
||||||
connection: ConnectionConfig,
|
connection: ConnectionConfig,
|
||||||
) -> None:
|
) -> None:
|
||||||
"""Restore planner-visible connection specs from a stored catalog snapshot."""
|
"""Register one connection source, hydrating specs from snapshot if present."""
|
||||||
if connection.id in self.capability_sources:
|
if connection.id in self.capability_sources:
|
||||||
return
|
return
|
||||||
|
|
||||||
snapshot = self.store.load_catalog(connection.id)
|
snapshot = self.store.load_catalog(connection.id)
|
||||||
if snapshot is None or not snapshot.nodes:
|
|
||||||
return
|
|
||||||
|
|
||||||
specs = {
|
specs = {
|
||||||
entry.qualified_name: self._spec_from_snapshot_entry(entry)
|
entry.qualified_name: self._spec_from_snapshot_entry(entry)
|
||||||
for entry in snapshot.nodes
|
for entry in (() if snapshot is None else snapshot.nodes)
|
||||||
}
|
}
|
||||||
|
description = (
|
||||||
|
f"Specs restored from catalog for {connection.id}."
|
||||||
|
if specs
|
||||||
|
else f"No catalog loaded for {connection.id}."
|
||||||
|
)
|
||||||
self.register_capability_source(
|
self.register_capability_source(
|
||||||
CapabilitySource(
|
CapabilitySource(
|
||||||
id=connection.id,
|
id=connection.id,
|
||||||
@@ -668,7 +670,7 @@ class WfMcpService:
|
|||||||
admin_dashboard=True,
|
admin_dashboard=True,
|
||||||
),
|
),
|
||||||
permissions=SourcePermissions(calls_upstream=True),
|
permissions=SourcePermissions(calls_upstream=True),
|
||||||
description=f"Specs restored from catalog for {connection.id}.",
|
description=description,
|
||||||
)
|
)
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|||||||
@@ -3,7 +3,10 @@ from __future__ import annotations
|
|||||||
import asyncio
|
import asyncio
|
||||||
|
|
||||||
import pytest
|
import pytest
|
||||||
|
from mcp import McpError
|
||||||
|
from mcp.types import ErrorData
|
||||||
|
|
||||||
|
from wf_mcp.capabilities import DiscoveredTool
|
||||||
from wf_mcp.broker import WfMcpService
|
from wf_mcp.broker import WfMcpService
|
||||||
from wf_mcp.models import ConnectionConfig
|
from wf_mcp.models import ConnectionConfig
|
||||||
from wf_mcp.sdk import McpSdkAdapter
|
from wf_mcp.sdk import McpSdkAdapter
|
||||||
@@ -17,6 +20,51 @@ from .test_support import (
|
|||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
class _ToolsOnlyAdapter:
|
||||||
|
async def list_tools(self, connection, auth):
|
||||||
|
return [
|
||||||
|
DiscoveredTool(
|
||||||
|
name="echo_tool",
|
||||||
|
title="Echo",
|
||||||
|
description="Echo text.",
|
||||||
|
input_schema={"type": "object", "properties": {}},
|
||||||
|
output_schema={"type": "object", "properties": {}},
|
||||||
|
)
|
||||||
|
]
|
||||||
|
|
||||||
|
async def list_resources(self, connection, auth):
|
||||||
|
raise McpError(ErrorData(code=-32601, message="Method not found"))
|
||||||
|
|
||||||
|
async def list_prompts(self, connection, auth):
|
||||||
|
raise McpError(ErrorData(code=-32601, message="Method not found"))
|
||||||
|
|
||||||
|
async def get_connection_metadata(self, connection, auth):
|
||||||
|
return {"server": connection.server}
|
||||||
|
|
||||||
|
async def read_resource(self, connection, auth, uri):
|
||||||
|
raise NotImplementedError
|
||||||
|
|
||||||
|
async def get_prompt(self, connection, auth, prompt_name, arguments=None):
|
||||||
|
raise NotImplementedError
|
||||||
|
|
||||||
|
async def invoke_method(self, connection, auth, method, params=None):
|
||||||
|
raise NotImplementedError
|
||||||
|
|
||||||
|
async def send_notification(self, connection, auth, method, params=None):
|
||||||
|
raise NotImplementedError
|
||||||
|
|
||||||
|
async def call_tool(self, connection, auth, tool_name, payload):
|
||||||
|
raise NotImplementedError
|
||||||
|
|
||||||
|
|
||||||
|
class _WrappedToolsOnlyAdapter(_ToolsOnlyAdapter):
|
||||||
|
async def list_resources(self, connection, auth):
|
||||||
|
raise ExceptionGroup(
|
||||||
|
"unhandled errors in a TaskGroup",
|
||||||
|
[McpError(ErrorData(code=-32601, message="Method not found"))],
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
def test_mcp_sdk_adapter_lists_and_calls_stdio_tool() -> None:
|
def test_mcp_sdk_adapter_lists_and_calls_stdio_tool() -> None:
|
||||||
service = WfMcpService(store=FileStore(local_temp_root() / "sdk_adapter_store"))
|
service = WfMcpService(store=FileStore(local_temp_root() / "sdk_adapter_store"))
|
||||||
service.register_connection(
|
service.register_connection(
|
||||||
@@ -125,3 +173,45 @@ def test_mcp_sdk_adapter_can_probe_everything_server() -> None:
|
|||||||
)
|
)
|
||||||
assert "resources" in payload
|
assert "resources" in payload
|
||||||
assert "prompts" in payload
|
assert "prompts" in payload
|
||||||
|
|
||||||
|
|
||||||
|
def test_refresh_catalog_keeps_tools_when_optional_lists_are_unsupported() -> None:
|
||||||
|
service = WfMcpService(
|
||||||
|
store=FileStore(local_temp_root() / "tools_only_server_store")
|
||||||
|
)
|
||||||
|
service.register_connection(
|
||||||
|
ConnectionConfig(
|
||||||
|
id="tools_only.personal",
|
||||||
|
server="tools_only",
|
||||||
|
account="personal",
|
||||||
|
)
|
||||||
|
)
|
||||||
|
service.register_adapter("tools_only", _ToolsOnlyAdapter())
|
||||||
|
|
||||||
|
asyncio.run(service.refresh_connection_catalog("tools_only.personal"))
|
||||||
|
|
||||||
|
payload = service.get_catalog().as_payload()
|
||||||
|
assert payload["nodes"][0]["qualified_name"] == "tools_only.personal.echo_tool"
|
||||||
|
assert payload["resources"] == []
|
||||||
|
assert payload["prompts"] == []
|
||||||
|
|
||||||
|
|
||||||
|
def test_refresh_catalog_unwraps_taskgroup_method_not_found() -> None:
|
||||||
|
service = WfMcpService(
|
||||||
|
store=FileStore(local_temp_root() / "wrapped_tools_only_server_store")
|
||||||
|
)
|
||||||
|
service.register_connection(
|
||||||
|
ConnectionConfig(
|
||||||
|
id="wrapped_tools_only.personal",
|
||||||
|
server="wrapped_tools_only",
|
||||||
|
account="personal",
|
||||||
|
)
|
||||||
|
)
|
||||||
|
service.register_adapter("wrapped_tools_only", _WrappedToolsOnlyAdapter())
|
||||||
|
|
||||||
|
asyncio.run(service.refresh_connection_catalog("wrapped_tools_only.personal"))
|
||||||
|
|
||||||
|
payload = service.get_catalog().as_payload()
|
||||||
|
assert payload["nodes"][0]["qualified_name"] == (
|
||||||
|
"wrapped_tools_only.personal.echo_tool"
|
||||||
|
)
|
||||||
|
|||||||
@@ -103,6 +103,19 @@ def test_service_installs_builtin_stdlib_specs_by_default() -> None:
|
|||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def test_service_registers_empty_source_for_connection_without_catalog() -> None:
|
||||||
|
service = WfMcpService(store=FileStore(local_temp_root() / "empty_source"))
|
||||||
|
|
||||||
|
service.register_connection(
|
||||||
|
ConnectionConfig(id="demo.personal", server="demo", account="personal")
|
||||||
|
)
|
||||||
|
|
||||||
|
source = service.capability_sources["demo.personal"]
|
||||||
|
assert source.enabled is True
|
||||||
|
assert source.capabilities.node_specs == {}
|
||||||
|
assert source.description == "No catalog loaded for demo.personal."
|
||||||
|
|
||||||
|
|
||||||
def test_service_lists_all_capability_sources_with_owned_capability_names() -> None:
|
def test_service_lists_all_capability_sources_with_owned_capability_names() -> None:
|
||||||
service = WfMcpService(store=FileStore(local_temp_root() / "source_inventory"))
|
service = WfMcpService(store=FileStore(local_temp_root() / "source_inventory"))
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user