Skip to content

281 make mcp clients persist throughout the run#289

Merged
Levivus merged 22 commits into
mainfrom
281-make-mcp-clients-persist-throughout-the-run
Jul 15, 2025
Merged

281 make mcp clients persist throughout the run#289
Levivus merged 22 commits into
mainfrom
281-make-mcp-clients-persist-throughout-the-run

Conversation

@Levivus

@Levivus Levivus commented Jul 14, 2025

Copy link
Copy Markdown
Contributor

This PR is somewhat of an overhaul of how from_mcp_server works. from_mcp_server no longer returns a list of tools/nodes, but instead a new class, MCPServer. This is the new user facing MCP interaction object. It allows for clients to be persistent, and gives the user control to be able to close the server. To get the tools like we did before, MCPServer.tools returns the same list.

So, usage will be as follows:

mcp_server = from_mcp_server(config)`  # At this point, the server has started in a background thread.
mcp_tools = mcp_server.tools

# ... Code that uses the tools or whatever ...
mcp_server.close() # This really seems to be unnecessary, only if they need to close it for whatever reason

@Levivus Levivus linked an issue Jul 14, 2025 that may be closed by this pull request
@Levivus
Levivus requested review from Aryan-Railtown and soulFood5632 and removed request for Aryan-Railtown July 14, 2025 22:05
@Levivus
Levivus requested review from Copilot and removed request for soulFood5632 July 14, 2025 22:17

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 overhauls the MCP client integration by replacing the old from_mcp_server helper with a new MCPServer class that maintains a persistent client connection and exposes tools via a .tools property. It also refactors MCPAsyncClient to use explicit connect() and close() methods instead of async context managers, and removes the OAuth callback server code.

  • Refactored MCPAsyncClient to use connect()/close() in place of __aenter__/__aexit__
  • Introduced MCPServer class and updated from_mcp_server to return an MCPServer instance
  • Updated tests, examples, and documentation to reference MCPServer and its .tools property

Reviewed Changes

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

Show a summary per file
File Description
src/requestcompletion/rc_mcp/main.py Added connect/close methods, introduced MCPServer, updated from_mcp
src/requestcompletion/nodes/library/mcp_tool.py Changed from_mcp_server to return MCPServer and removed async helper
tests/unit_tests/rc_mcp/test_oauth.py Removed obsolete OAuth tests
tests/unit_tests/rc_mcp/test_main.py Updated client tests for new connect/close API
tests/integration_tests/rc_mcp/*.py Adapted integration tests to use server.tools
examples/rc_basics/.py, examples/integrations/.py Updated examples to assign from_mcp_server result to server and use .tools
docs/tools/guides/*.md Updated code snippets to use MCPServer
Comments suppressed due to low confidence (4)

src/requestcompletion/nodes/library/mcp_tool.py:16

  • The docstring for from_mcp_server still says it returns a list of Node classes, but the function now returns an MCPServer instance. Please update the docstring to reflect the new return type.
        List of Nodes, one for each discovered tool.

docs/tools/guides/websearch_integration.md:50

  • There's a TODO in the documentation link. Please update the link to the correct location for the from_mcp_server reference.
Read more about the `from_mcp_server` utility [TODO: change this link](../../mcp/index.md). <br>

examples/rc_basics/mcp_example.py:5

  • The symbol async_from_mcp_server was removed in this PR, so importing it will cause an error. Remove or update that import to only reference from_mcp_server.
from requestcompletion.nodes.library.mcp_tool import async_from_mcp_server, from_mcp_server

@Aryan-Railtown Aryan-Railtown left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Another small thing. I believe patch_httpx_AsyncClient fixture in the conftest for unit_tests/rc_mcp folder is not being used elsewhere. That can be safely remove.

Other than that, the tests and implementation looko good to me 👍

Comment thread src/requestcompletion/rc_mcp/main.py Outdated
Comment thread tests/unit_tests/rc_mcp/test_main.py
@Aryan-Railtown

Copy link
Copy Markdown
Contributor
image

Good catch by copilot

@Levivus
Levivus requested a review from soulFood5632 July 14, 2025 22:31

@soulFood5632 soulFood5632 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Some design comments around the approach. Looking forward to your responses

Comment thread examples/rc_basics/mcp_example.py Outdated
Comment thread src/requestcompletion/rc_mcp/main.py
Comment thread src/requestcompletion/rc_mcp/main.py
Comment thread src/requestcompletion/rc_mcp/main.py Outdated
Comment thread src/requestcompletion/rc_mcp/main.py Outdated
Comment thread src/requestcompletion/rc_mcp/main.py Outdated
Comment thread src/requestcompletion/rc_mcp/main.py Outdated
Comment thread src/requestcompletion/rc_mcp/main.py Outdated
@Levivus Levivus self-assigned this Jul 15, 2025
@Levivus
Levivus merged commit 4af621e into main Jul 15, 2025
2 checks passed
@Levivus
Levivus deleted the 281-make-mcp-clients-persist-throughout-the-run branch July 15, 2025 18:20
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.

Make MCP clients persist throughout the run

4 participants