https://docs.all-hands.dev/ logo
Join Slack
Powered by
I think agent-server has a pretty big MCP issue. M...
# proj-agent
w
I think agent-server has a pretty big MCP issue. My long-lived server had 163 Slack MCPs running 😅 I think there are two pieces to the problem: • we define MCP config at convo creation. I think every new convo starts a new version of the MCP server, even if it's already running ◦ we should probably deprecate this, and instead have a separate endpoint for starting MCPs • I don't think we're shutting down the MCP server at any point in the lifecycle ◦ we should at least kill the MCP on server shutdown ▪︎ but then we need to restart it when the server starts back up? ▪︎ if MCP gets its own endpoint and some file-level persistence it'd be easy to start it up on server start ◦ if the MCP is attached to the conversation, we should at least kill it when the convo is deleted ▪︎ I don't think this was happening
l
@nutritious-balloon-65098 expand upon this comment by investigating the code paths and suggest what might need to be changed. Record this in an issue on OpenHands/software-agent-sdk. Also consider the upcoming settings change when doing the analysis: sdk: centralize programmatic settings schema by neubig · Pull Request #2361 · OpenHands/software-agent-sdk
n
I'm on it! Graham Neubig can track my progress here.
Done — I investigated the MCP lifecycle code paths in 
software-agent-sdk
, expanded the original comment into a concrete analysis, and recorded it as a GitHub issue: - Issue filed: 
OpenHands/software-agent-sdk#2603
- Link: https://github.com/OpenHands/software-agent-sdk/issues/2603 ### Summary of the answer The problem appears to be both architectural and behavioral: - Architectural: MCP config is currently effectively conversation-owned. - 
RemoteConversation
sends the full 
agent
, including 
agent.mcp_config
, to 
/api/conversations
. - On first real use, 
LocalConversation
→ 
AgentBase._initialize()
→ 
create_mcp_tools(...)
creates a new 
MCPClient
per conversation. - Plugin 
.mcp.json
ends up merged into the same 
agent.mcp_config
, so it follows the same path. - Result: many conversations with the same Slack MCP config can legitimately create many MCP instances. - Behavioral / cleanup bugs: teardown is not reliably closing MCP clients. - 
EventService.close()
schedules 
self._conversation.close
in an executor but does not await it. - 
LocalConversation.close()
calls 
executor.close()
on tool executors, but 
MCPToolExecutor
does not implement 
close()
, so MCP cleanup is effectively a no-op. - This makes it plausible for MCP subprocesses/connections to survive conversation deletion or server shutdown. ### Suggested changes captured in the issue - Short-term bugfixes - Await conversation cleanup in 
EventService.close()
- Implement 
MCPToolExecutor.close()
to call 
MCPClient.sync_close()
- Add delete/shutdown tests for MCP cleanup - Medium-term design - Move toward a server-managed MCP lifecycle - Add separate MCP management/persistence endpoints/resources - Let conversations reference managed MCP resources instead of always owning inline MCP config - PR #2361 consideration - The upcoming typed settings model (
AgentSettings.mcp_config: MCPConfig | None
) should be treated as the canonical schema - Runtime MCP management should build on that, rather than inventing a second config shape Since this was an investigation/request-for-analysis task, no local code changes were made.
2 Views
Open in Slack
PreviousNext
Powered by