fix(agent): ensure MCP cleanup on all Run() exit paths

- Add defer in Run() to guarantee MCP connection cleanup
- Handles both normal termination and context cancellation
- Prevents resource leaks when Run() exits via ctx.Done()
- MCP Manager.Close() is idempotent, safe to call from both defer and Stop()

This fixes GitHub Copilot feedback that MCP cleanup only happened in
Stop() but Run() could return on ctx.Done() without cleanup, causing
subprocess/session leaks on normal cancellation shutdown.
This commit is contained in:
yuchou87 2026-02-16 19:56:00 +08:00
parent aed7296c0d
commit 2318232b71

View file

@ -42,9 +42,9 @@ type AgentLoop struct {
state *state.Manager state *state.Manager
contextBuilder *ContextBuilder contextBuilder *ContextBuilder
tools *tools.ToolRegistry tools *tools.ToolRegistry
mcpManager *mcp.Manager // MCP server manager for resource cleanup mcpManager *mcp.Manager // MCP server manager for resource cleanup
mcpConfig *config.Config // Config for lazy MCP initialization mcpConfig *config.Config // Config for lazy MCP initialization
mcpInitOnce sync.Once // Ensures MCP is initialized only once mcpInitOnce sync.Once // Ensures MCP is initialized only once
running atomic.Bool running atomic.Bool
summarizing sync.Map // Tracks which sessions are currently being summarized summarizing sync.Map // Tracks which sessions are currently being summarized
channelManager *channels.Manager channelManager *channels.Manager
@ -192,6 +192,18 @@ func (al *AgentLoop) Run(ctx context.Context) error {
} }
}) })
// Ensure MCP connections are cleaned up on all exit paths
defer func() {
if al.mcpManager != nil {
if err := al.mcpManager.Close(); err != nil {
logger.ErrorCF("agent", "Failed to close MCP manager",
map[string]interface{}{
"error": err.Error(),
})
}
}
}()
for al.running.Load() { for al.running.Load() {
select { select {
case <-ctx.Done(): case <-ctx.Done():