Skip to content

Conversation

@nullchimp
Copy link
Owner

This pull request introduces significant enhancements to the MCP integration, asynchronous tool execution, and overall code organization. Key changes include the implementation of an MCP session manager, refactoring tool execution to support asynchronous operations, and introducing utility decorators for improved code readability and error handling. Below is a categorized summary of the most important changes:

Copilot AI review requested due to automatic review settings May 11, 2025 12:07
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR integrates MCP sessions, converts tool execution to async, and reorganizes decorators for cleaner control flow.

  • Introduces MCPSessionManager and updates main to discover and load MCP tools.
  • Refactors process_tool_calls and all tool run methods to async with callback support.
  • Adds utility decorators (graceful_exit, mainloop, chatutil) and updates tests for async behavior.

Reviewed Changes

Copilot reviewed 35 out of 35 changed files in this pull request and generated no comments.

Show a summary per file
File Description
tests/test_agent_root.py Updated tests to call process_tool_calls asynchronously with callbacks
tests/test_agent_process_coverage.py Added async tests for edge cases in process_tool_calls
tests/test_agent_main_block.py Added asyncio-based test for the __main__ block
tests/test_agent_edge_cases.py Consolidated and converted edge-case tests to a single async test
tests/test_agent_additional.py Updated exception-handling tests to async callbacks
src/utils/mcpclient/sessions_manager.py New MCPSessionManager for loading and listing MCP servers
src/utils/mcpclient/session.py New MCPSession wrapper for MCP client with graceful exits
src/utils/mcpclient/client.py Removed example client script
src/utils/azureopenai/chat.py Added add_tool method to register MCP tools dynamically
src/utils/init.py Introduced mainloop, graceful_exit, and chatutil decorators
src/tools/write_file.py Converted run to async
src/tools/web_fetch.py Converted run to async
src/tools/read_file.py Converted run to async
src/tools/list_files.py Converted run to async
src/tools/google_search.py Converted run to async
src/tools/init.py Enhanced Tool class to support async tool execution
src/main.py Orchestrates MCP discovery, decorator-driven loops, and agent tasks
src/chat.py Updated decorators and fixed invocation of run_conversation
src/agent.py Refactored to async process_tool_calls, added callback API
config/mcp.template.json Added template for MCP configuration
Comments suppressed due to low confidence (5)

src/agent.py:30

  • [nitpick] The parameter call_back would be clearer as callback (no underscore) to match common naming conventions.
async def process_tool_calls(response: Dict[str, Any], call_back) -> None:

src/utils/mcpclient/sessions_manager.py:10

  • There are no tests covering MCPSessionManager.load_mcp_sessions or list_tools. Consider adding unit tests or integration tests for session loading and tool listing.
class MCPSessionManager:

src/utils/mcpclient/sessions_manager.py:15

  • The return type is annotated as Dict[str, MCPSession] but the method returns True or None. Update it to return self._sessions on success or raise/log errors, and adjust the type hint accordingly.
async def load_mcp_sessions(self) -> Dict[str, mcp.MCPSession]:

src/utils/init.py:35

  • The chatutil decorator’s _wrapper does not return the wrapped function’s result, so the decorated function always returns None. Add return result at the end of _wrapper.
async def _wrapper(*args, **kwargs):

tests/test_agent_root.py:28

  • This test uses asyncio.run but the module doesn’t import asyncio. Add import asyncio at the top of the file so asyncio.run is defined.
with patch('builtins.print'):  # Suppress print output

@nullchimp nullchimp merged commit 2c39072 into main May 11, 2025
4 checks passed
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