From 815e43e3ef77cf06107d24b5e41982ba2305303e Mon Sep 17 00:00:00 2001 From: afjcjsbx Date: Sun, 12 Apr 2026 21:37:19 +0200 Subject: [PATCH] fix(agent): reinitialize MCP and discovery tools after reload --- pkg/agent/loop.go | 15 ++++++++++ pkg/agent/loop_mcp.go | 10 +++++++ pkg/agent/loop_mcp_test.go | 60 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 85 insertions(+) diff --git a/pkg/agent/loop.go b/pkg/agent/loop.go index a856c0fca..6588db9f5 100644 --- a/pkg/agent/loop.go +++ b/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 diff --git a/pkg/agent/loop_mcp.go b/pkg/agent/loop_mcp.go index b9c844d1a..21b6b9eb2 100644 --- a/pkg/agent/loop_mcp.go +++ b/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 diff --git a/pkg/agent/loop_mcp_test.go b/pkg/agent/loop_mcp_test.go index 35c3e49c8..1c810f003 100644 --- a/pkg/agent/loop_mcp_test.go +++ b/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