Merge pull request #2489 from afjcjsbx/fix/mcp-reload-discovery-tools

fix(agent): reinitialize MCP and discovery tools after reload

pkg/agent/loop.go

@@ -1056,8 +1056,23 @@ func (al *AgentLoop) ReloadProviderAndConfig(
 
 	al.mu.Unlock()
 
+	oldMCPManager := al.mcp.reset()
 	al.hookRuntime.reset(al)
 	configureHookManagerFromConfig(al.hooks, cfg)
+	if err := al.ensureHooksInitialized(ctx); err != nil {
+		logger.WarnCF("agent", "Configured hooks failed to reinitialize after reload",
+			map[string]any{"error": err.Error()})
+	}
+	if oldMCPManager != nil {
+		if err := oldMCPManager.Close(); err != nil {
+			logger.WarnCF("agent", "Failed to close previous MCP manager during reload",
+				map[string]any{"error": err.Error()})
+		}
+	}
+	if err := al.ensureMCPInitialized(ctx); err != nil {
+		logger.WarnCF("agent", "MCP failed to reinitialize after reload",
+			map[string]any{"error": err.Error()})
+	}
 
 	// Close old provider after releasing the lock
 	// This prevents blocking readers while closing

pkg/agent/loop_mcp.go

@@ -24,6 +24,16 @@ type mcpRuntime struct {
 	initErr  error
 }
 
+func (r *mcpRuntime) reset() *mcp.Manager {
+	r.mu.Lock()
+	manager := r.manager
+	r.manager = nil
+	r.initErr = nil
+	r.initOnce = sync.Once{}
+	r.mu.Unlock()
+	return manager
+}
+
 func (r *mcpRuntime) setManager(manager *mcp.Manager) {
 	r.mu.Lock()
 	r.manager = manager

pkg/agent/loop_mcp_test.go

@@ -7,13 +7,73 @@
 package agent
 
 import (
+	"context"
+	"errors"
 	"testing"
 
 	"github.com/sipeed/picoclaw/pkg/config"
+	"github.com/sipeed/picoclaw/pkg/mcp"
 )
 
 func boolPtr(b bool) *bool { return &b }
 
+func TestMCPRuntimeResetClearsState(t *testing.T) {
+	var rt mcpRuntime
+	manager := mcp.NewManager()
+	rt.setManager(manager)
+	rt.setInitErr(errors.New("stale init error"))
+	rt.initOnce.Do(func() {})
+
+	got := rt.reset()
+	if got != manager {
+		t.Fatalf("reset() manager = %p, want %p", got, manager)
+	}
+	if rt.hasManager() {
+		t.Fatal("expected manager to be cleared after reset")
+	}
+	if err := rt.getInitErr(); err != nil {
+		t.Fatalf("getInitErr() = %v, want nil", err)
+	}
+
+	reran := false
+	rt.initOnce.Do(func() { reran = true })
+	if !reran {
+		t.Fatal("expected initOnce to be reset")
+	}
+}
+
+func TestReloadProviderAndConfig_ResetsMCPRuntime(t *testing.T) {
+	al, cfg, _, _, cleanup := newTestAgentLoop(t)
+	defer cleanup()
+	defer al.Close()
+
+	manager := mcp.NewManager()
+	al.mcp.setManager(manager)
+	al.mcp.setInitErr(errors.New("stale init error"))
+	al.mcp.initOnce.Do(func() {})
+
+	if !al.mcp.hasManager() {
+		t.Fatal("expected MCP manager to exist before reload")
+	}
+
+	if err := al.ReloadProviderAndConfig(context.Background(), &mockProvider{}, cfg); err != nil {
+		t.Fatalf("ReloadProviderAndConfig() error = %v", err)
+	}
+
+	if al.mcp.hasManager() {
+		t.Fatal("expected MCP manager to be cleared when reloaded config has MCP disabled")
+	}
+	if err := al.mcp.getInitErr(); err != nil {
+		t.Fatalf("getInitErr() = %v, want nil", err)
+	}
+
+	reran := false
+	al.mcp.initOnce.Do(func() { reran = true })
+	if !reran {
+		t.Fatal("expected MCP initOnce to be reset after reload")
+	}
+}
+
 func TestServerIsDeferred(t *testing.T) {
 	tests := []struct {
 		name             string