Skip to content

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

Merged
Million-mo merged 2 commits into
mainfrom
fix/pr384-review-issues
Aug 21, 2026
Merged

fix: post-merge review fixes for PR #384 (tool-schema-overlap)#386
Million-mo merged 2 commits into
mainfrom
fix/pr384-review-issues

Conversation

@Million-mo

Copy link
Copy Markdown
Collaborator

Summary

Fixes five issues identified during code review of the merged PR #384.

Changes

Priority Issue Fix
🔴 P0 _ensure_client direct MCPClient never closed (resource leak) BaseAgent.__aexit__ now calls __aexit__ on _LifecycleCapable caps in _external_capabilities
⚠️ P1 Internal Sany URL in examples/kb_diag_agent.yaml Replaced with https://mcp.example.com/…
⚠️ P1 _get_mcp_server_info crashes if cap.list_tools() fails Wrapped in try/except with logger.exception
⚠️ P2 ParamOverride.enum only accepted list[str] Relaxed to list[Any] for integer/number enums
💡 P3 _AliasLoader duplicated in 10 shim files without explanation Added "intentional duplication" comments

P0 Detail

McpServerCap.__aexit__ already handles closing direct clients correctly (if self._session_pool is None and self._client is not None: await self._client.__aexit__(...)), but nothing in the agent lifecycle ever called it on config-defined capabilities. This fix adds a cleanup loop in BaseAgent.__aexit__ that iterates _external_capabilities and calls __aexit__ on any _LifecycleCapable instances, wrapped in try/except for safety.

Verification

  • ruff check — all passed
  • ruff format --check — all passed
  • mypy — no issues
  • pytest — 153 tests passed (mcp_server_cap, tool_schema_overlap, shim identity, manager status)

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.
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.

1 participant