fix: post-merge review fixes for PR #384 (tool-schema-overlap)

.github/workflows/opencode-review.yml

@@ -69,9 +69,11 @@ jobs:
         uses: anomalyco/opencode/github@latest
         env:
           DEEPSEEK_API_KEY: ${{ secrets.DEEPSEEK_API_KEY }}
+          GITHUB_TOKEN: ${{ github.token }}
         with:
           agent: review-lead
           model: deepseek/deepseek-v4-flash
+          use_github_token: true
           prompt: |
             Review this pull request. Read the AGENTS.md file first for project
             conventions, then analyze the diff and delegate to your specialist

changelog/unreleased/2026-08-21-pr384-review-fixes.md

@@ -0,0 +1,18 @@
+# Post-merge review fixes for PR #384 (tool-schema-overlap capability)
+
+Fixes five issues identified during code review of the merged tool-schema-overlap
+capability PR:
+
+- **P0 — Resource leak**: `BaseAgent.__aexit__` now calls `__aexit__` on
+  `_LifecycleCapable` instances in `_external_capabilities`, closing direct
+  `MCPClient` connections created by the `_ensure_client` fallback that were
+  previously never cleaned up.
+- **P1 — Internal URL**: Replaced the hardcoded internal Sany URL in
+  `examples/kb_diag_agent.yaml` with `https://mcp.example.com/…`.
+- **P1 — Unhandled exception**: `_get_mcp_server_info` now catches and logs
+  errors from `cap.list_tools()` on config-defined MCP capabilities instead
+  of crashing the entire status endpoint when a client disconnects.
+- **P2 — Enum type limitation**: `ParamOverride.enum` relaxed from
+  `list[str]` to `list[Any]` so integer/number enums are configurable.
+- **P3 — Code duplication**: Added "intentional duplication" comments to
+  `_AliasLoader` in all 10 `agentpool*` shim modules.

examples/kb_diag_agent.yaml

@@ -18,7 +18,7 @@ agents:
           config:
             type: streamable-http
             name: knowledge_diag
-            url: https://sprouts-dev-app-frontend.sany.com.cn/api-mcp/knowledge_diag/mcp
+            url: https://mcp.example.com/knowledge_diag/mcp
             # 去掉 search_kb 接口：若仍需保留该工具，删除下一行即可
             disabled_tools:
               - search_kb

src/agentpool/__init__.py

@@ -34,6 +34,11 @@ def __getattr__(name: str) -> object:
     return getattr(wolfharness, name)
 
 
+# NOTE: _AliasLoader is intentionally duplicated across all agentpool* shim
+# modules. These shims redirect to wolfharness* and must work before
+# wolfharness is imported, so the loader cannot be shared from a common
+# utility module. This duplication will be removed when the deprecated
+# shims are dropped.
 class _AliasLoader(abc.Loader):
     """Loader that returns an already-imported target module as-is.
 

src/agentpool_bot/__init__.py

@@ -34,6 +34,11 @@ def __getattr__(name: str) -> object:
     return getattr(wolfharness_bot, name)
 
 
+# NOTE: _AliasLoader is intentionally duplicated across all agentpool* shim
+# modules. These shims redirect to wolfharness* and must work before
+# wolfharness is imported, so the loader cannot be shared from a common
+# utility module. This duplication will be removed when the deprecated
+# shims are dropped.
 class _AliasLoader(abc.Loader):
     """Loader that returns an already-imported target module as-is.
 

src/agentpool_cli/__init__.py

@@ -34,6 +34,11 @@ def __getattr__(name: str) -> object:
     return getattr(wolfharness_cli, name)
 
 
+# NOTE: _AliasLoader is intentionally duplicated across all agentpool* shim
+# modules. These shims redirect to wolfharness* and must work before
+# wolfharness is imported, so the loader cannot be shared from a common
+# utility module. This duplication will be removed when the deprecated
+# shims are dropped.
 class _AliasLoader(abc.Loader):
     """Loader that returns an already-imported target module as-is.
 

src/agentpool_commands/__init__.py

@@ -34,6 +34,11 @@ def __getattr__(name: str) -> object:
     return getattr(wolfharness_commands, name)
 
 
+# NOTE: _AliasLoader is intentionally duplicated across all agentpool* shim
+# modules. These shims redirect to wolfharness* and must work before
+# wolfharness is imported, so the loader cannot be shared from a common
+# utility module. This duplication will be removed when the deprecated
+# shims are dropped.
 class _AliasLoader(abc.Loader):
     """Loader that returns an already-imported target module as-is.
 

src/agentpool_config/__init__.py

@@ -34,6 +34,11 @@ def __getattr__(name: str) -> object:
     return getattr(wolfharness_config, name)
 
 
+# NOTE: _AliasLoader is intentionally duplicated across all agentpool* shim
+# modules. These shims redirect to wolfharness* and must work before
+# wolfharness is imported, so the loader cannot be shared from a common
+# utility module. This duplication will be removed when the deprecated
+# shims are dropped.
 class _AliasLoader(abc.Loader):
     """Loader that returns an already-imported target module as-is.
 

src/agentpool_prompts/__init__.py

@@ -34,6 +34,11 @@ def __getattr__(name: str) -> object:
     return getattr(wolfharness_prompts, name)
 
 
+# NOTE: _AliasLoader is intentionally duplicated across all agentpool* shim
+# modules. These shims redirect to wolfharness* and must work before
+# wolfharness is imported, so the loader cannot be shared from a common
+# utility module. This duplication will be removed when the deprecated
+# shims are dropped.
 class _AliasLoader(abc.Loader):
     """Loader that returns an already-imported target module as-is.
 

