From 3326f15ab1f91fad985f90fddb51ceed5974ac39 Mon Sep 17 00:00:00 2001 From: lxowalle Date: Wed, 4 Mar 2026 19:23:59 +0800 Subject: [PATCH] * fix some issue --- pkg/agent/loop.go | 67 +++++++++++++++++++++++------------------ pkg/config/config.go | 5 +-- pkg/config/defaults.go | 4 ++- pkg/mcp/manager_test.go | 16 ++++++++-- 4 files changed, 56 insertions(+), 36 deletions(-) diff --git a/pkg/agent/loop.go b/pkg/agent/loop.go index d37620e0e..0509866e4 100644 --- a/pkg/agent/loop.go +++ b/pkg/agent/loop.go @@ -165,32 +165,40 @@ func registerSharedTools( } // Skill discovery and installation tools - registryMgr := skills.NewRegistryManagerFromConfig(skills.RegistryConfig{ - MaxConcurrentSearches: cfg.Tools.Skills.MaxConcurrentSearches, - ClawHub: skills.ClawHubConfig(cfg.Tools.Skills.Registries.ClawHub), - }) - searchCache := skills.NewSearchCache( - cfg.Tools.Skills.SearchCache.MaxSize, - time.Duration(cfg.Tools.Skills.SearchCache.TTLSeconds)*time.Second, - ) - if cfg.Tools.IsToolEnabled("find_skills") { - agent.Tools.Register(tools.NewFindSkillsTool(registryMgr, searchCache)) - } - if cfg.Tools.IsToolEnabled("install_skill") { - agent.Tools.Register(tools.NewInstallSkillTool(registryMgr, agent.Workspace)) + find_skills_enable := cfg.Tools.IsToolEnabled("find_skills") + install_skills_enable := cfg.Tools.IsToolEnabled("install_skill") + if find_skills_enable || install_skills_enable { + registryMgr := skills.NewRegistryManagerFromConfig(skills.RegistryConfig{ + MaxConcurrentSearches: cfg.Tools.Skills.MaxConcurrentSearches, + ClawHub: skills.ClawHubConfig(cfg.Tools.Skills.Registries.ClawHub), + }) + + if find_skills_enable { + searchCache := skills.NewSearchCache( + cfg.Tools.Skills.SearchCache.MaxSize, + time.Duration(cfg.Tools.Skills.SearchCache.TTLSeconds)*time.Second, + ) + agent.Tools.Register(tools.NewFindSkillsTool(registryMgr, searchCache)) + } + + if install_skills_enable { + agent.Tools.Register(tools.NewInstallSkillTool(registryMgr, agent.Workspace)) + } } // Spawn tool with allowlist checker - if cfg.Tools.IsToolEnabled("subagent") { - subagentManager := tools.NewSubagentManager(provider, agent.Model, agent.Workspace, msgBus) - subagentManager.SetLLMOptions(agent.MaxTokens, agent.Temperature) - if cfg.Tools.IsToolEnabled("spawn") { + if cfg.Tools.IsToolEnabled("spawn") { + if cfg.Tools.IsToolEnabled("subagent") { + subagentManager := tools.NewSubagentManager(provider, agent.Model, agent.Workspace, msgBus) + subagentManager.SetLLMOptions(agent.MaxTokens, agent.Temperature) spawnTool := tools.NewSpawnTool(subagentManager) currentAgentID := agentID spawnTool.SetAllowlistChecker(func(targetAgentID string) bool { return registry.CanSpawnSubagent(currentAgentID, targetAgentID) }) agent.Tools.Register(spawnTool) + } else { + logger.WarnCF("agent", "spawn tool requires subagent to be enabled", nil) } } } @@ -200,7 +208,7 @@ func (al *AgentLoop) Run(ctx context.Context) error { al.running.Store(true) // Initialize MCP servers for all agents - if al.cfg.Tools.MCP.Enabled { + if al.cfg.Tools.IsToolEnabled("mcp") { mcpManager := mcp.NewManager() // Ensure MCP connections are cleaned up on exit, regardless of initialization success // This fixes resource leak when LoadFromMCPConfig partially succeeds then fails @@ -242,18 +250,17 @@ func (al *AgentLoop) Run(ctx context.Context) error { if !ok { continue } - if al.cfg.Tools.IsToolEnabled("mcp") { - mcpTool := tools.NewMCPTool(mcpManager, serverName, tool) - agent.Tools.Register(mcpTool) - totalRegistrations++ - logger.DebugCF("agent", "Registered MCP tool", - map[string]any{ - "agent_id": agentID, - "server": serverName, - "tool": tool.Name, - "name": mcpTool.Name(), - }) - } + + mcpTool := tools.NewMCPTool(mcpManager, serverName, tool) + agent.Tools.Register(mcpTool) + totalRegistrations++ + logger.DebugCF("agent", "Registered MCP tool", + map[string]any{ + "agent_id": agentID, + "server": serverName, + "tool": tool.Name, + "name": mcpTool.Name(), + }) } } } diff --git a/pkg/config/config.go b/pkg/config/config.go index 5daaa0cab..27a99f8be 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -666,8 +666,7 @@ type MCPServerConfig struct { // MCPConfig defines configuration for all MCP servers type MCPConfig struct { - // Enabled globally enables/disables MCP integration - Enabled bool `json:"enabled" env:"PICOCLAW_TOOLS_MCP_ENABLED"` + ToolConfig `env:"PICOCLAW_TOOLS_MCP_"` // Servers is a map of server name to server configuration Servers map[string]MCPServerConfig `json:"servers,omitempty"` } @@ -892,6 +891,8 @@ func (t *ToolsConfig) IsToolEnabled(name string) bool { return t.WebFetch.Enabled case "write_file": return t.WriteFile.Enabled + case "mcp": + return t.MCP.Enabled default: return true } diff --git a/pkg/config/defaults.go b/pkg/config/defaults.go index 5bf188389..f451edd15 100644 --- a/pkg/config/defaults.go +++ b/pkg/config/defaults.go @@ -385,7 +385,9 @@ func DefaultConfig() *Config { }, }, MCP: MCPConfig{ - Enabled: false, + ToolConfig: ToolConfig{ + Enabled: true, + }, Servers: map[string]MCPServerConfig{}, }, AppendFile: ToolConfig{ diff --git a/pkg/mcp/manager_test.go b/pkg/mcp/manager_test.go index 8ce81d09e..f353942ab 100644 --- a/pkg/mcp/manager_test.go +++ b/pkg/mcp/manager_test.go @@ -194,7 +194,9 @@ func TestLoadFromMCPConfig_EmptyWorkspaceWithRelativeEnvFile(t *testing.T) { mgr := NewManager() mcpCfg := config.MCPConfig{ - Enabled: true, + ToolConfig: config.ToolConfig{ + Enabled: true, + }, Servers: map[string]config.MCPServerConfig{ "test-server": { Enabled: true, @@ -228,12 +230,20 @@ func TestNewManager_InitialState(t *testing.T) { func TestLoadFromMCPConfig_DisabledOrEmptyServers(t *testing.T) { mgr := NewManager() - err := mgr.LoadFromMCPConfig(context.Background(), config.MCPConfig{Enabled: false}, "/tmp") + err := mgr.LoadFromMCPConfig( + context.Background(), + config.MCPConfig{ToolConfig: config.ToolConfig{Enabled: false}}, + "/tmp", + ) if err != nil { t.Fatalf("expected nil error when MCP disabled, got: %v", err) } - err = mgr.LoadFromMCPConfig(context.Background(), config.MCPConfig{Enabled: true}, "/tmp") + err = mgr.LoadFromMCPConfig( + context.Background(), + config.MCPConfig{ToolConfig: config.ToolConfig{Enabled: true}}, + "/tmp", + ) if err != nil { t.Fatalf("expected nil error when no servers configured, got: %v", err) }