From 2318232b71737ac100495ec86fdf0833fe379c4c Mon Sep 17 00:00:00 2001
From: yuchou87 <yuchou87@gmail.com>
Date: Mon, 16 Feb 2026 19:56:00 +0800
Subject: [PATCH] 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.
---
 pkg/agent/loop.go | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/pkg/agent/loop.go b/pkg/agent/loop.go
index afd8e8481..a18e962fa 100644
--- a/pkg/agent/loop.go
+++ b/pkg/agent/loop.go
@@ -42,9 +42,9 @@ type AgentLoop struct {
 	state          *state.Manager
 	contextBuilder *ContextBuilder
 	tools          *tools.ToolRegistry
-	mcpManager     *mcp.Manager     // MCP server manager for resource cleanup
-	mcpConfig      *config.Config   // Config for lazy MCP initialization
-	mcpInitOnce    sync.Once        // Ensures MCP is initialized only once
+	mcpManager     *mcp.Manager   // MCP server manager for resource cleanup
+	mcpConfig      *config.Config // Config for lazy MCP initialization
+	mcpInitOnce    sync.Once      // Ensures MCP is initialized only once
 	running        atomic.Bool
 	summarizing    sync.Map // Tracks which sessions are currently being summarized
 	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() {
 		select {
 		case <-ctx.Done():