Improve MCP server lifecycle: caching and auto-restart#1872
Improve MCP server lifecycle: caching and auto-restart#1872dgageot merged 1 commit intodocker:mainfrom
Conversation
There was a problem hiding this comment.
Review Summary
This PR implements MCP server lifecycle improvements with caching and auto-restart functionality. The implementation is generally sound, but I found one concurrency issue with the notification handler callback:
Issue: The emitToolsChanged callback (invoked when MCP servers notify of tool list changes) blocks on r.CurrentAgentTools(context.Background()), which can take significant time if the MCP server is slow or unresponsive. Multiple rapid notifications could spawn concurrent blocking calls without rate limiting, potentially exhausting resources. While this affects sidebar updates rather than core functionality, it should be addressed to prevent resource exhaustion.
Positive findings: The caching mechanism with generation counters is well-designed, the restart logic with exponential backoff is correct, and the lock ordering avoids deadlocks.
- Cache tool and prompt lists in MCP Toolset, invalidated via ToolListChanged/PromptListChanged server notifications instead of re-fetching on every agent iteration - Auto-restart MCP servers that die unexpectedly with exponential backoff (up to 5 attempts), distinguishing crashes from intentional Stop() calls via a stopping flag Assisted-By: cagent Signed-off-by: David Gageot <david.gageot@docker.com>
Assisted-By: cagent