src/agentpool_server/__init__.py

@@ -34,6 +34,11 @@ def __getattr__(name: str) -> object:
     return getattr(wolfharness_server, name)
 
 
+# NOTE: _AliasLoader is intentionally duplicated across all agentpool* shim
+# modules. These shims redirect to wolfharness* and must work before
+# wolfharness is imported, so the loader cannot be shared from a common
+# utility module. This duplication will be removed when the deprecated
+# shims are dropped.
 class _AliasLoader(abc.Loader):
     """Loader that returns an already-imported target module as-is.
 

src/agentpool_storage/__init__.py

@@ -34,6 +34,11 @@ def __getattr__(name: str) -> object:
     return getattr(wolfharness_storage, name)
 
 
+# NOTE: _AliasLoader is intentionally duplicated across all agentpool* shim
+# modules. These shims redirect to wolfharness* and must work before
+# wolfharness is imported, so the loader cannot be shared from a common
+# utility module. This duplication will be removed when the deprecated
+# shims are dropped.
 class _AliasLoader(abc.Loader):
     """Loader that returns an already-imported target module as-is.
 

src/agentpool_sync/__init__.py

@@ -34,6 +34,11 @@ def __getattr__(name: str) -> object:
     return getattr(wolfharness_sync, name)
 
 
+# NOTE: _AliasLoader is intentionally duplicated across all agentpool* shim
+# modules. These shims redirect to wolfharness* and must work before
+# wolfharness is imported, so the loader cannot be shared from a common
+# utility module. This duplication will be removed when the deprecated
+# shims are dropped.
 class _AliasLoader(abc.Loader):
     """Loader that returns an already-imported target module as-is.
 

src/agentpool_toolsets/__init__.py

@@ -34,6 +34,11 @@ def __getattr__(name: str) -> object:
     return getattr(wolfharness_toolsets, name)
 
 
+# NOTE: _AliasLoader is intentionally duplicated across all agentpool* shim
+# modules. These shims redirect to wolfharness* and must work before
+# wolfharness is imported, so the loader cannot be shared from a common
+# utility module. This duplication will be removed when the deprecated
+# shims are dropped.
 class _AliasLoader(abc.Loader):
     """Loader that returns an already-imported target module as-is.
 

src/wolfharness/agents/base_agent.py

@@ -40,6 +40,7 @@
     from collections.abc import AsyncGenerator
     from contextvars import Token
     from datetime import datetime
+    from types import TracebackType
 
     from evented_config import EventConfig
     from exxec import ExecutionEnvironment
@@ -576,11 +577,20 @@ async def _get_mcp_server_info(self) -> dict[str, MCPServerStatus]:
             # Only check cap.client (no lazy connection) to avoid
             # triggering _ensure_client() during status reporting.
             if cap.client is not None:
-                tool_entries = await cap.list_tools()
-                tools = [t.name for t in tool_entries]
-                info = cap.client.server_info
-                server_name = info.get("name") if info else None
-                server_version = info.get("version") if info else None
+                try:
+                    tool_entries = await cap.list_tools()
+                    tools = [t.name for t in tool_entries]
+                    info = cap.client.server_info
+                    server_name = info.get("name") if info else None
+                    server_version = info.get("version") if info else None
+                except Exception:
+                    logger.exception(
+                        "Failed to list tools from config-defined MCP capability",
+                        extra={"capability": cap},
+                    )
+                    tools = []
+                    server_name = None
+                    server_version = None
             else:
                 tools = []
                 server_name = None
@@ -717,6 +727,37 @@ async def _with_session_capabilities(
         finally:
             self._session_capabilities.clear()
 
+    async def __aexit__(
+        self,
+        exc_type: type[BaseException] | None,
+        exc_val: BaseException | None,
+        exc_tb: TracebackType | None,
+    ) -> None:
+        """Clean up external capabilities before base cleanup.
+
+        Config-defined capabilities (e.g. ``type: mcp`` created via
+        ``EntryPointCapabilityConfig.build()``) are stored in
+        ``_external_capabilities`` and never registered with an
+        ``MCPManager``. Their async context manager ``__aexit__`` is
+        called here so that resources like direct ``MCPClient``
+        connections (created by the ``_ensure_client`` fallback) are
+        properly closed instead of leaking on process exit.
+        """
+        from wolfharness.capabilities.combined_toolset import _LifecycleCapable
+
+        for cap in self._external_capabilities:
+            if not isinstance(cap, _LifecycleCapable):
+                continue
+            try:
+                await cap.__aexit__(exc_type, exc_val, exc_tb)
+            except Exception:
+                logger.warning(
+                    "Error during capability cleanup",
+                    exc_info=True,
+                    extra={"capability": cap},
+                )
+        await super().__aexit__(exc_type, exc_val, exc_tb)
+
     @asynccontextmanager
     async def _temporary_tools(
         self,

src/wolfharness/capabilities/tool_schema_overlap_config.py

@@ -119,7 +119,7 @@ class ParamOverride(BaseModel):
     name: NonEmptyStr | None = None
     description: str | None = None
     type: str | None = None
-    enum: list[str] | None = None
+    enum: list[Any] | None = None
     required: bool | None = None
     default: Any = UNDEFINED