Skip to content

v1 mcp tools capbility overlap - #384

Merged
ykf173 merged 4 commits into
mainfrom
feature/mcp_cap_overlap
Aug 20, 2026
Merged

v1 mcp tools capbility overlap#384
ykf173 merged 4 commits into
mainfrom
feature/mcp_cap_overlap

Conversation

@ykf173

@ykf173 ykf173 commented Aug 20, 2026

Copy link
Copy Markdown
Collaborator

工具能力覆盖

闫凯峰 added 4 commits August 20, 2026 15:22
… code

- Remove duplicate MCPServerStatus import from TYPE_CHECKING block in
  base_agent.py (already imported at runtime) — fixes ruff TC004
- Add class-level _config: MCPServerConfig annotation in mcp_server_cap.py
  to help mypy infer instance attribute type through dict-normalization
  branch (TypeAdapter.validate_python returns Any) — fixes 6 mypy has-type
- Use conditional expression for MCPServerStatus.status field instead of
  intermediate str variable — fixes mypy arg-type (Literal mismatch)
Fix I001 (unsorted imports), D205 (blank line between summary and
description), D209 (closing quotes on separate line) in test files
that were failing CI ruff check.
@ykf173
ykf173 merged commit c24b567 into main Aug 20, 2026
12 checks passed
@Million-mo

Copy link
Copy Markdown
Collaborator

PR #384 检视报告

总体评价: 架构设计优秀,测试覆盖充分(~100 个测试)。以下按优先级列出需要关注的问题。


🔴 P0: _ensure_client 回退分支资源泄漏

mcp_server_cap.py_ensure_client() 回退分支创建了直接 MCPClient 但缺少对应的 __aexit__ 清理:

client = MCPClient(config=self._config)
await client.__aenter__()  # 建立连接
self._client = client
return client
# ← 没有任何关闭路径

config-defined 的 McpServerCap 不受任何 MCPManager 管理,进程退出时 TCP 连接 / stdio 进程会泄漏。对比 MCPManager__aexit__ 中统一关闭所有客户端连接。

建议: 让 McpServerCap 实现 AsyncExitStack 模式,或在 agent 的 cleanup() / __aexit__ 中遍历 _all_capabilities 关闭 McpServerCap._client


⚠️ P1: 示例 YAML 包含内部 URL

examples/kb_diag_agent.yaml 第 383 行包含三一内部开发环境 URL:

url: https://sprouts-dev-app-frontend.sany.com.cn/api-mcp/knowledge_diag/mcp

建议替换为占位符如 https://mcp.example.com/knowledge_diag/mcp


⚠️ P1: _get_mcp_server_infolist_tools() 缺少异常保护

base_agent.py 新增的状态扫描代码:

if cap.client is not None:
    tool_entries = await cap.list_tools()  # ← 客户端断开时会抛异常
    tools = [t.name for t in tool_entries]

如果已连接的客户端恰好断开,list_tools() 异常会导致整个 _get_mcp_server_info() 失败。建议加 try/except 并报告为 "error""disconnected" 状态。


⚠️ P2: enum 字段只支持字符串值

ParamOverride.enum 类型为 list[str] | None,不支持整数/数字枚举:

enum: list[str] | None = None

JSON Schema 允许任意类型的 enum(如 enum: [200, 404, 500])。建议改为 list[Any] | None,已有的 _default_matches_json_type 可以继续做类型检查。


⚠️ P2: param_additionsdefault: None 语义与 param_overrides 不同

  • param_overridesdefault: None = "移除现有默认值"
  • param_additionsdefault: None = "无默认值"

同一个 sentinel 的双重语义可能引起混淆。建议在文档中明确区分这一行为。


💡 P3: _AliasLoader 在 10 个 shim 文件中完全重复

_AliasLoader 类(~25 行)被完整复制到 10 个 agentpool*/__init__.py 中。理解这是因为 shim 需要在 wolfharness 导入前工作,无法从 wolfharness 导入共享工具类。建议在这些文件中加注释标注"有意重复,随 shim 废弃而移除"。


✅ 设计亮点

  • 身份驱动匹配: 使用 server_name + original_mcp_tool_name metadata 而非解析工具名,正确处理了 tool_prefix 场景
  • 两阶段验证: 构造时纯配置检查 + 首次 listing 时 schema 相关检查,fail-fast 在 agent 启动期暴露错误
  • 固定的 schema 改写顺序: removals → descriptions → field overrides → renames → additions
  • 与 display 层正交: get_ordering() 声明 wrapped_by=[ToolDisplayCapability]
  • 降级策略: 无身份 metadata 的工具原样透传,不做 name-guessing
  • 测试覆盖: 配置验证 / schema 改写 / prefix 组合 / display 组合 / 双服务器隔离 / 降级 / YAML round-trip / shim identity

Million-mo added a commit to Million-mo/agentpool that referenced this pull request Aug 21, 2026
…erving-degradation

Resolve conflict in mcp_server_cap.py: integrate PR wolf1069b#384's direct client
fallback and _connect_via_pool() extraction with our connect cooldown.
The cooldown check now lives inside _connect_via_pool() (pool-based
retry path) while the direct-client fallback (no pool) proceeds without
cooldown.
Million-mo added a commit that referenced this pull request Aug 21, 2026
* fix: post-merge review fixes for PR #384 (tool-schema-overlap)

P0: BaseAgent.__aexit__ now calls __aexit__ on _LifecycleCapable
    capabilities in _external_capabilities, closing direct MCPClient
    connections that were previously leaked.

P1: Replace internal Sany URL in examples/kb_diag_agent.yaml with
    placeholder.

P1: _get_mcp_server_info catches and logs errors from cap.list_tools()
    on config-defined MCP capabilities instead of crashing.

P2: ParamOverride.enum relaxed from list[str] to list[Any] to support
    integer/number enums.

P3: Added 'intentional duplication' comments to _AliasLoader in all 10
    agentpool* shim modules.

* fix(ci): use GITHUB_TOKEN to bypass OpenCode OIDC bug

OpenCode GitHub Action fails on repos created after 2026-07-15 due to
GitHub's new immutable OIDC sub format (anomalyco/opencode#37823).
Setting use_github_token: true bypasses the broken OIDC exchange path.

See: anomalyco/opencode#37823
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants