From fe27ac9ac7f8de56285255f8369c700949785828 Mon Sep 17 00:00:00 2001 From: Million <15158090088@163.com> Date: Sat, 15 Aug 2026 15:00:23 +0800 Subject: [PATCH 01/11] feat(mcp): add tool_prefix to McpServerCap and ResourceAccess delegation to SkillManagerCap McpServerCap gains a model-visible tool namespace (derived from display_name) so prefixed MCP tool names are predictable. list_resources() now prefers the server-provided title over the raw name. SkillManagerCap implements ResourceAccess, delegating to its skill-level MCP children, so top-level resources are discoverable through the ExtensionRegistry (RFC-0058). Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus --- .../capabilities/mcp_server_cap.py | 20 ++++- .../capabilities/skill_manager_cap.py | 90 +++++++++++++++++++ 2 files changed, 107 insertions(+), 3 deletions(-) diff --git a/src/wolfharness/capabilities/mcp_server_cap.py b/src/wolfharness/capabilities/mcp_server_cap.py index b0e01cd2a..0df015a79 100644 --- a/src/wolfharness/capabilities/mcp_server_cap.py +++ b/src/wolfharness/capabilities/mcp_server_cap.py @@ -91,6 +91,7 @@ def __init__( *, name: str | None = None, client: MCPClient | None = None, + tool_prefix: str | None = None, ) -> None: """Initialize the capability. @@ -101,10 +102,14 @@ def __init__( name: Optional name override. Defaults to ``config.client_id``. client: Optional pre-created ``MCPClient``. When provided, bypasses the session pool and uses this client directly. + tool_prefix: Optional model-visible tool namespace derived from + the server's ``display_name``. When ``None``, falls back to + ``config.display_name``. """ self._config = config self._session_pool = session_pool self._name = name or config.client_id + self._tool_prefix = tool_prefix or config.display_name self._client: MCPClient | None = client self._change_queues: set[asyncio.Queue[ChangeEvent]] = set() @@ -115,6 +120,11 @@ def name(self) -> str: """Return the capability name.""" return self._name + @property + def tool_prefix(self) -> str: + """Return the model-visible tool namespace for this server.""" + return self._tool_prefix + @property def config(self) -> MCPServerConfig: """Return the MCP server config.""" @@ -242,7 +252,7 @@ async def _build_toolset( tools = await client.list_tools() if not tools: return None - from pydantic_ai.toolsets import CombinedToolset, FunctionToolset + from pydantic_ai.toolsets import CombinedToolset, FunctionToolset, PrefixedToolset from wolfharness.tools.tool_wrapping import wrap_tool_for_pydantic_ai @@ -253,7 +263,11 @@ async def _build_toolset( ] if not toolsets: return None - return CombinedToolset(toolsets) + combined = CombinedToolset(toolsets) + prefix = self._tool_prefix + if not prefix: + return combined + return PrefixedToolset(wrapped=combined, prefix=prefix) return _build_toolset @@ -351,7 +365,7 @@ async def list_resources(self) -> Sequence[ResourceEntry]: return [ ResourceEntry( uri=str(r.uri), - name=r.name, + name=r.title or r.name, description=r.description or "", mime_type=r.mimeType if r.mimeType else "", ) diff --git a/src/wolfharness/capabilities/skill_manager_cap.py b/src/wolfharness/capabilities/skill_manager_cap.py index 269e48660..e817232f2 100644 --- a/src/wolfharness/capabilities/skill_manager_cap.py +++ b/src/wolfharness/capabilities/skill_manager_cap.py @@ -41,11 +41,15 @@ from wolfharness.capabilities.combined_toolset import CombinedToolsetCapability from wolfharness.capabilities.resource_protocols import ( + BlobResourceContent, ChangeObservable, CommandEntry, CommandResource, + ResourceAccess, + ResourceEntry, SkillEntry, SkillResource, + TextResourceContent, ) from wolfharness.log import get_logger from wolfharness.skills.skill import Skill @@ -203,6 +207,7 @@ class SkillManagerCap( CombinedToolsetCapability[AgentDepsT], SkillResource, CommandResource, + ResourceAccess, ChangeObservable, ): """Unified skill management capability. @@ -697,6 +702,91 @@ async def _build_dynamic_skill_content(self, ctx: RunContext[AgentDepsT]) -> str return "\n\n".join(parts) if parts else None + # ---- ResourceAccess delegation (RFC-0058) ---- + + async def list_resources(self) -> Sequence[ResourceEntry]: + """List resources from all per-skill MCP children. + + Delegates to every per-skill ``McpServerCap`` child implementing + :class:`ResourceAccess` and aggregates their ``ResourceEntry`` + instances. A failing child is skipped so one broken MCP server + does not prevent resource listing from the others. + + Returns: + Aggregated sequence of ``ResourceEntry`` descriptors. + """ + entries: list[ResourceEntry] = [] + for caps in self._skill_mcp_children.values(): + for cap in caps: + if isinstance(cap, ResourceAccess): + try: + entries.extend(await cap.list_resources()) + except Exception: + logger.warning( + "Failed to list resources from per-skill MCP child", + server=cap.name, + exc_info=True, + ) + continue + return entries + + async def read_resource( + self, uri: str + ) -> list[TextResourceContent | BlobResourceContent] | None: + """Read an MCP resource by URI from per-skill MCP children. + + Tries each per-skill ``McpServerCap`` child implementing + :class:`ResourceAccess` until one returns a non-``None`` result. + A failing child is skipped so a broken server does not abort the read. + + Args: + uri: Resource URI to read. + + Returns: + List of resource content instances, or ``None`` if no child + has the resource. + """ + for caps in self._skill_mcp_children.values(): + for cap in caps: + if isinstance(cap, ResourceAccess): + try: + result = await cap.read_resource(uri) + except Exception: + logger.warning( + "Failed to read resource %r from per-skill MCP child", + uri, + exc_info=True, + ) + continue + if result is not None: + return result + return None + + async def resource_exists(self, uri: str) -> bool: + """Check if a resource URI exists in any per-skill MCP child. + + Args: + uri: Resource URI to check. + + Returns: + ``True`` if any per-skill ``McpServerCap`` child reports the + resource, ``False`` otherwise. + """ + for caps in self._skill_mcp_children.values(): + for cap in caps: + if isinstance(cap, ResourceAccess): + try: + if await cap.resource_exists(uri): + return True + except Exception: + logger.warning( + "Failed to check resource %r existence in per-skill MCP child", + uri, + exc_info=True, + ) + continue + return False + @property def has_wrap_node_run(self) -> bool: """Return False — no node run wrapping needed.""" From ab475ba730661db80d812808468697d1035264a6 Mon Sep 17 00:00:00 2001 From: Million <15158090088@163.com> Date: Sat, 15 Aug 2026 15:03:06 +0800 Subject: [PATCH 02/11] feat(mcp): register top-level McpServerCap at POOL scope and inject tools directly AgentPool._rebuild_skill_capabilities() now registers each top-level McpServerCap independently in the ExtensionRegistry at POOL scope, making them discoverable via get_resource_access() for @ mention. get_agentlet() injects these providers directly into tool_capabilities and passes exclude_global=True to get_capabilities() so session/skill configs are not double-processed. MCPManager de-duplicates display_name tool prefixes and supports the exclude_global flag. /experimental/resource lists resources by URI so opencode @ mention surfaces them consistently (RFC-0058). Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus --- src/wolfharness/agents/native_agent/agent.py | 17 +++++++-- src/wolfharness/delegation/pool.py | 7 ++++ src/wolfharness/mcp_server/manager.py | 36 +++++++++++++++---- .../opencode_server/routes/agent_routes.py | 2 +- 4 files changed, 52 insertions(+), 10 deletions(-) diff --git a/src/wolfharness/agents/native_agent/agent.py b/src/wolfharness/agents/native_agent/agent.py index edbcb8c34..cb19a4b4f 100644 --- a/src/wolfharness/agents/native_agent/agent.py +++ b/src/wolfharness/agents/native_agent/agent.py @@ -1119,14 +1119,25 @@ async def get_agentlet[AgentOutputType]( # noqa: PLR0915 ) tool_capabilities.append(create_approval_bridge_capability(self, input_provider)) - # 4. MCP servers + # 4. MCP servers. + # Top-level (non-ACP) providers are injected directly — their tools + # come from McpServerCap.get_toolset(). ACP providers continue via + # the aggregating provider (Path C). Session-scoped configs (session + # + skill) still use get_capabilities() with global configs excluded. + from wolfharness.capabilities.mcp_server_cap import McpServerCap + + pool = self._agent_pool + if pool is not None: + tool_capabilities.extend( + provider for provider in pool.mcp.providers if isinstance(provider, McpServerCap) + ) mcp_capabilities = await self.mcp.get_capabilities( - session_id=run_ctx.session_id if run_ctx else None + session_id=run_ctx.session_id if run_ctx else None, + exclude_global=True, ) tool_capabilities.extend(mcp_capabilities) # 5. Skill capabilities — from pool-scoped instances created during __aenter__. # Each SkillManagerCap provides tools and MCP servers. - pool = self._agent_pool if pool is not None: pool_capabilities = pool.skill_capabilities if pool_capabilities: diff --git a/src/wolfharness/delegation/pool.py b/src/wolfharness/delegation/pool.py index 07bb4ccbf..2de9e73ed 100644 --- a/src/wolfharness/delegation/pool.py +++ b/src/wolfharness/delegation/pool.py @@ -685,6 +685,13 @@ async def _rebuild_skill_capabilities(self) -> None: # Register the new SkillManagerCap with ExtensionRegistry at POOL scope. self._extension_registry.register(cap, pool_scope) + # Register each top-level McpServerCap independently at POOL scope so + # they are directly discoverable via get_resource_access() for ``@`` + # mention and ResourceCapability (RFC-0058). They no longer live only + # inside SkillManagerCap.children. + for provider in self.mcp.providers: + self._extension_registry.register(provider, pool_scope) + logger.debug( "Rebuilt skill capabilities", count=len(self._skill_capabilities), diff --git a/src/wolfharness/mcp_server/manager.py b/src/wolfharness/mcp_server/manager.py index dae61ed34..a325c2994 100644 --- a/src/wolfharness/mcp_server/manager.py +++ b/src/wolfharness/mcp_server/manager.py @@ -236,6 +236,10 @@ def __init__( # Used by get_server_status() to report status="error" for failed # servers instead of silently dropping them. self._setup_errors: dict[str, str] = {} + # Model-visible tool prefixes in use, for display_name de-duplication + # (RFC-0058): two servers sharing a display_name get prefixes + # ``name``, ``name_2``, ... + self._used_tool_prefixes: set[str] = set() def add_server_config(self, cfg: MCPServerConfig | str) -> None: """Add a new MCP server to the manager.""" @@ -409,6 +413,16 @@ async def setup_server( ) return None + # De-duplicate the model-visible tool prefix across servers sharing + # a display_name so prefixed tool names never collide (RFC-0058). + base_prefix = config.display_name + candidate = base_prefix + n = 2 + while candidate in self._used_tool_prefixes: + candidate = f"{base_prefix}_{n}" + n += 1 + self._used_tool_prefixes.add(candidate) + from wolfharness.mcp_server.client import MCPClient try: @@ -421,6 +435,7 @@ async def setup_server( config=config, name=f"{self.name}_{config.display_name}", client=client, + tool_prefix=candidate, ) provider = await self.exit_stack.enter_async_context(provider) self.providers.append(provider) @@ -649,6 +664,8 @@ def get_aggregating_provider(self) -> CombinedToolsetCapability: async def get_capabilities( # noqa: PLR0915 self, session_id: str | None = None, + *, + exclude_global: bool = False, ) -> list[MCP]: """Return pydantic-ai MCP capabilities for all configured servers. @@ -683,6 +700,10 @@ async def get_capabilities( # noqa: PLR0915 session_id: Optional session identifier for per-session MCP config isolation. When None, only global configs from ``self.servers`` are processed. + exclude_global: When True, skip pool + agent global configs. + Used when top-level McpServerCap instances are injected + directly into the agent (RFC-0058); only session-scoped + configs (session + skill) are processed. Returns: A list of ``pydantic_ai.capabilities.MCP`` instances, one per @@ -810,7 +831,8 @@ async def _process_session_configs( ctx = self._session_contexts.get(session_id) if session_id is not None else None if ctx is not None and ctx.snapshot is not None: - await _process_global_configs(ctx.snapshot, self._toolset_cache) + if not exclude_global: + await _process_global_configs(ctx.snapshot, self._toolset_cache) if ctx.connection_pool is not None: await _process_session_configs( ctx.snapshot, @@ -824,11 +846,13 @@ async def _process_session_configs( "falling back to global-only MCP capabilities.", session_id, ) - for server in self.servers: - if not server.enabled or isinstance(server, AcpMCPServerConfig): - continue - transport = await self._global_pool.get_transport(server) - capabilities.append(await _make_capability(server, transport, self._toolset_cache)) + if not exclude_global: + for server in self.servers: + if not server.enabled or isinstance(server, AcpMCPServerConfig): + continue + transport = await self._global_pool.get_transport(server) + caps = await _make_capability(server, transport, self._toolset_cache) + capabilities.append(caps) return capabilities diff --git a/src/wolfharness_server/opencode_server/routes/agent_routes.py b/src/wolfharness_server/opencode_server/routes/agent_routes.py index 5f80afe19..bf7cd316e 100644 --- a/src/wolfharness_server/opencode_server/routes/agent_routes.py +++ b/src/wolfharness_server/opencode_server/routes/agent_routes.py @@ -538,7 +538,7 @@ async def list_mcp_resources(state: StateDep) -> dict[str, McpResource]: client_name = client.replace("/", "_") resource_name = resource.name.replace("/", "_") result[f"{client_name}:{resource_name}"] = McpResource( - name=resource.name, + name=resource.uri, uri=resource.uri, description=resource.description, mime_type=resource.mime_type, From ccb350204b0512f5df570b482a49e56ecba90b5e Mon Sep 17 00:00:00 2001 From: Million <15158090088@163.com> Date: Sat, 15 Aug 2026 15:07:32 +0800 Subject: [PATCH 03/11] test(mcp): cover top-level McpServerCap POOL registration and ResourceAccess delegation Adds test_skill_manager_cap_resource_access.py (9 delegation cases for the new ResourceAccess implementation), McpServerCap resource title preference and prefixed toolset cases, top-level MCP pool registration + tool_prefix de-duplication in test_pool_skills.py, and updates the factory scope assertion to filter ResourceAccess caps correctly (RFC-0058). Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus --- tests/capabilities/test_mcp_server_cap.py | 18 ++ .../test_skill_manager_cap_resource_access.py | 157 ++++++++++++++++++ tests/delegation/test_pool_skills.py | 109 +++++++++++- tests/host/test_factory.py | 9 +- 4 files changed, 289 insertions(+), 4 deletions(-) create mode 100644 tests/capabilities/test_skill_manager_cap_resource_access.py diff --git a/tests/capabilities/test_mcp_server_cap.py b/tests/capabilities/test_mcp_server_cap.py index 347c781d5..be68b7295 100644 --- a/tests/capabilities/test_mcp_server_cap.py +++ b/tests/capabilities/test_mcp_server_cap.py @@ -167,6 +167,7 @@ def _make_resource( res = MagicMock() res.uri = uri res.name = name + res.title = None res.description = description res.mimeType = mime_type return res @@ -352,6 +353,23 @@ async def test_list_resources_delegation() -> None: assert result[0].mime_type == "text/plain" +@pytest.mark.anyio +async def test_list_resources_prefers_title() -> None: + """Title is used as the ResourceEntry name when present.""" + titled = MagicMock() + titled.uri = "file:///titled" + titled.name = "get_titled" + titled.title = "Human Readable Title" + titled.description = "" + titled.mimeType = "text/plain" + client = FakeMCPClient(_resources=[titled]) + cap = McpServerCap(config=_make_config(), session_pool=FakeSessionPool(client)) + + result = await cap.list_resources() + + assert result[0].name == "Human Readable Title" + + @pytest.mark.anyio async def test_read_resource_existing() -> None: """read_resource() returns TextResourceContent list for existing resource.""" diff --git a/tests/capabilities/test_skill_manager_cap_resource_access.py b/tests/capabilities/test_skill_manager_cap_resource_access.py new file mode 100644 index 000000000..bd950e252 --- /dev/null +++ b/tests/capabilities/test_skill_manager_cap_resource_access.py @@ -0,0 +1,157 @@ +"""Unit tests for ``SkillManagerCap`` ResourceAccess delegation (RFC-0058). + +Verifies that ``SkillManagerCap`` (which now inherits ``ResourceAccess``) +aggregates resource listing/reading/existence from its per-skill MCP +children, without needing top-level MCP providers as ``children``. +""" + +from __future__ import annotations + +from typing import Any + +import pytest + +from wolfharness.capabilities.resource_protocols import ResourceAccess, ResourceEntry +from wolfharness.capabilities.skill_manager_cap import SkillManagerCap +from wolfharness.skills.skill import Skill +from wolfharness_config.skills import SkillMcpServerConfig + + +pytestmark = pytest.mark.unit + + +class FakeResourceChild: + """Stand-in for a per-skill ``McpServerCap`` implementing ``ResourceAccess``.""" + + def __init__( + self, + name: str, + entries: list[ResourceEntry], + read_results: dict[str, Any], + ) -> None: + self.name = name + self._entries = entries + self._read_results = read_results + self.read_calls: list[str] = [] + self.exists_calls: list[str] = [] + + async def list_resources(self) -> list[ResourceEntry]: + return list(self._entries) + + async def read_resource(self, uri: str) -> Any: + self.read_calls.append(uri) + return self._read_results.get(uri) + + async def resource_exists(self, uri: str) -> bool: + self.exists_calls.append(uri) + return uri in self._read_results + + +def _entry(uri: str, name: str = "") -> ResourceEntry: + return ResourceEntry(uri=uri, name=name or uri, description="", mime_type="text/plain") + + +def _skill_with_mcp(name: str = "alpha", server_name: str = "remote") -> Skill: + return Skill( + name=name, + description=name, + skill_path=f"/tmp/{name}", + mcp_servers={ + server_name: SkillMcpServerConfig(url="http://localhost:9999/mcp"), + }, + ) + + +def _cap_with_children(children: list[FakeResourceChild]) -> SkillManagerCap: + cap = SkillManagerCap(local_skills={"alpha": _skill_with_mcp()}, name="pool-skills") + cap._skill_mcp_children = {"alpha": children} # type: ignore[assignment] + return cap + + +async def test_skill_manager_cap_is_resource_access() -> None: + cap = SkillManagerCap(local_skills={}, name="pool-skills") + assert isinstance(cap, ResourceAccess) + + +async def test_list_resources_aggregates_child_entries() -> None: + child_a = FakeResourceChild("a", [_entry("mcp://a/one"), _entry("mcp://a/two")], {}) + child_b = FakeResourceChild("b", [_entry("mcp://b/three")], {}) + cap = _cap_with_children([child_a, child_b]) + + entries = await cap.list_resources() + + uris = sorted(e.uri for e in entries) + assert uris == ["mcp://a/one", "mcp://a/two", "mcp://b/three"] + + +async def test_read_resource_returns_child_result() -> None: + result = [{"uri": "mcp://a/one", "text": "content"}] + child = FakeResourceChild("a", [], {"mcp://a/one": result}) + cap = _cap_with_children([child]) + + got = await cap.read_resource("mcp://a/one") + + assert got == result + assert child.read_calls == ["mcp://a/one"] + + +async def test_read_resource_in_turn_skips_none() -> None: + child_hit = FakeResourceChild("hit", [], {"mcp://file/one": {"text": "x"}}) + child_miss = FakeResourceChild("miss", [], {}) + cap = _cap_with_children([child_hit, child_miss]) + + got = await cap.read_resource("mcp://file/one") + + assert got == {"text": "x"} + # The child that has the resource is consulted first and its result + # returned immediately; the remaining children are not queried. + assert child_hit.read_calls == ["mcp://file/one"] + assert child_miss.read_calls == [] + + +async def test_read_resource_returns_none_when_absent_everywhere() -> None: + child = FakeResourceChild("a", [], {}) + cap = _cap_with_children([child]) + + assert await cap.read_resource("mcp://nowhere") is None + + +async def test_read_resource_skips_failing_child() -> None: + class ExplodingChild(FakeResourceChild): + async def read_resource(self, uri: str) -> Any: + raise ConnectionError("boom") + + good = FakeResourceChild("good", [], {"mcp://x": {"text": "ok"}}) + bad = ExplodingChild("bad", [], {}) + cap = _cap_with_children([bad, good]) + + got = await cap.read_resource("mcp://x") + + assert got == {"text": "ok"} + + +async def test_resource_exists_returns_true_if_any_child_has_it() -> None: + child_hit = FakeResourceChild("hit", [], {"mcp://file/one": {"text": "x"}}) + child_miss = FakeResourceChild("miss", [], {}) + cap = _cap_with_children([child_hit, child_miss]) + + assert await cap.resource_exists("mcp://file/one") is True + + +async def test_resource_exists_false_when_none_have_it() -> None: + child = FakeResourceChild("a", [], {}) + cap = _cap_with_children([child]) + + assert await cap.resource_exists("mcp://nowhere") is False + + +async def test_resource_exists_skips_failing_child() -> None: + class ExplodingChild(FakeResourceChild): + async def resource_exists(self, uri: str) -> bool: + raise ConnectionError("boom") + + good = FakeResourceChild("good", [], {"mcp://x": {"text": "ok"}}) + bad = ExplodingChild("bad", [], {}) + cap = _cap_with_children([bad, good]) + + assert await cap.resource_exists("mcp://x") is True diff --git a/tests/delegation/test_pool_skills.py b/tests/delegation/test_pool_skills.py index d03c70161..68a919874 100644 --- a/tests/delegation/test_pool_skills.py +++ b/tests/delegation/test_pool_skills.py @@ -6,7 +6,7 @@ from __future__ import annotations -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, Any, Self import pytest from upathtools import UPath @@ -350,3 +350,110 @@ async def test_register_before_setup_buffers_and_drains( async with AgentPool(manifest_with_skills) as pool: pending = getattr(pool, "_pending_skill_providers", []) assert len(pending) == 0 + + +# ============================================================================= +# Test Class: TopLevelMcpRegistration (RFC-0058) +# ============================================================================= + + +@pytest.mark.integration +class TestTopLevelMcpPoolRegistration: + """Top-level McpServerCap instances are independently registered at POOL scope.""" + + @pytest.fixture + def manifest_with_mcp(self) -> AgentsManifest: + """Create a manifest with a top-level MCP server configured.""" + from wolfharness_config.mcp_server import StreamableHTTPMCPServerConfig + + agent_config = NativeAgentConfig( + name="test_agent", + model="test", + system_prompt="You are a test agent", + ) + return AgentsManifest( + agents={"test_agent": agent_config}, + mcp_servers=[ + StreamableHTTPMCPServerConfig( + url="http://127.0.0.1:1/mcp", + name="kb", + ) + ], + ) + + @staticmethod + def _attach_cap(pool: Any) -> None: + """Inject a provider directly for registration testing. + + This avoids requiring a live MCP server connection. + """ + from wolfharness.capabilities.mcp_server_cap import McpServerCap + + cap = McpServerCap( + pool.mcp.servers[0], + name="pool_mcp_kb", + client=object(), + ) + pool.mcp.providers.append(cap) + + async def test_mcp_servers_registered_at_pool_scope( + self, + manifest_with_mcp: AgentsManifest, + ) -> None: + """The top-level McpServerCap is discoverable via get_resource_access().""" + from wolfharness.capabilities.extension_registry import Scope, ScopeLevel + from wolfharness.capabilities.mcp_server_cap import McpServerCap + + async with AgentPool(manifest_with_mcp) as pool: + self._attach_cap(pool) + await pool._rebuild_skill_capabilities() + pool_scope = Scope(level=ScopeLevel.POOL) + + visible = pool.extension_registry.get_resource_access(pool_scope) + mcp_caps = [c for c in visible if isinstance(c, McpServerCap)] + + assert mcp_caps, "McpServerCap should be registered at POOL scope" + + async def test_mcp_server_tool_prefix_de_duplicated( + self, + manifest_with_mcp: AgentsManifest, + monkeypatch: Any, + ) -> None: + """Two servers sharing a display_name get unique tool_prefix values.""" + from wolfharness.mcp_server.manager import MCPManager + from wolfharness_config.mcp_server import StreamableHTTPMCPServerConfig + + # Patch MCPClient so setup_server() proceeds without a live server. + class _FakeClient: + def __init__(self, *args: object, **kwargs: object) -> None: + pass + + async def __aenter__(self) -> Self: + return self + + async def __aexit__(self, *args: object) -> None: + return None + + monkeypatch.setattr( + "wolfharness.mcp_server.client.MCPClient", + _FakeClient, + ) + + manager = MCPManager( + name="pool_mcp", + servers=[ + StreamableHTTPMCPServerConfig( + url="http://127.0.0.1:1/mcp", + name="kb", + ), + StreamableHTTPMCPServerConfig( + url="http://127.0.0.1:2/mcp", + name="kb", + ), + ], + ) + async with manager: + prefixes = [p.tool_prefix for p in manager.providers] + assert len(prefixes) == 2 + assert prefixes[0] == "kb" + assert prefixes[1] == "kb_2" diff --git a/tests/host/test_factory.py b/tests/host/test_factory.py index 16b3aa634..6dc466d27 100644 --- a/tests/host/test_factory.py +++ b/tests/host/test_factory.py @@ -162,12 +162,15 @@ def test_compile_registers_config_capabilities_at_agent_scope( agent_scope = Scope(level=ScopeLevel.AGENT, agent_name="test_agent") visible = minimal_pool.extension_registry.get_visible_capabilities(agent_scope) - # TestResourceAccessCap should be at AGENT scope + # TestResourceAccessCap should be at AGENT scope. from wolfharness.capabilities.resource_protocols import ResourceAccess ra_caps = [c for c in visible if isinstance(c, ResourceAccess)] - assert len(ra_caps) == 1 - assert isinstance(ra_caps[0], TestResourceAccessCap) + # SkillManagerCap now also implements ResourceAccess (RFC-0058); filter to + # the config-defined capability under test. + test_ra_caps = [c for c in ra_caps if isinstance(c, TestResourceAccessCap)] + assert len(test_ra_caps) == 1 + assert isinstance(test_ra_caps[0], TestResourceAccessCap) def test_get_visible_capabilities_no_duplicates_across_scopes( From e629c896291c25e66a1187575405174ad34a718d Mon Sep 17 00:00:00 2001 From: Million <15158090088@163.com> Date: Sat, 15 Aug 2026 15:08:16 +0800 Subject: [PATCH 04/11] docs(rfc): add RFC-0058 unified MCP server registration design Describes the dual-object problem (tools via get_capabilities, resources via SkillManagerCap.children) and the decision to register each top-level McpServerCap independently at POOL scope with direct tool injection. Documents the RFC-0051 -> RFC-0052 -> RFC-0058 decision lineage and the open question on top-level vs skill-MCP prefix convention divergence. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus --- ...-0058-mcp-resource-unified-registration.md | 842 ++++++++++++++++++ 1 file changed, 842 insertions(+) create mode 100644 docs/rfcs/draft/RFC-0058-mcp-resource-unified-registration.md diff --git a/docs/rfcs/draft/RFC-0058-mcp-resource-unified-registration.md b/docs/rfcs/draft/RFC-0058-mcp-resource-unified-registration.md new file mode 100644 index 000000000..52b1d9aee --- /dev/null +++ b/docs/rfcs/draft/RFC-0058-mcp-resource-unified-registration.md @@ -0,0 +1,842 @@ +--- +rfc_id: RFC-0058 +title: "Unified MCP Server Registration: Eliminating the Dual-Object Problem for Tool and Resource Access" +status: DRAFT +author: pinjun.mo +reviewers: + - name: yuchen.liu + status: pending +created: 2026-08-14 +last_updated: 2026-08-14 (v4: RFC-0051→0052→0058 decision lineage added to Historical Context; open question #7 — top-level vs skill-MCP prefix convention divergence) +decision_date: +related_rfcs: + - RFC-0051 (Extension Source Architecture — Resource Protocols and Client Injection) + - RFC-0052 (Restore Skill Capabilities — SkillManagerCap children wiring) +related_specs: + - docs/specs/mcp-resource-technical-report.md (MCP Resource consumption architecture) +--- + +# RFC-0058: Unified MCP Server Registration — Eliminating the Dual-Object Problem for Tool and Resource Access + +## Table of Contents + +- [Overview](#overview) +- [Background & Context](#background--context) +- [Problem Statement](#problem-statement) +- [Goals & Non-Goals](#goals--non-goals) +- [Evaluation Criteria](#evaluation-criteria) +- [Options Analysis](#options-analysis) +- [Recommendation](#recommendation) +- [Technical Design](#technical-design) +- [Security Considerations](#security-considerations) +- [Implementation Plan](#implementation-plan) +- [Open Questions](#open-questions) +- [Decision Record](#decision-record) +- [References](#references) + +--- + +## Overview + +AgentPool currently maintains two independent object representations for each top-level MCP server: a `McpServerCap` instance (created by `MCPManager.setup_server()`) and a pydantic-ai `MCP` capability (built on-demand by `MCPManager.get_capabilities()`). The former handles resource/skill protocol access; the latter handles tool exposure. This dual-object design causes `@` mention resource access to fail for top-level MCP servers, creates state synchronization risk, and complicates the tool injection path. + +This RFC proposes **unifying on `McpServerCap` as the single object representation** for each MCP server — responsible for both tool exposure (via `get_toolset()`) and resource access (via `ResourceAccess` protocol). The `MCPManager.get_capabilities()` path for top-level servers is retired in favor of direct `McpServerCap` injection into agent tool capabilities. `SkillManagerCap` gains `ResourceAccess` delegation for skill-level MCP children. + +**Expected outcome**: `@` mention works for all MCP servers, tool and resource access share a single object per server, and the `MCPManager.get_capabilities()` legacy path is removed for top-level servers. + +--- + +## Background & Context + +### Current State + +The MCP server lifecycle in AgentPool involves two parallel paths: + +**Path A — McpServerCap (resource/skill path)**: +- `pool.py:175` creates `MCPManager(servers=top_level_servers)` +- `MCPManager.__aenter__()` (manager.py:320) calls `setup_server()` per server +- `setup_server()` (manager.py:420) creates `McpServerCap(config, client=pre_created_client)`, appends to `self.providers` +- `_rebuild_skill_capabilities()` (pool.py:656) filters providers by `isinstance(provider, SkillResource)`, stuffs them into `SkillManagerCap.children` +- `SkillManagerCap` registers at POOL scope (pool.py:686) +- `SkillManagerCap.get_toolset()` (skill_manager_cap.py:448) **fully overrides** `CombinedToolsetCapability.get_toolset()`, does NOT call `super()`, does NOT iterate `self._children` for toolset purposes — children serve only `SkillResource`/`CommandResource` protocol queries + +**Path B — pydantic-ai MCP capability (tool path)**: +- `NativeAgent.__init__()` (agent.py:358) calls `self.mcp.get_aggregating_provider()` → `CombinedToolsetCapability` over ACP-only providers → appended to `_external_capabilities` +- `get_agentlet()` (agent.py:1123) calls `self.mcp.get_capabilities(session_id)` → builds fresh pydantic-ai `MCP` capabilities from config snapshot → `tool_capabilities.extend(mcp_capabilities)` +- `MCPManager.get_capabilities()` (manager.py:649) reads `McpConfigSnapshot`, creates `MCPToolset` per server via `GlobalConnectionPool.get_transport()`, wraps in `MCP(url, local=toolset)` + +**Path C — ACP aggregating provider (ACP-only tool path)**: +- `get_aggregating_provider()` (manager.py:637) filters `isinstance(p.config, AcpMCPServerConfig)`, wraps in `CombinedToolsetCapability` +- Injected via `_inject_pool_providers()` (factory.py:883) into `_external_capabilities` for child sessions + +### Historical Context + +**Decision lineage: RFC-0051 designed unification → RFC-0052 split it → RFC-0058 restores it.** + +- **RFC-0051 (Extension Source Architecture, 2026-07-11)** designed `McpServerCap` (then `McpResource`) to implement `ResourceAccess` and be **independently registered in `ExtensionRegistry` at POOL scope** — its lifecycle diagram (`register(McpServerCap(config), POOL)`, §Lifecycle Management) shows it as a standalone capability, not a child of `SkillManagerCap`. Tools were designed to flow through `AgentFactory.compile() → McpServerCap.get_toolset()`. The intent was **one server = one capability = tools + resources together**. +- **RFC-0052 (Restore Skill Capabilities, 2026-07-12)** split this design to fix three M3 skill regressions. Its **Option B split registration along the resource/tool boundary**: (a) top-level McpServerCap instances implementing `SkillResource` were stuffed into `SkillManagerCap.children` (pool.py `_rebuild_skill_capabilities()`) to restore `skill://` URI resolution for remote skills — silently removing the independent POOL-scope registration that RFC-0051 specified; (b) the **tool surface was left on the untouched `MCPManager.get_capabilities()` path**. RFC-0052's D2 decision (fully-rewritten `SkillManagerCap.get_toolset()`) explicitly declared non-skill children "unprefixed" (get_toolset() case 3): children in `_capabilities` other than per-skill MCP were appended to the combined toolset **without** a `PrefixedToolset` wrapper (implemented in commit `6f07fd36f`, `src/agentpool/capabilities/skill_manager_cap.py`). The implicit assumption was that non-skill children are independent, mutually-distinct capabilities that never collide. **That assumption breaks when multiple top-level MCP servers are configured** — their McpServerCap instances are all stuffed into `children`, and two servers exposing the same tool name (e.g., both have `search`) collide silently under the unprefixed path. RFC-0058's per-server prefixing corrects this false assumption. +- The `MCPManager.get_capabilities()` path predates `McpServerCap` and was **not retired when `McpServerCap` was introduced** — RFC-0051's implementation (commit `7d7cf9560`) assumed `get_capabilities()` would be narrowed to session/skill scope, but the top-level call in `get_agentlet()` was left in place. As a result, the McpServerCap tool path designed in RFC-0051 is effectively dead code today: the `SkillManagerCap.get_toolset()` override does not iterate `children` for tools, and tools for top-level servers come exclusively from `get_capabilities()`. The dual-object problem is the accumulated outcome of these two decisions, not a single deliberate split. +- **RFC-0058 restores RFC-0051's design intent**: independently register McpServerCap at POOL scope (Phase 2), migrate tool exposure back to `McpServerCap.get_toolset()` (Phase 3), and narrow `get_capabilities()` to session/skill scope. This is a restoration, not a novel architecture. + +### Glossary + +| Term | Definition | +|------|------------| +| McpServerCap | AgentPool capability wrapping a single MCP server connection; implements `ResourceAccess`, `SkillResource`, `CommandResource`, `ToolAccess`, `ChangeObservable` | +| MCPManager | Manages lifecycle of top-level and agent-level MCP servers; creates McpServerCap instances; provides `get_capabilities()` and `get_aggregating_provider()` | +| ExtensionRegistry | 4-level scope registry (POOL > AGENT > SESSION > TURN) for capability lookup | +| ResourceAccess | Protocol providing `list_resources()`, `read_resource()`, `resource_exists()` | +| SkillManagerCap | Capability managing local skills, per-skill MCP, and remote skill discovery; inherits `CombinedToolsetCapability` | +| Dual-object problem | Same MCP server represented by two objects: McpServerCap (resources) and pydantic-ai MCP (tools) | +| `@` mention | Editor-driven resource injection via `GET /experimental/resource` endpoint → `list_mcp_resources()` → `registry.get_resource_access()` | +| ResourceCapability | Agent-facing capability exposing 5 resource tools (`list_resources`, `read_resource`, etc.) for model-initiated resource access | + +--- + +## Problem Statement + +### The Problem + +1. **`@` mention cannot access top-level MCP resources**: `list_mcp_resources()` (agent_routes.py:492) calls `registry.get_resource_access(scope)`, which returns capabilities implementing `ResourceAccess`. `McpServerCap` implements `ResourceAccess`, but it is registered as a child of `SkillManagerCap`, which does NOT implement `ResourceAccess`. The registry only sees directly-registered capabilities, not their children. Therefore `get_resource_access()` returns an empty list for top-level MCP servers. + +2. **Dual-object state divergence**: The same MCP server has two object representations — `McpServerCap` with a pre-created `MCPClient` (in `MCPManager.providers`) and pydantic-ai `MCP` with a separately created `MCPToolset` (in `get_capabilities()`). Each maintains its own connection. Connection state, caching, and error handling can diverge. + +3. **Unnecessary connection multiplicity**: For a single MCP server, two TCP connections may be established — one by `MCPClient` (in `setup_server()`) and one by `MCPToolset` (in `get_capabilities()` via `GlobalConnectionPool`). This doubles resource consumption and complicates connection lifecycle management. + +4. **Tool name collisions across MCP servers**: Neither the `McpServerCap.get_toolset()` path nor the `MCPManager.get_capabilities()` path applies a server-level prefix to tool names. `MCPClient.convert_tool()` (client.py:557) sets `tool_callable.__name__ = tool.name` — the raw MCP tool name with no namespace. pydantic-ai's `MCPToolset` provides a `tool_name_conflict_hint` suggesting `.prefixed("...")` but does not auto-prefix. `load_mcp_toolsets()` (mcp.py:1754) demonstrates the intended pattern — `toolset.prefixed(name)` — but `get_capabilities()` does not follow it. If two MCP servers expose a tool with the same name (e.g., both have `search`), pydantic-ai's `CombinedToolset` will silently overwrite one with the other. Skill-level MCP already solves this via `PrefixedToolset(prefix=f"{skill_name}__mcp__")` (skill_manager_cap.py:548), but top-level MCP has no equivalent. + +### Evidence + +- `GET /experimental/resource` returns `200 OK` with empty `{}` body when only top-level MCP servers are configured (observed in `~/Library/Logs/agentpool/opencode.log`) +- `curl` to MCP server at `localhost:8002/mcp` confirms 5 resources are exposed via `resources/list` protocol +- Code trace confirms `SkillManagerCap` does not implement `ResourceAccess` (skill_manager_cap.py class declaration), and `get_resource_access()` (extension_registry.py:389) filters by `isinstance(cap, ResourceAccess)` +- `SkillManagerCap.get_toolset()` (skill_manager_cap.py:448-554) does not iterate `self._children` for toolset — confirmed by full method read + +### Impact of Inaction + +- **Cost**: `@` mention feature is broken for all top-level MCP servers, requiring users to manually paste resource content or use model-initiated tools (which may not always be appropriate) +- **Risk**: Dual connections increase the chance of connection exhaustion, stale state, and inconsistent error handling +- **Opportunity**: Without unification, every new MCP-related feature (e.g., resource subscription, change notification) must be implemented twice — once for McpServerCap, once for the pydantic-ai MCP path + +--- + +## Goals & Non-Goals + +### Goals (In Scope) + +1. `@` mention works for top-level MCP servers registered at POOL scope +2. `@` mention works for skill-level MCP servers via `SkillManagerCap` ResourceAccess delegation +3. `McpServerCap` is the single object representation per MCP server — responsible for both tool exposure and resource access +4. `MCPManager.get_capabilities()` is retired for top-level servers (Path B is eliminated) +5. No duplicate tool exposure for any MCP server +6. **Tool names are namespaced per server — no silent collisions when multiple MCP servers expose same-named tools** +7. Model-initiated resource access via `ResourceCapability` continues to work unchanged +8. ACP transport MCP servers continue to work via the aggregating provider path (Path C) + +### Non-Goals (Out of Scope) + +1. MCP server-side resource exposure (AgentPool as MCP server) — documented as a gap in the technical report (§5.9), not addressed here +2. URI conflict detection across multiple MCP servers — deferred to a follow-up; custom scheme convention is assumed +3. Refactoring of session-scoped MCP config snapshot mechanism — the `McpConfigSnapshot` continues to serve session-scoped and skill-scoped configs +4. `ResourceCapability` redesign — it is already a registry consumer and requires no changes +5. **Tool Registry / Tool Selection / semantic routing** — the "1000 tools, show the model 10-30" problem (tool retrieval, `search_tools` tool, intent-based routing between same-named tools across servers). This RFC only solves registration and flat namespacing for the current config-driven scale (single-digit servers). Tool selection and retrieval are a separate layer to be designed in a follow-up RFC. OpenCode's direction (server namespace + permission/agent filtering + Code Mode for context control) is acknowledged as the target architecture but not implemented here. + +### Success Criteria + +- [ ] `GET /experimental/resource` returns resources from all configured top-level MCP servers +- [ ] `GET /experimental/resource` returns resources from skill-level MCP servers +- [ ] Model can call `list_resources` / `read_resource` tools and receive results from all MCP servers +- [ ] Model can call MCP tools (e.g., `search_database`) from top-level MCP servers +- [ ] No duplicate tools appear in the agent's tool list for any MCP server +- [ ] Tools from different MCP servers with the same raw name are distinguishable (no silent overwrite) +- [ ] Only one TCP connection per MCP server (verified by connection count) +- [ ] ACP transport MCP servers continue to expose tools via aggregating provider + +--- + +## Evaluation Criteria + +| Criterion | Weight | Description | Minimum Threshold | +|-----------|--------|-------------|-------------------| +| Resource access correctness | High | `@` mention and `ResourceCapability` can discover and read resources from all MCP server types | 100% of configured servers | +| Tool exposure correctness | High | All MCP tools reach the agent's `tool_capabilities` without duplication | 0 duplicates | +| Tool namespacing | High | Tools from different servers with same raw name are distinguishable | 0 silent collisions | +| Architectural simplicity | High | Number of object representations per MCP server | 1 | +| Backward compatibility | Medium | Existing YAML configs work without modification | All existing configs | +| Implementation effort | Medium | Lines of code changed, files touched, test updates needed | < 500 LOC changed | +| ACP transport support | Medium | ACP MCP servers continue to work through their existing path | No regression | +| Connection efficiency | Low | TCP connections per MCP server | 1 | + +--- + +## Options Analysis + +### Option 1: Unified McpServerCap Registration (Recommended) + +**Description** + +Register each top-level `McpServerCap` independently at POOL scope in `ExtensionRegistry`. Replace `MCPManager.get_capabilities()` call in `get_agentlet()` with direct injection of `McpServerCap` instances into `tool_capabilities`. Add `ResourceAccess` delegation to `SkillManagerCap` for skill-level MCP children. + +Key changes: +- `pool.py:_rebuild_skill_capabilities()`: Stop stuffing top-level McpServerCap into `SkillManagerCap.children`. Register each at POOL scope independently. +- `agent.py:get_agentlet()`: Replace `mcp_capabilities = await self.mcp.get_capabilities(session_id)` with direct injection of `McpServerCap` instances into `tool_capabilities`. Wrap each in a `PrefixedToolset` using the server's `display_name` as prefix. +- `mcp_server_cap.py:get_toolset()`: Wrap the returned `CombinedToolset` in a `PrefixedToolset` using the server's `display_name` as prefix (via a new `tool_prefix` property), so tools are automatically namespaced per server. +- `mcp_server/manager.py:setup_server()`: De-duplicate `display_name` across servers (append `_2`, `_3`, ... on collision) so each McpServerCap's tool prefix is unique even when two configured servers share the same name. +- `skill_manager_cap.py`: Add `ResourceAccess` protocol implementation that delegates to `_skill_mcp_children`. +- `agent_routes.py:list_mcp_resources()`: No change needed — `get_resource_access()` will now find POOL-scoped McpServerCap instances directly. + +**Advantages** + +- Single object per MCP server — `McpServerCap` handles both tools (`get_toolset()`) and resources (`ResourceAccess`) +- Tool namespacing via `PrefixedToolset` prevents silent collisions when multiple servers expose same-named tools +- `@` mention works for top-level MCP servers without any endpoint changes +- Eliminates the `MCPManager.get_capabilities()` path for top-level servers, removing ~180 lines of complex snapshot/transport/cache logic from the hot path +- Single TCP connection per server (the `MCPClient` created in `setup_server()`) +- Consistent with RFC-0051's original design intent — McpServerCap as independently-registered capability +- Namespacing pattern aligns with pydantic-ai's own `load_mcp_toolsets()` example (mcp.py:1754: `toolset.prefixed(name)`) + +**Disadvantages** + +- `MCPManager.get_capabilities()` must remain for session-scoped and skill-scoped MCP configs (it cannot be fully removed) +- `get_agentlet()` must handle two tool injection paths: McpServerCap instances (top-level) and `get_capabilities()` (session-scoped) — though the latter is simplified +- Session-scoped MCP config isolation currently relies on `McpConfigSnapshot` + `get_capabilities()` partition; retiring it for top-level configs means top-level McpServerCap instances are shared across all sessions (which is already the case for `MCPClient` connections) +- `SkillManagerCap` gains `ResourceAccess` implementation, slightly increasing its responsibility surface + +**Evaluation Against Criteria** + +| Criterion | Rating | Notes | +|-----------|--------|-------| +| Resource access correctness | Excellent | POOL-scope registration makes McpServerCap directly discoverable by `get_resource_access()` | +| Tool exposure correctness | Excellent | `get_toolset()` returns lazy ToolsetFunc wrapped in PrefixedToolset; no duplication risk since `get_capabilities()` path is removed for top-level | +| Tool namespacing | Excellent | PrefixedToolset with server display_name; follows pydantic-ai's own convention | +| Architectural simplicity | Excellent | Single object per server; RFC-0051 alignment | +| Backward compatibility | Good | YAML config unchanged; `get_capabilities()` retained for session/skill scope | +| Implementation effort | Medium | ~300 LOC across pool.py, agent.py, skill_manager_cap.py; test updates needed | +| ACP transport support | Excellent | ACP aggregating provider path (Path C) is unaffected | +| Connection efficiency | Excellent | Single MCPClient per server, no MCPToolset duplicate | + +**Effort Estimate** + +- Complexity: Medium +- Resources: 1 engineer, 2-3 days +- Dependencies: None (self-contained refactor) + +**Risk Assessment** + +| Risk | Likelihood | Impact | Mitigation | +|------|------------|--------|------------| +| Session-scoped tool isolation breaks | Low | Medium | Top-level servers were always shared; session isolation only applies to session/skill configs which still use `get_capabilities()` | +| `get_toolset()` lazy client fails | Low | High | `setup_server()` pre-creates `MCPClient`, so `_ensure_client()` returns immediately | +| `for_run()` not overridden on McpServerCap | Low | Low | McpServerCap inherits default `for_run()` → returns `self`; connection is server-scoped, sharing across runs is intended | + +--- + +### Option 2: SkillManagerCap ResourceAccess Proxy Only + +**Description** + +Keep top-level McpServerCap instances inside `SkillManagerCap.children` (no change to registration). Add `ResourceAccess` implementation to `SkillManagerCap` that delegates to all children implementing `ResourceAccess`. Keep `MCPManager.get_capabilities()` for tool exposure unchanged. + +Key changes: +- `skill_manager_cap.py`: Add `ResourceAccess` protocol implementation delegating to `self._children`. +- No changes to `pool.py`, `agent.py`, or `manager.py`. + +**Advantages** + +- Minimal code change — only `skill_manager_cap.py` is modified +- No risk to tool exposure path — `get_capabilities()` continues as-is +- `SkillManagerCap` already delegates `SkillResource` and `CommandResource`; adding `ResourceAccess` follows the same pattern + +**Disadvantages** + +- Dual-object problem persists — `McpServerCap` for resources, pydantic-ai `MCP` for tools +- Two TCP connections per server remain +- `SkillManagerCap.children` semantically wrong — top-level MCP servers are not skills +- `get_capabilities()` complexity remains in the hot path +- Future features must still be implemented in two places + +**Evaluation Against Criteria** + +| Criterion | Rating | Notes | +|-----------|--------|-------| +| Resource access correctness | Good | `@` mention works via delegation, but only for children that implement `ResourceAccess` | +| Tool exposure correctness | Good | No change to existing path; but dual-object problem means tools and resources may diverge | +| Tool namespacing | Poor | `get_capabilities()` does not prefix tools; collision risk remains | +| Architectural simplicity | Poor | Dual-object problem remains; SkillManagerCap semantically overloaded | +| Backward compatibility | Excellent | No changes to any other file | +| Implementation effort | Low | ~80 LOC in skill_manager_cap.py only | +| ACP transport support | Excellent | Unaffected | +| Connection efficiency | Poor | Two connections per server persist | + +**Effort Estimate** + +- Complexity: Low +- Resources: 1 engineer, 0.5 days +- Dependencies: None + +**Risk Assessment** + +| Risk | Likelihood | Impact | Mitigation | +|------|------------|--------|------------| +| `SkillManagerCap` becomes too broad | Medium | Low | Acceptable trade-off for minimal-change approach | +| Future features require dual implementation | High | Medium | Document as known tech debt | + +--- + +### Option 3: Hybrid — Independent Registration + Retain get_capabilities() + +**Description** + +Register top-level McpServerCap independently at POOL scope (like Option 1) for resource access, but retain `MCPManager.get_capabilities()` for tool exposure (like Option 2). Add `ResourceAccess` delegation to `SkillManagerCap` for skill-level MCP. + +Key changes: +- `pool.py`: Register McpServerCap at POOL scope, AND keep them in `SkillManagerCap.children`. +- `agent.py`: No change to `get_capabilities()` call. +- `skill_manager_cap.py`: Add `ResourceAccess` delegation to children. +- `agent_routes.py`: Deduplicate `get_resource_access()` results (McpServerCap appears both at POOL scope and as SkillManagerCap child). + +**Advantages** + +- Resource access works immediately via POOL-scope registration +- Tool exposure path is untouched — zero risk of tool regression +- Gradual migration path — can retire `get_capabilities()` later + +**Disadvantages** + +- McpServerCap registered twice (POOL scope + SkillManagerCap child) — deduplication needed +- Dual-object problem persists +- `SkillManagerCap.children` still semantically wrong +- Most complex of the three options — adds registration without removing the old path + +**Evaluation Against Criteria** + +| Criterion | Rating | Notes | +|-----------|--------|-------| +| Resource access correctness | Good | Works but requires deduplication in `get_resource_access()` consumers | +| Tool exposure correctness | Good | No change to existing path | +| Tool namespacing | Poor | `get_capabilities()` path unchanged; collision risk remains | +| Architectural simplicity | Poor | Adds a path without removing the old one; highest complexity | +| Backward compatibility | Excellent | All existing paths preserved | +| Implementation effort | Medium | ~200 LOC but with deduplication complexity | +| ACP transport support | Excellent | Unaffected | +| Connection efficiency | Poor | Two connections per server persist | + +**Effort Estimate** + +- Complexity: Medium +- Resources: 1 engineer, 1-2 days +- Dependencies: None + +**Risk Assessment** + +| Risk | Likelihood | Impact | Mitigation | +|------|------------|--------|------------| +| Duplicate resources in `@` mention list | High | Low | Deduplicate by URI in `list_mcp_resources()` | +| Confusion from dual registration | Medium | Low | Document as transitional state | + +--- + +### Options Comparison Summary + +| Criterion | Option 1: Unified | Option 2: Proxy Only | Option 3: Hybrid | +|-----------|-------------------|---------------------|-----------------| +| Resource access correctness | Excellent | Good | Good | +| Tool exposure correctness | Excellent | Good | Good | +| Tool namespacing | Excellent | Poor | Poor | +| Architectural simplicity | Excellent | Poor | Poor | +| Backward compatibility | Good | Excellent | Excellent | +| Implementation effort | Medium | Low | Medium | +| ACP transport support | Excellent | Excellent | Excellent | +| Connection efficiency | Excellent | Poor | Poor | +| **Overall** | **Best** | Acceptable (short-term) | Not recommended | + +--- + +## Recommendation + +### Recommended Option + +**Option 1: Unified McpServerCap Registration** + +### Justification + +Based on the evaluation criteria, Option 1 scores highest on architectural simplicity (single object per server, RFC-0051 alignment) and connection efficiency (single TCP connection). The implementation effort is moderate (~300 LOC) and the risk profile is manageable — the primary risk (session-scoped tool isolation) does not apply because top-level servers were always shared across sessions via `MCPClient`. + +Option 2 is viable as a short-term stopgap if implementation time is constrained, but it leaves the dual-object problem unresolved and accumulates tech debt. Option 3 adds complexity without removing the old path, making it the worst long-term option. + +### Accepted Trade-offs + +1. **`MCPManager.get_capabilities()` retained for session/skill scope**: The full retirement of `get_capabilities()` is not feasible in this RFC because session-scoped and skill-scoped MCP configs rely on the snapshot mechanism. This is acceptable — the dual-object problem only affects top-level servers, which are the common case. +2. **`SkillManagerCap` gains `ResourceAccess` responsibility**: This slightly broadens `SkillManagerCap`'s surface area, but the delegation pattern is identical to existing `SkillResource` and `CommandResource` delegation — no new architectural pattern is introduced. +3. **Top-level McpServerCap shared across all sessions**: This is already the behavior for `MCPClient` connections (created once in `setup_server()`). The change makes tool exposure consistent with this existing sharing semantics. + +### Conditions + +- ACP transport MCP servers must continue to work via the aggregating provider path without regression +- Existing tests for `MCPManager.get_capabilities()` must continue to pass (they exercise session-scoped configs) +- The `ResourceCapability` (model-initiated resource access) must work unchanged + +--- + +## Technical Design + +### Architecture Overview + +``` + ┌─────────────────────────────┐ + │ ExtensionRegistry │ + │ (POOL scope) │ + │ │ + │ ┌────────────────────┐ │ + │ │ SkillManagerCap │ │ + │ │ ├─ SkillResource │ │ + │ │ ├─ CommandResource│ │ + │ │ ├─ ResourceAccess │ ← NEW delegation + │ │ │ to _skill_mcp │ │ + │ │ │ _children │ │ + │ │ └─ get_toolset() │ │ + │ │ (builtin + │ │ + │ │ skill tools + │ │ + │ │ skill MCP) │ │ + │ └────────────────────┘ │ + │ │ + │ ┌────────────────────┐ │ + │ │ McpServerCap A │ ← NEW independent + │ │ ├─ ToolAccess │ registration + │ │ ├─ ResourceAccess │ │ + │ │ ├─ SkillResource │ │ + │ │ ├─ CommandResource│ │ + │ │ └─ get_toolset() │ │ + │ └────────────────────┘ │ + │ ┌────────────────────┐ │ + │ │ McpServerCap B │ ← NEW independent + │ │ └─ ... │ registration + │ └────────────────────┘ │ + └─────────────────────────────┘ + │ + ┌───────────────────┼───────────────────┐ + │ │ │ + ┌─────▼─────┐ ┌──────▼──────┐ ┌──────▼──────┐ + │ @ mention │ │ Resource │ │ get_agentlet│ + │ endpoint │ │ Capability │ │ tool_caps │ + │ │ │ (model tools)│ │ │ + └───────────┘ └─────────────┘ └─────────────┘ + get_resource_ list_resources pool.mcp.providers + access(scope) read_resource → McpServerCap + .get_toolset() +``` + +### Key Changes + +#### 1. pool.py — `_rebuild_skill_capabilities()` + +**Before**: +```python +mcp_children = [ + provider for provider in self.mcp.providers + if isinstance(provider, SkillResource) +] +cap = SkillManagerCap(local_skills=..., children=mcp_children, ...) +self._extension_registry.register(cap, pool_scope) +``` + +**After**: +```python +# Register each top-level McpServerCap independently at POOL scope +for provider in self.mcp.providers: + self._extension_registry.register(provider, pool_scope) + +# SkillManagerCap only manages local skills and per-skill MCP +cap = SkillManagerCap(local_skills=..., children=[], ...) +self._extension_registry.register(cap, pool_scope) +``` + +Note: `SkillManagerCap` still needs `SkillResource` access to top-level MCP providers for remote skill listing. This is handled via `ExtensionRegistry.get_skill_resources(scope)` which returns all POOL-scoped `SkillResource` implementations — including the independently registered McpServerCap instances. The `SkillURIResolver` registration at pool.py:610 is unchanged. + +**Dead code elimination**: With top-level McpServerCap no longer in `SkillManagerCap._capabilities`, RFC-0052's D2 case 3 ("non-skill children unprefixed") becomes dead code — `_capabilities` now only holds per-skill MCP children (already handled by case 2). The unprefixed branch in `skill_manager_cap.py:get_toolset()` SHOULD be removed to prevent future confusion and to ensure no capability silently escapes namespacing. + +#### 2. agent.py — `get_agentlet()` + +**Before**: +```python +# 4. MCP servers +mcp_capabilities = await self.mcp.get_capabilities( + session_id=run_ctx.session_id if run_ctx else None +) +tool_capabilities.extend(mcp_capabilities) +``` + +**After**: +```python +# 4. MCP servers — top-level: inject McpServerCap directly +pool = self._agent_pool +if pool is not None: + # Non-ACP top-level providers: inject as capabilities (tools via get_toolset()) + for provider in pool.mcp.providers: + if not isinstance(provider.config, AcpMCPServerConfig): + tool_capabilities.append(provider) + # Session-scoped configs (session + skill): still use get_capabilities() + session_mcp_caps = await self.mcp.get_capabilities( + session_id=run_ctx.session_id if run_ctx else None, + exclude_global=True, # ← NEW param to skip pool+agent configs + ) + tool_capabilities.extend(session_mcp_caps) +``` + +Note: `get_capabilities()` gains an `exclude_global` parameter to skip pool-level and agent-level configs (already handled by McpServerCap injection). It continues processing session-scoped and skill-scoped configs. ACP providers continue through the aggregating provider path (Path C, unchanged). + +#### 3. mcp_server_cap.py — `get_toolset()` with PrefixedToolset + +**Before**: +```python +def get_toolset(self) -> Any: + async def _build_toolset(ctx): + client = await self._ensure_client() + tools = await client.list_tools() + if not tools: + return None + converted = [client.convert_tool(t) for t in tools] + pydantic_tools = [wrap_tool_for_pydantic_ai(tool) for tool in converted] + toolsets = [FunctionToolset[Any]([tool]) for tool in pydantic_tools] + return CombinedToolset(toolsets) + return _build_toolset +``` + +**After**: +```python +def get_toolset(self) -> Any: + async def _build_toolset(ctx): + client = await self._ensure_client() + tools = await client.list_tools() + if not tools: + return None + converted = [client.convert_tool(t) for t in tools] + pydantic_tools = [wrap_tool_for_pydantic_ai(tool) for tool in converted] + toolsets = [FunctionToolset[Any]([tool]) for tool in pydantic_tools] + combined = CombinedToolset(toolsets) + # Namespace tools by server name to prevent cross-server collisions. + # Follows pydantic-ai's own convention: load_mcp_toolsets() uses + # toolset.prefixed(name) at mcp.py:1754. + # Skill-level MCP already uses PrefixedToolset(prefix=f"{skill}__mcp__"). + return PrefixedToolset(wrapped=combined, prefix=self._tool_prefix) + return _build_toolset +``` + +Naming convention: the prefix is derived from the server's `display_name` only — the manager prefix (`pool_mcp_`) is **not** included, as it carries no semantic information for the model. Aligns with OpenCode's `_` convention (e.g., `github_search`, `slack_send_message`). For a pool-level server with `display_name` `xeno-kb`, the prefix is `xeno-kb`. A tool named `search_database` becomes `xeno-kb_search_database` in the model's tool list. `McpServerCap` gains a `_tool_prefix` property: + +```python +class McpServerCap(...): + def __init__(self, config, *, name=None, ...): + # `name` retains the manager-qualified identifier (pool_mcp_xeno-kb) + # for status/logging/internal identity; `_tool_prefix` is the + # model-visible namespace (xeno-kb). + self._name = name or config.client_id + self._tool_prefix = config.display_name + + @property + def tool_prefix(self) -> str: + return self._tool_prefix +``` + +Design note: The prefix uses `_` (underscore) as separator, consistent with `PrefixedToolset`'s implementation (`f'{self.prefix}_{name}'`, prefixed.py:32), with skill-level MCP's `__mcp__` convention, and with OpenCode's current `_` naming (per OpenCode MCP docs). The model sees fully-qualified names; the `PrefixedToolset.call_tool()` method (prefixed.py:38) strips the prefix before dispatching to the original tool, so `MCPClient.call_tool()` receives the raw tool name as before. + +#### 3. skill_manager_cap.py — Add ResourceAccess delegation + +**New methods**: +```python +async def list_resources(self) -> Sequence[ResourceEntry]: + """Delegate to skill-level MCP children implementing ResourceAccess.""" + entries: list[ResourceEntry] = [] + for caps in self._skill_mcp_children.values(): + for cap in caps: + if isinstance(cap, ResourceAccess): + try: + entries.extend(await cap.list_resources()) + except Exception: + continue + return entries + +async def read_resource(self, uri: str) -> list[TextResourceContent | BlobResourceContent] | None: + """Delegate to skill-level MCP children.""" + for caps in self._skill_mcp_children.values(): + for cap in caps: + if isinstance(cap, ResourceAccess): + try: + result = await cap.read_resource(uri) + except Exception: + continue + if result is not None: + return result + return None + +async def resource_exists(self, uri: str) -> bool: + """Delegate to skill-level MCP children.""" + for caps in self._skill_mcp_children.values(): + for cap in caps: + if isinstance(cap, ResourceAccess) and await cap.resource_exists(uri): + return True + return False +``` + +#### 4. manager.py — `get_capabilities()` adjustment + +Add `exclude_global: bool = False` parameter. When `True`, skip `snap.global_configs` processing (pool + agent configs) since those are handled by McpServerCap injection. Session-scoped configs continue to be processed. + +```python +async def get_capabilities( + self, + session_id: str | None = None, + *, + exclude_global: bool = False, +) -> list[MCP]: + ... + if ctx is not None and ctx.snapshot is not None: + if not exclude_global: + await _process_global_configs(ctx.snapshot, self._toolset_cache) + if ctx.connection_pool is not None: + await _process_session_configs(...) + else: + if not exclude_global: + # Legacy path: process self.servers + for server in self.servers: + ... + return capabilities +``` + +#### 5. Unchanged components + +| Component | Why unchanged | +|-----------|---------------| +| `agent_routes.py:list_mcp_resources()` | Already calls `registry.get_resource_access(scope)` — now finds POOL-scoped McpServerCap directly | +| `resource_capability.py` | Already calls `registry.get_resource_access(scope)` — benefits automatically | +| `resource_resolver.py` | Already iterates `resource_caps` from registry — benefits automatically | +| `mcp_server_cap.py` | `get_toolset()` and `ResourceAccess` implementations are already correct | +| `factory.py:_inject_pool_providers()` | ACP aggregating provider injection (Path C) is unchanged | +| `MCPManager.setup_server()` | McpServerCap creation logic is unchanged | +| `MCPManager.get_aggregating_provider()` | ACP-only filtering is unchanged | + +### Data Flow After Changes + +**`@` mention** (editor → resource list): +``` +GET /experimental/resource + → list_mcp_resources() + → registry.get_resource_access(SESSION scope) + → returns: [McpServerCap_A, McpServerCap_B, SkillManagerCap] + → McpServerCap_A.list_resources() → MCP resources/list → 5 entries + → McpServerCap_B.list_resources() → MCP resources/list → 3 entries + → SkillManagerCap.list_resources() → delegates to _skill_mcp_children → 2 entries + → Total: 10 resources, aggregated, returned to editor +``` + +**Model tool call** (model → MCP tool): +``` +Model calls search_database(query="...") + → pydantic-ai resolves tool from capabilities list + → McpServerCap.get_toolset() returned ToolsetFunc + → ToolsetFunc calls _ensure_client() → MCPClient (pre-created) + → client.call_tool("search_database", {"query": "..."}) + → Result returned to model +``` + +**Model resource access** (model → ResourceCapability): +``` +Model calls list_resources tool + → ResourceCapability.list_resources() + → registry.get_resource_access(scope) + → returns: [McpServerCap_A, McpServerCap_B, SkillManagerCap] + → Aggregated results formatted as text table + → Returned to model as tool result +``` + +--- + +## Security Considerations + +### Threat Analysis + +| Threat | Impact | Likelihood | Mitigation | +|--------|--------|------------|------------| +| POOL-scope McpServerCap visible to all agents | Medium | Low | Intended behavior — top-level MCP servers are pool-wide resources. Agent-level isolation is maintained by agent-scope registration. | +| Resource URI leakage across agents | Medium | Low | Same as above — top-level server resources are intentionally shared. Session-scoped resources remain isolated via `get_capabilities()` snapshot. | +| `get_capabilities()` session scope bypass | High | Low | `exclude_global` flag only skips global configs; session-scoped configs are still processed through the snapshot mechanism. | + +### Security Measures + +- [ ] Verify that session-scoped MCP configs (session + skill) are NOT affected by the `exclude_global` flag +- [ ] Confirm that `get_resource_access(SESSION scope)` does not leak TURN-scoped capabilities from other sessions +- [ ] Ensure McpServerCap connection sharing across sessions does not expose per-session state (e.g., MCP session headers) + +--- + +## Implementation Plan + +### Phases + +#### Phase 1: SkillManagerCap ResourceAccess Delegation + +- **Scope**: Add `ResourceAccess` implementation to `SkillManagerCap` for `_skill_mcp_children` +- **Deliverables**: Updated `skill_manager_cap.py`, unit tests for delegation +- **Dependencies**: None +- **Risk**: Low — additive change, no existing behavior modified + +#### Phase 2: Independent POOL-scope Registration + +- **Scope**: Stop stuffing top-level McpServerCap into `SkillManagerCap.children`; register independently at POOL scope. Add `display_name` de-duplication in `MCPManager.setup_server()`. +- **Deliverables**: Updated `pool.py` `_rebuild_skill_capabilities()`, updated `manager.py` `setup_server()`, updated tests +- **Dependencies**: Phase 1 (SkillManagerCap no longer needs children for ResourceAccess) +- **Risk**: Medium — `SkillManagerCap` loses direct access to top-level MCP SkillResource providers; must rely on `ExtensionRegistry.get_skill_resources()` instead. Verify that `list_skills` / `read_skill` / `list_commands` still work via registry queries. + +**display_name de-duplication** (in `manager.py:setup_server()`): the tool prefix derives from `display_name`, so two configured servers sharing a name would produce colliding prefixes. Resolve at server-setup time by tracking used names per manager and appending a numeric suffix on collision: + +```python +# manager.py — inside __aenter__, before creating providers: +used_names: set[str] = set() +for server in self.servers: + base = server.display_name + candidate = base + n = 2 + while candidate in used_names: + candidate = f"{base}_{n}" + n += 1 + used_names.add(candidate) + # Pass `display_name=candidate` when constructing the McpServerCap +``` + +Behavior: two servers both named `github` become tool prefixes `github` and `github_2`. The `McpServerCap._name` (manager-qualified internal id) remains unique and unchanged; only the model-visible `tool_prefix` is de-duplicated. + +#### Phase 3: Tool Exposure Migration + +- **Scope**: Replace `get_capabilities()` for top-level servers with direct McpServerCap injection in `get_agentlet()` +- **Deliverables**: Updated `agent.py`, `manager.py` (`exclude_global` param), integration tests +- **Dependencies**: Phase 2 (McpServerCap already at POOL scope) +- **Risk**: Medium — must ensure no duplicate tools and no missing tools. ACP path must be unaffected. + +#### Phase 4: Cleanup and Documentation + +- **Scope**: Update RFC-0051 references, technical report, AGENTS.md; remove dead code paths +- **Deliverables**: Documentation updates, dead code removal +- **Dependencies**: Phase 3 complete and tested +- **Risk**: Low + +### Milestones + +| Milestone | Description | Target | Status | +|-----------|-------------|--------|--------| +| M1 | Phase 1 + 2: `@` mention works for top-level MCP | Day 2 | Not Started | +| M2 | Phase 3: Tool exposure via McpServerCap | Day 3 | Not Started | +| M3 | Phase 4: Documentation and cleanup | Day 4 | Not Started | + +### Rollback Strategy + +Each phase is independently revertible: +- Phase 1: Remove `ResourceAccess` methods from `SkillManagerCap` +- Phase 2: Restore `children=mcp_children` in `_rebuild_skill_capabilities()` +- Phase 3: Restore `get_capabilities()` call without `exclude_global` + +Full rollback: revert all three phases in reverse order. No data migration is involved. + +--- + +## Open Questions + +1. **Should `SkillManagerCap.list_skills()` query the registry instead of `self._children`?** + - Context: After Phase 2, `SkillManagerCap` no longer has top-level McpServerCap in `self._children`. Remote skill listing must either query `registry.get_skill_resources(scope)` or accept that only local + per-skill skills are listed. + - Owner: pinjun.mo + - Status: Open — leaning toward registry query for consistency with `ResourceAccess` + +2. **Should `_setup_skills_provider()` (pool.py:610) also use the registry instead of `self.mcp.providers`?** + - Context: `SkillURIResolver.register_provider()` currently iterates `self.mcp.providers` directly. After Phase 2, these providers are in the registry, but the resolver doesn't query the registry. + - Owner: pinjun.mo + - Status: Open — may defer to keep the resolver's direct registration path + +3. **Does `get_capabilities()` need to handle the case where a top-level server is both in `self.providers` AND in session-scoped configs?** + - Context: An agent could override a pool-level MCP server with an agent-level config of the same name. Currently `get_capabilities()` handles this via snapshot partitioning. After the change, the pool-level McpServerCap is injected directly, and the agent-level config goes through `get_capabilities()`. + - Owner: yuchen.liu + - Status: Open — needs verification that no duplicate tools result from this scenario + +4. **Should `MCPManager.get_aggregating_provider()` be expanded to include non-ACP providers?** + - Context: Currently ACP-only. After this RFC, non-ACP providers are injected via `pool.mcp.providers` in `get_agentlet()`. The aggregating provider could be a single injection point for all providers, simplifying `get_agentlet()`. + - Owner: pinjun.mo + - Status: Open — deferred to a follow-up; current design separates ACP (Path C) from non-ACP (direct injection) for clarity + +5. **Should internal identity be separated from the model-visible tool name?** + - Context: Currently `McpServerCap._name` (manager-qualified id, e.g. `pool_mcp_xeno-kb`) serves as the internal identity AND is used in status keys (`get_server_status`), resource `source_uri` (`mcp://{name}`), and logging. The new `tool_prefix` (model-visible, e.g. `xeno-kb`) is separate. The industry pattern (per ChatGPT/OpenCode discussion) is to fully separate `internal_id` (`mcp_01:tool_17`) from `modelName` (`github_search`), so server renames/reconnects never break tool identity. Deferring — current config-driven scale has stable display_names, and `_name` is documented as identity. + - Owner: pinjun.mo + - Status: Open — deferred to a follow-up; revisit when dynamic server config (add/remove servers at runtime) is introduced + +6. **Should namespace use `.` (dot) instead of `_` (underscore) separators?** + - Context: ChatGPT's original suggestion used dot notation (`github.search`), but its own follow-up confirmed OpenCode uses underscore (`github_search`) and pydantic-ai's `PrefixedToolset` is underscore-native (`f'{prefix}_{name}'`). This RFC adopts underscore. A dot-based separator would require bypassing `PrefixedToolset` and custom-prefixing tool definitions. + - Owner: pinjun.mo + - Status: Resolved — underscore; aligns with OpenCode + PrefixedToolset. Documented here to record the deliberation. + +7. **Should top-level and skill-MCP tool prefix conventions be unified?** + - Context: this RFC introduces `{display_name}` as the tool prefix for top-level MCP servers (e.g. `xeno-kb_search_database`), while skill-level MCP keeps the RFC-0052-established `{skill_name}__mcp__` prefix (e.g. `python-expert__mcp__read_code`). Both solve collision avoidance, but the two conventions coexist. Options: (a) keep them separate (skill prefix carries the `__mcp__` transport hint, top-level prefix is a pure server namespace); (b) unify skill MCP to `{skill_name}_{server_name}` dropping `__mcp__`; (c) establish a general `{namespace}__{subtype}__` scheme for all MCP prefixes. This RFC deliberately keeps the existing `__mcp__` convention to minimize blast radius; unification is a naming-polish follow-up. + - Owner: pinjun.mo + - Status: Open — deferred to a follow-up; current dual convention is internally consistent (both prevent collision) and backward compatible + +--- + +## Decision Record + +> Complete this section after RFC review is concluded. + +### Decision + +**Status**: [PENDING REVIEW] + +**Date**: + +**Approvers**: +- [ ] + +### Decision Summary + +[TBD] + +### Key Discussion Points + +[TBD] + +### Conditions of Approval + +[TBD] + +### Dissenting Opinions + +[TBD] + +--- + +## References + +### Related Documents + +- [RFC-0051: Extension Source Architecture](RFC-0051-extension-source-architecture.md) — Original design for Resource Protocols and McpServerCap +- [RFC-0052: Restore Skill Capabilities](RFC-0052-restore-skill-capabilities.md) — SkillManagerCap children wiring that caused the ResourceAccess gap +- [MCP Resource Technical Report](../../specs/mcp-resource-technical-report.md) — Full MCP resource consumption architecture documentation + +### External Resources + +- [MCP Specification 2026-07-28 — Resources](https://modelcontextprotocol.io/specification/2026-07-28/server/resources) +- [RFC 3986 — Uniform Resource Identifier](https://www.rfc-editor.org/rfc/rfc3986) +- [RFC 6570 — URI Template](https://www.rfc-editor.org/rfc/rfc6570) + +### Appendix + +#### A. File-to-Change Mapping + +| File | Phase | Change | +|------|-------|--------| +| `src/wolfharness/capabilities/skill_manager_cap.py` | 1 | Add `ResourceAccess` delegation methods | +| `src/wolfharness/capabilities/skill_manager_cap.py` | 2 | Remove RFC-0052 D2 case-3 "non-skill children unprefixed" dead code | +| `src/wolfharness/capabilities/mcp_server_cap.py` | 3 | Wrap `get_toolset()` result in `PrefixedToolset` for tool namespacing | +| `src/wolfharness/delegation/pool.py` | 2 | Independent POOL-scope registration; remove `children=mcp_children` | +| `src/wolfharness/agents/native_agent/agent.py` | 3 | Replace `get_capabilities()` with direct McpServerCap injection | +| `src/wolfharness/mcp_server/manager.py` | 2 | De-duplicate `display_name` in `setup_server()` for unique tool prefixes | +| `src/wolfharness/mcp_server/manager.py` | 3 | Add `exclude_global` param to `get_capabilities()` | +| `tests/capabilities/test_skill_manager_cap.py` | 1-2 | Test ResourceAccess delegation; test without top-level children | +| `tests/delegation/test_pool.py` | 2 | Test independent POOL-scope registration | +| `tests/agents/test_native_agent.py` | 3 | Test tool exposure via McpServerCap; test no duplicate tools | +| `tests/mcp_server/test_manager_capability.py` | 3 | Test `exclude_global` parameter | +| `tests/servers/opencode_server/test_resource_resolution.py` | 2 | Test `@` mention with top-level MCP resources | From 2084aa23fbfb98d8da432cdc0329f2591968bbbe Mon Sep 17 00:00:00 2001 From: Million <15158090088@163.com> Date: Sat, 15 Aug 2026 15:11:07 +0800 Subject: [PATCH 05/11] feat(examples): add knowledge base MCP server with dynamic resources/list Demo server exposing kb_data/ files as kb:// static resources plus resource templates. resources/list is re-scanned in the background (--scan-interval) so files added or removed from kb_data appear in @ mention without a server restart (RFC-0058). Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus --- examples/kb_data/docs/architecture.md | 17 ++ examples/kb_data/docs/intro.md | 16 ++ examples/kb_data/docs/quickstart.md | 23 ++ examples/kb_data/docs/readme.md | 3 + examples/kb_data/images/diagram.png | Bin 0 -> 96 bytes examples/kb_data/images/logo.png | Bin 0 -> 96 bytes examples/kb_mcp_server_example.py | 379 ++++++++++++++++++++++++++ 7 files changed, 438 insertions(+) create mode 100644 examples/kb_data/docs/architecture.md create mode 100644 examples/kb_data/docs/intro.md create mode 100644 examples/kb_data/docs/quickstart.md create mode 100644 examples/kb_data/docs/readme.md create mode 100644 examples/kb_data/images/diagram.png create mode 100644 examples/kb_data/images/logo.png create mode 100644 examples/kb_mcp_server_example.py diff --git a/examples/kb_data/docs/architecture.md b/examples/kb_data/docs/architecture.md new file mode 100644 index 000000000..42b9ae7ce --- /dev/null +++ b/examples/kb_data/docs/architecture.md @@ -0,0 +1,17 @@ +# Architecture Overview + +AgentPool bridges multiple protocols with native PydanticAI agents. + +## Layers + +1. **Configuration** — YAML manifests parsed into Pydantic models +2. **Orchestration** — EventBus, SessionController, RunLoop +3. **Protocols** — ACP, OpenCode, MCP, AG-UI, OpenAI API +4. **Capabilities** — Tools, skills, MCP servers, resources + +## Message Flow + +``` +Client → Protocol Server → SessionController → Agent (PydanticAI) + → Tool Execution → Event Bus → Protocol Converter → Client +``` \ No newline at end of file diff --git a/examples/kb_data/docs/intro.md b/examples/kb_data/docs/intro.md new file mode 100644 index 000000000..ad2165691 --- /dev/null +++ b/examples/kb_data/docs/intro.md @@ -0,0 +1,16 @@ +# Introduction to AgentPool + +AgentPool is a unified agent orchestration framework for +YAML-based configuration of heterogeneous AI agents. + +## Core Philosophy + +Define once in YAML, expose through multiple protocols, +enable seamless inter-agent collaboration. + +## Key Features + +- Multi-agent orchestration (teams, chains) +- ACP, OpenCode, MCP, AG-UI protocol support +- YAML-based agent definition +- Structured output with Pydantic \ No newline at end of file diff --git a/examples/kb_data/docs/quickstart.md b/examples/kb_data/docs/quickstart.md new file mode 100644 index 000000000..46bbfe5a6 --- /dev/null +++ b/examples/kb_data/docs/quickstart.md @@ -0,0 +1,23 @@ +# Quick Start + +## Installation + +```bash +uv tool install wolfharness +``` + +## Minimal config + +```yaml +agents: + assistant: + type: native + model: openai:gpt-4o + system_prompt: "You are a helpful assistant." +``` + +## Run + +```bash +wolfharness run assistant "Hello!" +``` \ No newline at end of file diff --git a/examples/kb_data/docs/readme.md b/examples/kb_data/docs/readme.md new file mode 100644 index 000000000..67842209c --- /dev/null +++ b/examples/kb_data/docs/readme.md @@ -0,0 +1,3 @@ +# Docs subdirectory + +A nested markdown file to exercise the `file://{path}` template with a subdirectory. diff --git a/examples/kb_data/images/diagram.png b/examples/kb_data/images/diagram.png new file mode 100644 index 0000000000000000000000000000000000000000..700105f0c93a9b42fd7b1c9020d7d36da28070ea GIT binary patch literal 96 zcmeAS@N?(olHy`uVBq!ia0vp^3LwnE1SJ1Ryj={W)ID7sLn`LHJ!{Czz`(=2;Foxm tZ0N>hP6-8Du|@|40S*?XMu!io5(x)ROrMynzzWpD;OXk;vd$@?2>|ic7t8hP6-8Du|@|40S*?XMu!io5(x)ROrMynzzWpD;OXk;vd$@?2>|ic7t8 str: + """Return the MIME type for a file, falling back to ``application/octet-stream``.""" + return _mimetypes.guess_type(path.name)[0] or "application/octet-stream" + + +def _kbs_files() -> list[Path]: + """All files under kb_data/, grouped by namespace directory.""" + files: list[Path] = [] + for _dir in _NAMESPACE_DIRS.values(): + if _dir.is_dir(): + files.extend(p for p in _dir.rglob("*") if p.is_file()) + return sorted(files) + + +def _uri_for(path: Path) -> str: + """Map a file on disk to its kb:// resource URI. + + The namespace directory name becomes the URI host: ``kb_data/docs/intro.md`` + → ``kb://docs/intro.md``. + """ + rel = path.relative_to(KB_DIR).as_posix() # e.g. "docs/intro.md" + ns, _, name = rel.partition("/") + return f"kb://{ns}/{name}" + + +def _read_file(path: Path) -> str | bytes: + if path.suffix in {".md", ".txt", ".markdown"}: + return path.read_text(encoding="utf-8") + return path.read_bytes() + + +def _resolve_kb_uri(ns: str, name: str) -> Path: + """Resolve a kb:// namespace + name to a file, raising KeyError on issues.""" + base = _NAMESPACE_DIRS.get(ns) + if base is None: + raise KeyError(f"Unknown namespace: {ns!r}") + target = (base / name).resolve() + if not target.is_relative_to(base.resolve()): + raise KeyError(f"Path escapes namespace {ns!r}: {name!r}") + if not target.is_file(): + raise KeyError(f"Resource not found in kb://{ns}/{name}") + return target + + +# --------------------------------------------------------------------------- +# MCP Server +# --------------------------------------------------------------------------- + + +class _KBServer(FastMCP): + """FastMCP server that keeps ``resources/list`` in sync with ``kb_data/``. + + A background task re-scans the KB directory every ``scan_interval`` + seconds so newly added files appear in ``resources/list`` (hence in + ``@`` mention) without a server restart. + """ + + def __init__(self, scan_interval: float = 5.0, **kwargs: Any) -> None: + super().__init__(lifespan=Lifespan(_kb_lifespan(scan_interval)), **kwargs) + + +def _kb_lifespan(scan_interval: float): + async def _enter(server: FastMCP[Any]) -> AsyncIterator[dict[str, Any]]: + sync_static_resources() + task = asyncio.create_task(_scan_loop(scan_interval)) + try: + yield {} + finally: + task.cancel() + with contextlib.suppress(asyncio.CancelledError): + await task + + return _enter + + +async def _scan_loop(interval: float) -> None: + while True: + await asyncio.sleep(interval) + try: + sync_static_resources() + except Exception: + logger.warning("kb_data/ sync failed; will retry next scan", exc_info=True) + + +_registered_uris: set[str] = set() + + +def _remove_resource(uri: str) -> None: + """Remove a static resource from the underlying provider. + + Ignores ``KeyError`` (resource already gone). FastMCP has no public + ``remove_resource``; the provider layer does. + """ + for provider in getattr(mcp, "providers", []): + remove = getattr(provider, "remove_resource", None) + if remove is None: + continue + with contextlib.suppress(KeyError): + remove(uri) + return + + +def sync_static_resources() -> int: + """Sync the static resource list with the KB directory. + + Returns the number of resources now registered. Scans ``kb_data/`` and + adds/removes ``FileResource`` entries so ``resources/list`` reflects the + current directory contents without restarting the server. Deleted or + renamed files are removed; new files are added. + """ + current = {_uri_for(path) for path in _kbs_files()} + for uri in _registered_uris - current: + _remove_resource(uri) + _registered_uris.discard(uri) + for path in _kbs_files(): + uri = _uri_for(path) + if uri in _registered_uris: + continue + is_binary = path.suffix in {".png", ".jpg", ".jpeg"} + mcp.add_resource( + FileResource( + uri=uri, + path=path, + is_binary=is_binary, + mime_type=_mime_for(path), + title=f"{path.stem} ({path.parent.name})", + ) + ) + _registered_uris.add(uri) + return len(_registered_uris) + + +mcp = _KBServer( + name="kb-server", + instructions=( + "Knowledge Base MCP Server.\n\n" + "Serves files from the local `kb_data/` directory via the MCP Resource " + "protocol using the custom `kb://` URI scheme.\n" + "Use `list_resources` to discover available files, then `read_resource` " + "to fetch their contents.\n\n" + "Resource URIs:\n" + "- kb://docs/ — Markdown documents (e.g. kb://docs/intro.md)\n" + "- kb://images/ — PNG images (e.g. kb://images/logo.png)\n" + "- kb://search{?q} — search the KB by query string (template)" + ), +) + + +# --- Resource templates (dynamic URIs) --------------------------------------- + + +@mcp.resource("kb://docs/{name*}.md", mime_type="text/markdown") +def get_document(name: str) -> list[ResourceContent]: + """Read a Markdown document from the knowledge base. + + Args: + name: Document path under the docs namespace, without the ``.md`` suffix, + e.g. "intro", "sub/readme". + + Raises: + KeyError: If the document does not exist or resolves outside the docs + namespace. + """ + content = _read_file(_resolve_kb_uri("docs", f"{name}.md")) + return [ResourceContent(content, mime_type="text/markdown")] + + +@mcp.resource("kb://images/{name*}.png", mime_type="image/png") +def get_image(name: str) -> list[ResourceContent]: + """Read a PNG image from the knowledge base. + + Args: + name: Image path under the images namespace, without the ``.png`` suffix, + e.g. "logo", "diagram". + + Raises: + KeyError: If the image does not exist or resolves outside the images + namespace. + """ + content = _read_file(_resolve_kb_uri("images", f"{name}.png")) + return [ResourceContent(content, mime_type="image/png")] + + +@mcp.resource("kb://search{?q}") +def search_resource(q: str = "") -> str: + """Search the knowledge base, returning matches as a JSON snippet list. + + Args: + q: Search term to match against text documents. + + Returns: + The first matching document's content, or an empty string. + """ + if not q: + return "" + for path in _kbs_files(): + if path.suffix not in {".md", ".txt", ".markdown"}: + continue + content = path.read_text(encoding="utf-8") + if q.lower() in content.lower(): + return content + return "" + + +# --- Tools ------------------------------------------------------------------ + + +@mcp.tool +def search_kb(query: Annotated[str, Field(description="Search query string")]) -> str: + """Search the knowledge base for text files matching the query. + + Performs a simple case-insensitive substring search across all text + documents (``.md``, ``.txt``). Returns matching file paths and relevant + excerpts. + + Args: + query: The search term to look for. + + Returns: + JSON string with search results. + """ + query_lower = query.lower() + results: list[dict[str, Any]] = [] + for path in _kbs_files(): + if path.suffix not in {".md", ".txt", ".markdown"}: + continue + content = path.read_text(encoding="utf-8") + if query_lower not in content.lower(): + continue + idx = content.lower().find(query_lower) + start = max(0, idx - 40) + end = min(len(content), idx + len(query) + 40) + prefix = "..." if start > 0 else "" + suffix = "..." if end < len(content) else "" + results.append({ + "file": path.name, + "uri": _uri_for(path), + "snippet": f"{prefix}{content[start:end]}{suffix}", + }) + return json.dumps( + {"query": query, "match_count": len(results), "results": results}, + indent=2, + ) + + +@mcp.tool +def list_kb_files() -> str: + """List all files in the knowledge base. + + Returns a JSON string listing every file with its resource URI, MIME type, + and size in bytes. + """ + files = [ + { + "file": p.name, + "uri": _uri_for(p), + "mime_type": _mime_for(p), + "size": p.stat().st_size, + } + for p in _kbs_files() + ] + return json.dumps( + {"total_files": len(files), "files": files}, + indent=2, + ) + + +# --------------------------------------------------------------------------- +# Entrypoint +# --------------------------------------------------------------------------- + + +def main() -> None: + """Parse CLI args and start the MCP server.""" + parser = argparse.ArgumentParser(description="Knowledge Base MCP Server Demo") + parser.add_argument( + "--transport", + choices=["stdio", "sse", "streamable-http"], + default="stdio", + help="MCP transport type (default: stdio)", + ) + parser.add_argument("--host", default="localhost", help="Host for HTTP transports") + parser.add_argument("--port", type=int, default=8002, help="Port for HTTP transports") + parser.add_argument( + "--scan-interval", + type=float, + default=5.0, + help="Seconds between kb_data/ re-scans for dynamic resources/list (default: 5.0)", + ) + args = parser.parse_args() + + print(f"Starting KB MCP Server (transport={args.transport})") + print(f" KB directory: {KB_DIR}") + for path in _kbs_files(): + print(f" {_uri_for(path)} ({_mime_for(path)})") + print() + + if args.transport == "stdio": + mcp.run(transport="stdio") + else: + mcp.run(transport=args.transport, host=args.host, port=args.port) + + +if __name__ == "__main__": + main() From 7310ad8bf9a9c2937503f45692f257875eee4090 Mon Sep 17 00:00:00 2001 From: Million <15158090088@163.com> Date: Mon, 17 Aug 2026 11:25:56 +0800 Subject: [PATCH 06/11] fix(mcp): preserve dedicated-agent MCP servers when injecting top-level providers exclude_global=True skipped ALL of the agent's global MCP configs even when the agent owns a dedicated MCPManager (NativeAgentConfig with mcp_servers=[]). Such agents never have their servers surfaced via pool.mcp.providers, so the exclude dropped them entirely and MCP tools disappeared from the model (test_mcp_tool_with_progress KeyError). Only exclude when the agent shares the pool's MCPManager, whose providers are injected directly above. Fixes the Integration tests failure on #372. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus --- src/wolfharness/agents/native_agent/agent.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/wolfharness/agents/native_agent/agent.py b/src/wolfharness/agents/native_agent/agent.py index cb19a4b4f..2e73c6eda 100644 --- a/src/wolfharness/agents/native_agent/agent.py +++ b/src/wolfharness/agents/native_agent/agent.py @@ -1131,9 +1131,17 @@ async def get_agentlet[AgentOutputType]( # noqa: PLR0915 tool_capabilities.extend( provider for provider in pool.mcp.providers if isinstance(provider, McpServerCap) ) + # Top-level providers are injected directly above. When the agent + # shares the pool's MCPManager (no agent-level MCP servers), its + # global configs are already covered by ``pool.mcp.providers`` and + # must not be re-added via an MCP capability. But an agent with its + # own dedicated MCPManager owns its servers exclusively — those + # global configs are NOT in ``pool.mcp.providers`` and must still be + # processed (RFC-0058 exclude path only dedups the pool-shared case). + shares_pool_mcp = pool is not None and self.mcp is pool.mcp mcp_capabilities = await self.mcp.get_capabilities( session_id=run_ctx.session_id if run_ctx else None, - exclude_global=True, + exclude_global=shares_pool_mcp, ) tool_capabilities.extend(mcp_capabilities) # 5. Skill capabilities — from pool-scoped instances created during __aenter__. From 82f91ca183bfc58939a8b150c5cbfea6d3958c0a Mon Sep 17 00:00:00 2001 From: Million <15158090088@163.com> Date: Mon, 17 Aug 2026 11:26:03 +0800 Subject: [PATCH 07/11] feat(mcp): dynamic notification callbacks and explicit tool_prefix semantics MCPClient gains a public set_notification_callbacks() API replacing the private attribute writes in McpServerCap.\_ensure_client. The message handler reads callbacks dynamically from the client on each notification, so a handler created before callbacks are bound still observes them (\_rebind_session_message_handler after transport connect). McpServerCap.tool_prefix now defaults to None instead of falling back to config.display_name. The manager already passes an explicit prefix for POOL-scope servers, and skill-level MCP wraps its children itself; falling back to display_name silently prefixed dedicated-agent servers and broke their raw MCP tool names. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus --- .../capabilities/mcp_server_cap.py | 21 +++-- src/wolfharness/mcp_server/client.py | 78 +++++++++++++++---- src/wolfharness/mcp_server/message_handler.py | 31 +++++--- 3 files changed, 98 insertions(+), 32 deletions(-) diff --git a/src/wolfharness/capabilities/mcp_server_cap.py b/src/wolfharness/capabilities/mcp_server_cap.py index 0df015a79..be746b437 100644 --- a/src/wolfharness/capabilities/mcp_server_cap.py +++ b/src/wolfharness/capabilities/mcp_server_cap.py @@ -102,14 +102,17 @@ def __init__( name: Optional name override. Defaults to ``config.client_id``. client: Optional pre-created ``MCPClient``. When provided, bypasses the session pool and uses this client directly. - tool_prefix: Optional model-visible tool namespace derived from - the server's ``display_name``. When ``None``, falls back to - ``config.display_name``. + tool_prefix: Optional model-visible tool namespace for MCP tools. + Passed by ``MCPManager`` for POOL-scope servers so prefixed tool + names never collide across servers sharing a ``display_name`` + (RFC-0058). When ``None`` (direct-construction paths such as + ``NativeAgentConfig(mcp_servers=[...])``), tools keep their raw + MCP names for backward compatibility. """ self._config = config self._session_pool = session_pool self._name = name or config.client_id - self._tool_prefix = tool_prefix or config.display_name + self._tool_prefix = tool_prefix self._client: MCPClient | None = client self._change_queues: set[asyncio.Queue[ChangeEvent]] = set() @@ -220,10 +223,12 @@ async def _on_prompts_changed() -> None: for q in list(self._change_queues): await q.put(event) - client._tool_change_callback = _on_tools_changed - client._resource_list_changed_callback = _on_resource_list_changed - client._resource_updated_callback = _on_resource_updated - client._prompt_change_callback = _on_prompts_changed + client.set_notification_callbacks( + tool_change_callback=_on_tools_changed, + resource_list_changed_callback=_on_resource_list_changed, + resource_updated_callback=_on_resource_updated, + prompt_change_callback=_on_prompts_changed, + ) self._client = client return client diff --git a/src/wolfharness/mcp_server/client.py b/src/wolfharness/mcp_server/client.py index 34582b639..097ca04a3 100644 --- a/src/wolfharness/mcp_server/client.py +++ b/src/wolfharness/mcp_server/client.py @@ -91,6 +91,8 @@ def __init__( self._sampling_callback = sampling_callback # Store message handler or mark for lazy creation self._message_handler = message_handler + # Lazily-created wolfharness message handler (see _get_message_handler). + self._wolfharness_message_handler: MCPMessageHandler | None = None self._accessible_roots = accessible_roots or [] self._tool_change_callback = tool_change_callback self._prompt_change_callback = prompt_change_callback @@ -114,6 +116,45 @@ def connected(self) -> bool: """Check if client is connected by examining session state.""" return self._client.is_connected() + def set_notification_callbacks( + self, + *, + tool_change_callback: Callable[[], Awaitable[None]] | None = None, + prompt_change_callback: Callable[[], Awaitable[None]] | None = None, + resource_list_changed_callback: Callable[[], Awaitable[None]] | None = None, + resource_updated_callback: Callable[[str], Awaitable[None]] | None = None, + ) -> None: + """Set server-notification callbacks after the client is connected. + + The ``MCPMessageHandler`` reads these callbacks dynamically from the + client on each notification (rather than snapshotting them at + construction), so callbacks set here apply immediately even if the + message handler was already created. + """ + self._tool_change_callback = tool_change_callback + self._prompt_change_callback = prompt_change_callback + self._resource_list_changed_callback = resource_list_changed_callback + self._resource_updated_callback = resource_updated_callback + + def _get_message_handler(self) -> MessageHandlerT | MessageHandler: + """Return the wolfharness message handler for this client. + + The handler reads notification callbacks dynamically from the client + (see ``set_notification_callbacks``), so it can be created any time + after construction and still see the latest callbacks. + """ + if self._message_handler is not None: + return self._message_handler + if self._wolfharness_message_handler is None: + self._wolfharness_message_handler = MCPMessageHandler( + self, + self._tool_change_callback, + self._prompt_change_callback, + self._resource_list_changed_callback, + self._resource_updated_callback, + ) + return self._wolfharness_message_handler + @property def server_info(self) -> dict[str, str] | None: """Get server info (name and version) from the connected client. @@ -184,6 +225,11 @@ async def __aenter__(self) -> Self: else: raise + # When a shared transport (e.g. SessionConnectionPool's stdio + # owner-task) pre-connects before this MCPClient exists, fastmcp + # reuses that session and our MCPMessageHandler is never bound. + # Rebind so server notifications reach wolfharness callbacks. + self._rebind_session_message_handler() return self async def __aexit__(self, *args: object) -> None: @@ -193,6 +239,22 @@ async def __aexit__(self, *args: object) -> None: except Exception as e: # noqa: BLE001 logger.warning("Error during FastMCP client cleanup", error=e) + def _rebind_session_message_handler(self) -> None: + """Rebind the underlying session's message handler to wolfharness's. + + ``SessionConnectionPool`` pre-connects stdio transports inside an + owner task *before* this ``MCPClient`` exists, so fastmcp reuses + that session and our ``MCPMessageHandler`` would otherwise never be + bound. mcp SDK sessions read ``_message_handler`` dynamically for + each notification, so setting it here takes effect immediately. + """ + if not self._client.is_connected(): + return + session = self._client.session + handler = self._get_message_handler() + if getattr(session, "_message_handler", None) is not handler: + session._message_handler = handler # type: ignore[attr-defined] + def get_resource_fs(self) -> MCPFileSystem: """Get a filesystem for accessing MCP resources.""" from upathtools.filesystems import MCPFileSystem @@ -283,13 +345,7 @@ def _get_client( oauth = config.auth.oauth # Create message handler if needed - msg_handler = self._message_handler or MCPMessageHandler( - self, - self._tool_change_callback, - self._prompt_change_callback, - self._resource_list_changed_callback, - self._resource_updated_callback, - ) + msg_handler = self._get_message_handler() # Build client_info if client_name is provided client_info: Implementation | None = None @@ -326,13 +382,7 @@ def _get_client_from_transport(self, transport: ClientTransport) -> fastmcp.Clie import fastmcp from mcp.types import Icon, Implementation - msg_handler = self._message_handler or MCPMessageHandler( - self, - self._tool_change_callback, - self._prompt_change_callback, - self._resource_list_changed_callback, - self._resource_updated_callback, - ) + msg_handler = self._get_message_handler() client_info: Implementation | None = None if self._client_name: diff --git a/src/wolfharness/mcp_server/message_handler.py b/src/wolfharness/mcp_server/message_handler.py index 7b085caca..0385363ed 100644 --- a/src/wolfharness/mcp_server/message_handler.py +++ b/src/wolfharness/mcp_server/message_handler.py @@ -102,24 +102,33 @@ async def on_notification(self, message: mcp.types.ServerNotification) -> None: async def on_tool_list_changed(self, message: mcp.types.ToolListChangedNotification) -> None: """Handle tool list changes.""" logger.info("MCP tool list changed", message=message) - # Call the tool change callback if provided - if self.tool_change_callback: - await self.tool_change_callback() + # Prefer the construction-time field; fall back to reading the callback + # dynamically so a client's callback can be swapped after the message + # handler was created (see MCPClient.set_notification_callbacks). + callback = self.tool_change_callback or getattr(self.client, "_tool_change_callback", None) + if callback: + await callback() async def on_resource_list_changed( self, message: mcp.types.ResourceListChangedNotification ) -> None: """Handle resource list changes.""" logger.info("MCP resource list changed", message=message) - if self.resource_list_changed_callback: - await self.resource_list_changed_callback() + callback = self.resource_list_changed_callback or getattr( + self.client, "_resource_list_changed_callback", None + ) + if callback: + await callback() async def on_resource_updated(self, message: mcp.types.ResourceUpdatedNotification) -> None: """Handle resource content updates.""" uri = str(message.params.uri) logger.info("MCP resource updated", uri=uri) - if self.resource_updated_callback: - await self.resource_updated_callback(uri) + callback = self.resource_updated_callback or getattr( + self.client, "_resource_updated_callback", None + ) + if callback: + await callback(uri) async def on_progress(self, message: mcp.types.ProgressNotification) -> None: """Handle progress notifications with proper context.""" @@ -131,9 +140,11 @@ async def on_prompt_list_changed( ) -> None: """Handle prompt list changes.""" logger.info("MCP prompt list changed", message=message) - # Call the prompt change callback if provided - if self.prompt_change_callback: - await self.prompt_change_callback() + callback = self.prompt_change_callback or getattr( + self.client, "_prompt_change_callback", None + ) + if callback: + await callback() async def on_cancelled(self, message: mcp.types.CancelledNotification) -> None: """Handle cancelled operations.""" From f843237d3a2d42ced85397567142a393c3f0ca34 Mon Sep 17 00:00:00 2001 From: Million <15158090088@163.com> Date: Mon, 17 Aug 2026 11:26:09 +0800 Subject: [PATCH 08/11] test(mcp): sync test doubles with set_notification_callbacks API FakeMCPClient in test_mcp_server_cap and test_review_fixes gains the public set_notification_callbacks() method and missing callback fields so McpServerCap._ensure_client can bind through the public API. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus --- tests/capabilities/test_mcp_server_cap.py | 19 +++++++++++++++++-- tests/capabilities/test_review_fixes.py | 17 +++++++++++++++++ 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/tests/capabilities/test_mcp_server_cap.py b/tests/capabilities/test_mcp_server_cap.py index be68b7295..f4a2d5b35 100644 --- a/tests/capabilities/test_mcp_server_cap.py +++ b/tests/capabilities/test_mcp_server_cap.py @@ -14,6 +14,7 @@ import asyncio from dataclasses import dataclass, field +from types import SimpleNamespace from typing import TYPE_CHECKING, Any from unittest.mock import AsyncMock, MagicMock @@ -116,6 +117,20 @@ async def call_tool(self, name: str, *args: Any, **kwargs: Any) -> str: def convert_tool(self, tool: Any) -> Any: return tool + def set_notification_callbacks( + self, + *, + tool_change_callback: Any = None, + prompt_change_callback: Any = None, + resource_list_changed_callback: Any = None, + resource_updated_callback: Any = None, + ) -> None: + """Mirror MCPClient.set_notification_callbacks.""" + self._tool_change_callback = tool_change_callback + self._prompt_change_callback = prompt_change_callback + self._resource_list_changed_callback = resource_list_changed_callback + self._resource_updated_callback = resource_updated_callback + async def trigger_tool_change(self) -> None: """Simulate MCP server sending notifications/tools/list_changed.""" if self._tool_change_callback is not None: @@ -792,7 +807,7 @@ async def callback(uri: str) -> None: @pytest.mark.anyio async def test_message_handler_resource_list_changed_no_callback() -> None: """on_resource_list_changed does not raise when callback is None.""" - handler = MCPMessageHandler(client=MagicMock()) + handler = MCPMessageHandler(client=SimpleNamespace(_resource_list_changed_callback=None)) notification = mcp.types.ResourceListChangedNotification() await handler.on_resource_list_changed(notification) @@ -801,7 +816,7 @@ async def test_message_handler_resource_list_changed_no_callback() -> None: @pytest.mark.anyio async def test_message_handler_resource_updated_no_callback() -> None: """on_resource_updated does not raise when callback is None.""" - handler = MCPMessageHandler(client=MagicMock()) + handler = MCPMessageHandler(client=SimpleNamespace(_resource_updated_callback=None)) notification = mcp.types.ResourceUpdatedNotification( params=mcp.types.ResourceUpdatedNotificationParams( diff --git a/tests/capabilities/test_review_fixes.py b/tests/capabilities/test_review_fixes.py index 7b3707eea..b2a7ee454 100644 --- a/tests/capabilities/test_review_fixes.py +++ b/tests/capabilities/test_review_fixes.py @@ -41,6 +41,9 @@ class FakeMCPClient: _read_results: dict[str, list[Any]] = field(default_factory=dict) _connected: bool = False _tool_change_callback: Any = None + _prompt_change_callback: Any = None + _resource_list_changed_callback: Any = None + _resource_updated_callback: Any = None _exited: bool = False config: Any = None @@ -75,6 +78,20 @@ async def call_tool(self, name: str, *args: Any, **kwargs: Any) -> str: def convert_tool(self, tool: Any) -> Any: return tool + def set_notification_callbacks( + self, + *, + tool_change_callback: Any = None, + prompt_change_callback: Any = None, + resource_list_changed_callback: Any = None, + resource_updated_callback: Any = None, + ) -> None: + """Mirror MCPClient.set_notification_callbacks.""" + self._tool_change_callback = tool_change_callback + self._prompt_change_callback = prompt_change_callback + self._resource_list_changed_callback = resource_list_changed_callback + self._resource_updated_callback = resource_updated_callback + class FakeSessionPool: """Fake SessionConnectionPool that returns a FakeMCPClient.""" From 0679f222268c4d0014a4832b31070524aa9fc0b9 Mon Sep 17 00:00:00 2001 From: Million <15158090088@163.com> Date: Mon, 17 Aug 2026 11:26:16 +0800 Subject: [PATCH 09/11] feat(examples): broadcast resources/list_changed from kb MCP server Capture connected sessions via a FastMCP middleware instead of a manual caller_register tool. A background scan loop detects kb_data changes and broadcasts notifications/resources/list_changed to every active session, so @-mention resource listings stay fresh without a server restart. FileResource entries carry annotations.lastModified; add a kb://docs/ directory listing resource and name/describe the resource templates. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus --- examples/kb_mcp_server_example.py | 171 ++++++++++++++++++++++++++---- 1 file changed, 152 insertions(+), 19 deletions(-) diff --git a/examples/kb_mcp_server_example.py b/examples/kb_mcp_server_example.py index 6f31c1c02..a3737d680 100644 --- a/examples/kb_mcp_server_example.py +++ b/examples/kb_mcp_server_example.py @@ -36,6 +36,7 @@ import argparse import asyncio import contextlib +from datetime import UTC, datetime import json import logging import mimetypes @@ -46,6 +47,8 @@ from fastmcp.resources import FileResource from fastmcp.resources.base import ResourceContent from fastmcp.server.lifespan import Lifespan +from fastmcp.server.middleware import CallNext, Middleware, MiddlewareContext +import mcp.types as mcp_types from pydantic import Field @@ -55,15 +58,26 @@ logger = logging.getLogger("kb-mcp-demo") -KB_DIR = Path(__file__).parent / "kb_data" +# Root of the knowledge base on disk. Defaults to ``examples/kb_data/``; the +# ``--kb-dir`` CLI flag overrides it, which lets tests point the server at a +# temporary directory. Kept mutable (not ``Final``) so tests can also re-point +# it before registering resources. +KB_DIR: Path = Path(__file__).parent / "kb_data" -# Namespace → filesystem directory. The ``kb://`` scheme uses the URI host as a -# namespace: ``kb://docs/intro.md`` reads ``kb_data/docs/intro.md`` (or, when -# the directory mirrors the host, ``kb_data/docs/intro.md``). -_NAMESPACE_DIRS = { - "docs": KB_DIR / "docs", - "images": KB_DIR / "images", -} + +def _namespace_dirs(kb_dir: Path = KB_DIR) -> dict[str, Path]: + """Map URI hosts (``kb://docs``, ``kb://images``) to filesystem dirs. + + The ``kb://`` scheme uses the URI host as a namespace: ``kb://docs/intro.md`` + reads ``/docs/intro.md``. + """ + return { + "docs": kb_dir / "docs", + "images": kb_dir / "images", + } + + +_NAMESPACE_DIRS = _namespace_dirs() # MIME types that ``mimetypes`` does not resolve from a bare filename. _MIME_BY_SUFFIX = {".md": "text/markdown"} @@ -126,11 +140,41 @@ class _KBServer(FastMCP): A background task re-scans the KB directory every ``scan_interval`` seconds so newly added files appear in ``resources/list`` (hence in - ``@`` mention) without a server restart. + ``@`` mention) without a server restart. When the list changes, a + ``notifications/resources/list_changed`` broadcast goes to every client + session the server has seen (see ``_SessionRegistryMiddleware``). """ def __init__(self, scan_interval: float = 5.0, **kwargs: Any) -> None: super().__init__(lifespan=Lifespan(_kb_lifespan(scan_interval)), **kwargs) + # Auto-register every client session that makes a request so the + # scan loop can broadcast list_changed without client cooperation. + self.add_middleware(_SessionRegistryMiddleware()) + + +class _SessionRegistryMiddleware(Middleware): + """Register every client session with the live-session registry. + + FastMCP 3.4.4 has no broadcast API; notifications are per-session. This + middleware captures each client's session on inbound requests, so the + background scan loop can reach all connected clients without requiring + them to call a registration tool first. + """ + + async def on_message( + self, + context: MiddlewareContext[Any], + call_next: CallNext[Any, Any], + ) -> Any: + ctx = context.fastmcp_context + if ctx is not None: + try: + session = ctx.session + except RuntimeError: + session = None + if session is not None: + _active_sessions.add(session) + return await call_next(context) def _kb_lifespan(scan_interval: float): @@ -151,13 +195,27 @@ async def _scan_loop(interval: float) -> None: while True: await asyncio.sleep(interval) try: - sync_static_resources() + if sync_static_resources(): + await _broadcast_resources_list_changed() except Exception: logger.warning("kb_data/ sync failed; will retry next scan", exc_info=True) _registered_uris: set[str] = set() +# --- Dynamic change notification (best-practice demo) ------------------------ +# +# FastMCP (3.4.4) has no FastMCP-level broadcast API: notifications are +# per-session, sent through ``Context.send_notification()``. Since the +# background scan loop created inside the lifespan only holds the ``FastMCP`` +# server (not any session), we keep a registry of live client sessions here. +# ``_SessionRegistryMiddleware`` fills it automatically on every inbound +# request; the scan loop iterates it when ``resources/list`` changes and +# pushes ``notifications/resources/list_changed`` to each registered client. + +_active_sessions: set[Any] = set() +_session_lock = asyncio.Lock() + def _remove_resource(uri: str) -> None: """Remove a static resource from the underlying provider. @@ -174,18 +232,42 @@ def _remove_resource(uri: str) -> None: return -def sync_static_resources() -> int: +async def _broadcast_resources_list_changed() -> None: + """Push ``notifications/resources/list_changed`` to registered clients. + + Iterates the registry of live sessions (filled automatically by + ``_SessionRegistryMiddleware``) and sends the notification to each + still-connected one, dropping dead sessions as they are found. + """ + async with _session_lock: + snapshot = list(_active_sessions) + for session in snapshot: + try: + await session.send_notification( + mcp_types.ServerNotification(mcp_types.ResourceListChangedNotification()) + ) + except (ConnectionError, OSError, ValueError): + # Client disconnected mid-broadcast: drop it so the next scan + # doesn't try again. This is deliberate best-effort broadcasting. + async with _session_lock: + _active_sessions.discard(session) + + +def sync_static_resources() -> bool: """Sync the static resource list with the KB directory. - Returns the number of resources now registered. Scans ``kb_data/`` and - adds/removes ``FileResource`` entries so ``resources/list`` reflects the - current directory contents without restarting the server. Deleted or - renamed files are removed; new files are added. + Returns ``True`` if the resource set changed (added or removed). Scans + ``kb_data/`` and adds/removes ``FileResource`` entries so + ``resources/list`` reflects the current directory contents without + restarting the server. Deleted or renamed files are removed; new files + are added. """ + changed = False current = {_uri_for(path) for path in _kbs_files()} for uri in _registered_uris - current: _remove_resource(uri) _registered_uris.discard(uri) + changed = True for path in _kbs_files(): uri = _uri_for(path) if uri in _registered_uris: @@ -198,10 +280,14 @@ def sync_static_resources() -> int: is_binary=is_binary, mime_type=_mime_for(path), title=f"{path.stem} ({path.parent.name})", + annotations=mcp_types.Annotations( + lastModified=datetime.fromtimestamp(path.stat().st_mtime, UTC).isoformat() + ), ) ) _registered_uris.add(uri) - return len(_registered_uris) + changed = True + return changed mcp = _KBServer( @@ -223,7 +309,12 @@ def sync_static_resources() -> int: # --- Resource templates (dynamic URIs) --------------------------------------- -@mcp.resource("kb://docs/{name*}.md", mime_type="text/markdown") +@mcp.resource( + "kb://docs/{name*}.md", + name="Document", + description="A Markdown document in the docs namespace", + mime_type="text/markdown", +) def get_document(name: str) -> list[ResourceContent]: """Read a Markdown document from the knowledge base. @@ -239,7 +330,12 @@ def get_document(name: str) -> list[ResourceContent]: return [ResourceContent(content, mime_type="text/markdown")] -@mcp.resource("kb://images/{name*}.png", mime_type="image/png") +@mcp.resource( + "kb://images/{name*}.png", + name="Image", + description="A PNG image in the images namespace", + mime_type="image/png", +) def get_image(name: str) -> list[ResourceContent]: """Read a PNG image from the knowledge base. @@ -255,7 +351,33 @@ def get_image(name: str) -> list[ResourceContent]: return [ResourceContent(content, mime_type="image/png")] -@mcp.resource("kb://search{?q}") +@mcp.resource( + "kb://docs/", + name="Docs index", + description="JSON listing of all documents in the docs namespace", + mime_type="application/json", +) +def list_docs() -> str: + """Return a JSON listing of every document under ``kb://docs/``. + + Directory reads return a single JSON listing content (not one content per + file), because the MCP spec intends each ``contents[].uri`` to identify a + concrete resource. Clients then read each file through the + ``kb://docs/{name*}.md`` template. + """ + files = [ + {"uri": _uri_for(p), "name": p.name, "size": p.stat().st_size} + for p in _kbs_files() + if p.parent == _NAMESPACE_DIRS["docs"] + ] + return json.dumps({"namespace": "docs", "files": files}, indent=2) + + +@mcp.resource( + "kb://search{?q}", + name="Search", + description="Search the knowledge base by query string", +) def search_resource(q: str = "") -> str: """Search the knowledge base, returning matches as a JSON snippet list. @@ -361,8 +483,19 @@ def main() -> None: default=5.0, help="Seconds between kb_data/ re-scans for dynamic resources/list (default: 5.0)", ) + parser.add_argument( + "--kb-dir", + type=Path, + default=None, + help="Knowledge base directory (defaults to examples/kb_data)", + ) args = parser.parse_args() + if args.kb_dir is not None: + global KB_DIR, _NAMESPACE_DIRS # noqa: PLW0603 + KB_DIR = args.kb_dir + _NAMESPACE_DIRS = _namespace_dirs(KB_DIR) + print(f"Starting KB MCP Server (transport={args.transport})") print(f" KB directory: {KB_DIR}") for path in _kbs_files(): From e65cd4b168fc2fb5f8c3125b454568a4eb39a0c3 Mon Sep 17 00:00:00 2001 From: Million <15158090088@163.com> Date: Mon, 17 Aug 2026 11:26:22 +0800 Subject: [PATCH 10/11] test(e2e): verify kb server list_changed reaches McpServerCap Spawn the kb MCP server as a real subprocess against a temp kb-dir, add a file, and assert the McpServerCap receives a ChangeEvent(kind=resource_list_changed). Exercises the full chain: server broadcast -> MCP transport -> MCPMessageHandler -> McpServerCap. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus --- tests/e2e/test_kb_mcp_server_list_changed.py | 81 ++++++++++++++++++++ 1 file changed, 81 insertions(+) create mode 100644 tests/e2e/test_kb_mcp_server_list_changed.py diff --git a/tests/e2e/test_kb_mcp_server_list_changed.py b/tests/e2e/test_kb_mcp_server_list_changed.py new file mode 100644 index 000000000..650846954 --- /dev/null +++ b/tests/e2e/test_kb_mcp_server_list_changed.py @@ -0,0 +1,81 @@ +"""E2E: kb_mcp_server_example.py emits resources/list_changed -> McpServerCap ChangeEvent. + +L4 subprocess test (``@pytest.mark.e2e``): spins up the real example MCP server +as a subprocess pointed at a temp ``--kb-dir``, connects to it through +``McpServerCap`` (via ``SessionConnectionPool``), then adds a file on disk and +asserts the capability surfaces a ``ChangeEvent(kind="resource_list_changed")``. + +This proves the full best-practice chain works: +server scan loop -> notifications/resources/list_changed -> MCPMessageHandler -> +MCPClient.set_notification_callbacks -> McpServerCap.on_change(). +""" + +from __future__ import annotations + +import asyncio +from pathlib import Path +import sys + +import pytest + +from wolfharness.capabilities.mcp_server_cap import McpServerCap +from wolfharness.mcp_server.session_pool import SessionConnectionPool +from wolfharness_config.mcp_server import StdioMCPServerConfig + + +pytestmark = pytest.mark.e2e + +_SERVER_PATH = ( + Path(__file__).parent / ".." / ".." / "examples" / "kb_mcp_server_example.py" +).resolve() +_SCAN_INTERVAL = 1.0 + + +@pytest.fixture +def kb_dir(tmp_path: Path) -> Path: + """A temp knowledge base seeded with one doc and one image namespace.""" + (tmp_path / "docs").mkdir() + (tmp_path / "images").mkdir() + (tmp_path / "docs" / "intro.md").write_text("# Intro\nhello kb", encoding="utf-8") + return tmp_path + + +def _server_config(kb_dir: Path) -> StdioMCPServerConfig: + return StdioMCPServerConfig( + name="kb-server", + command=sys.executable, + args=[ + str(_SERVER_PATH), + "--kb-dir", + str(kb_dir), + "--scan-interval", + str(_SCAN_INTERVAL), + ], + ) + + +@pytest.mark.asyncio +async def test_list_changed_event_flows_to_mcp_server_cap(kb_dir: Path) -> None: + """Adding a file to kb_data/ yields resource_list_changed on McpServerCap.""" + pool = SessionConnectionPool(session_id="kb-list-changed-test") + cap = McpServerCap(config=_server_config(kb_dir), name="kb-server", session_pool=pool) + + # Trigger lazy connection so the notification callbacks are wired. + resources = await cap.list_resources() + assert any(str(r.uri).startswith("kb://docs/") for r in resources) + + stream = cap.on_change() + assert stream is not None + + try: + # Add a file: the server's scan loop detects it, broadcasts + # notifications/resources/list_changed, and the capability emits a + # ChangeEvent. + (kb_dir / "docs" / "new-doc.md").write_text("# New doc", encoding="utf-8") + event = await asyncio.wait_for(stream.__anext__(), timeout=20.0) + finally: + await stream.aclose() + + assert event.kind == "resource_list_changed" + assert event.capability_name == "kb-server" + assert event.source_uri == "mcp://kb-server" From 1e177477e9f2dafa8982e340ae8d776c46750e5d Mon Sep 17 00:00:00 2001 From: Million <15158090088@163.com> Date: Mon, 17 Aug 2026 11:39:23 +0800 Subject: [PATCH 11/11] fix(mcp): align tool_prefix return type and drop stale type ignore McpServerCap.tool_prefix is str | None now that tool_prefix no longer falls back to display_name. The attr-defined ignore on ClientSession. _message_handler became unused after the mcp SDK gained the attribute. Fixes remaining mypy failures on #372. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus --- src/wolfharness/capabilities/mcp_server_cap.py | 2 +- src/wolfharness/mcp_server/client.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/wolfharness/capabilities/mcp_server_cap.py b/src/wolfharness/capabilities/mcp_server_cap.py index be746b437..f25f33aff 100644 --- a/src/wolfharness/capabilities/mcp_server_cap.py +++ b/src/wolfharness/capabilities/mcp_server_cap.py @@ -124,7 +124,7 @@ def name(self) -> str: return self._name @property - def tool_prefix(self) -> str: + def tool_prefix(self) -> str | None: """Return the model-visible tool namespace for this server.""" return self._tool_prefix diff --git a/src/wolfharness/mcp_server/client.py b/src/wolfharness/mcp_server/client.py index 097ca04a3..5820f7d48 100644 --- a/src/wolfharness/mcp_server/client.py +++ b/src/wolfharness/mcp_server/client.py @@ -253,7 +253,7 @@ def _rebind_session_message_handler(self) -> None: session = self._client.session handler = self._get_message_handler() if getattr(session, "_message_handler", None) is not handler: - session._message_handler = handler # type: ignore[attr-defined] + session._message_handler = handler def get_resource_fs(self) -> MCPFileSystem: """Get a filesystem for accessing MCP resources."""