mirror of
https://github.com/sipeed/picoclaw.git
synced 2026-06-12 18:08:54 +00:00
fix(tools): propagate tool registry to subagents (#1711)
* fix(tools): propagate tool registry to subagents via Clone SubagentManager was created with an empty ToolRegistry and SetTools() was never called, causing all subagent tool invocations to fail with "tool not found". This was a regression from the multi-agent refactor. Fix: clone the parent agent's tool registry into the subagent manager after creation but before spawn/spawn_status registration — giving subagents access to file, exec, web, and other tools while preventing recursive subagent spawning. - Add ToolRegistry.Clone() for independent shallow copies - Call subagentManager.SetTools(agent.Tools.Clone()) in registerSharedTools - Add tests for Clone isolation, empty clone, and hidden tool state Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(tools): fix cron_test build error and add TTL clone test - Fix cron_test.go:229 — replace non-existent SubscribeOutbound(ctx) with select on OutboundChan(), matching the MessageBus channel API - Add TestToolRegistry_Clone_PreservesTTLValue per reviewer feedback - Add version reset note to Clone() doc comment Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -239,6 +239,11 @@ func registerSharedTools(
|
||||
if (spawnEnabled || spawnStatusEnabled) && cfg.Tools.IsToolEnabled("subagent") {
|
||||
subagentManager := tools.NewSubagentManager(provider, agent.Model, agent.Workspace)
|
||||
subagentManager.SetLLMOptions(agent.MaxTokens, agent.Temperature)
|
||||
// Clone the parent's tool registry so subagents can use all
|
||||
// tools registered so far (file, web, etc.) but NOT spawn/
|
||||
// spawn_status which are added below — preventing recursive
|
||||
// subagent spawning.
|
||||
subagentManager.SetTools(agent.Tools.Clone())
|
||||
if spawnEnabled {
|
||||
spawnTool := tools.NewSpawnTool(subagentManager)
|
||||
currentAgentID := agentID
|
||||
|
||||
@@ -336,6 +336,28 @@ func (r *ToolRegistry) List() []string {
|
||||
return r.sortedToolNames()
|
||||
}
|
||||
|
||||
// Clone creates an independent copy of the registry containing the same tool
|
||||
// entries (shallow copy of each ToolEntry). This is used to give subagents a
|
||||
// snapshot of the parent agent's tools without sharing the same registry —
|
||||
// tools registered on the parent after cloning (e.g. spawn, spawn_status)
|
||||
// will NOT be visible to the clone, preventing recursive subagent spawning.
|
||||
// The version counter is reset to 0 in the clone as it's a new independent registry.
|
||||
func (r *ToolRegistry) Clone() *ToolRegistry {
|
||||
r.mu.RLock()
|
||||
defer r.mu.RUnlock()
|
||||
clone := &ToolRegistry{
|
||||
tools: make(map[string]*ToolEntry, len(r.tools)),
|
||||
}
|
||||
for name, entry := range r.tools {
|
||||
clone.tools[name] = &ToolEntry{
|
||||
Tool: entry.Tool,
|
||||
IsCore: entry.IsCore,
|
||||
TTL: entry.TTL,
|
||||
}
|
||||
}
|
||||
return clone
|
||||
}
|
||||
|
||||
// Count returns the number of registered tools.
|
||||
func (r *ToolRegistry) Count() int {
|
||||
r.mu.RLock()
|
||||
|
||||
@@ -336,6 +336,96 @@ func TestToolToSchema(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestToolRegistry_Clone(t *testing.T) {
|
||||
r := NewToolRegistry()
|
||||
r.Register(newMockTool("read_file", "reads files"))
|
||||
r.Register(newMockTool("exec", "runs commands"))
|
||||
r.Register(newMockTool("web_search", "searches the web"))
|
||||
|
||||
clone := r.Clone()
|
||||
|
||||
// Clone should have the same tools
|
||||
if clone.Count() != 3 {
|
||||
t.Errorf("expected clone to have 3 tools, got %d", clone.Count())
|
||||
}
|
||||
for _, name := range []string{"read_file", "exec", "web_search"} {
|
||||
if _, ok := clone.Get(name); !ok {
|
||||
t.Errorf("expected clone to have tool %q", name)
|
||||
}
|
||||
}
|
||||
|
||||
// Registering on parent should NOT affect clone
|
||||
r.Register(newMockTool("spawn", "spawns subagent"))
|
||||
if r.Count() != 4 {
|
||||
t.Errorf("expected parent to have 4 tools, got %d", r.Count())
|
||||
}
|
||||
if clone.Count() != 3 {
|
||||
t.Errorf("expected clone to still have 3 tools after parent mutation, got %d", clone.Count())
|
||||
}
|
||||
if _, ok := clone.Get("spawn"); ok {
|
||||
t.Error("expected clone NOT to have 'spawn' tool registered on parent after cloning")
|
||||
}
|
||||
|
||||
// Registering on clone should NOT affect parent
|
||||
clone.Register(newMockTool("custom", "custom tool"))
|
||||
if clone.Count() != 4 {
|
||||
t.Errorf("expected clone to have 4 tools, got %d", clone.Count())
|
||||
}
|
||||
if _, ok := r.Get("custom"); ok {
|
||||
t.Error("expected parent NOT to have 'custom' tool registered on clone")
|
||||
}
|
||||
}
|
||||
|
||||
func TestToolRegistry_Clone_Empty(t *testing.T) {
|
||||
r := NewToolRegistry()
|
||||
clone := r.Clone()
|
||||
if clone.Count() != 0 {
|
||||
t.Errorf("expected empty clone, got count %d", clone.Count())
|
||||
}
|
||||
}
|
||||
|
||||
func TestToolRegistry_Clone_PreservesHiddenToolState(t *testing.T) {
|
||||
r := NewToolRegistry()
|
||||
r.RegisterHidden(newMockTool("mcp_tool", "dynamic MCP tool"))
|
||||
|
||||
clone := r.Clone()
|
||||
|
||||
// Hidden tools with TTL=0 should not be gettable (same behavior as parent)
|
||||
if _, ok := clone.Get("mcp_tool"); ok {
|
||||
t.Error("expected hidden tool with TTL=0 to be invisible in clone")
|
||||
}
|
||||
|
||||
// But the entry should exist (count includes hidden tools)
|
||||
if clone.Count() != 1 {
|
||||
t.Errorf("expected clone count 1 (hidden entry exists), got %d", clone.Count())
|
||||
}
|
||||
}
|
||||
|
||||
func TestToolRegistry_Clone_PreservesTTLValue(t *testing.T) {
|
||||
r := NewToolRegistry()
|
||||
r.RegisterHidden(newMockTool("ttl_tool", "tool with TTL"))
|
||||
|
||||
// Manually set a non-zero TTL on the entry
|
||||
r.mu.RLock()
|
||||
if entry, ok := r.tools["ttl_tool"]; ok {
|
||||
entry.TTL = 5
|
||||
}
|
||||
r.mu.RUnlock()
|
||||
|
||||
clone := r.Clone()
|
||||
|
||||
// Verify TTL value is preserved in the clone
|
||||
clone.mu.RLock()
|
||||
defer clone.mu.RUnlock()
|
||||
entry, ok := clone.tools["ttl_tool"]
|
||||
if !ok {
|
||||
t.Fatal("expected ttl_tool to exist in clone")
|
||||
}
|
||||
if entry.TTL != 5 {
|
||||
t.Errorf("expected TTL=5 in clone, got %d", entry.TTL)
|
||||
}
|
||||
}
|
||||
|
||||
func TestToolRegistry_ConcurrentAccess(t *testing.T) {
|
||||
r := NewToolRegistry()
|
||||
var wg sync.WaitGroup
|
||||
|
||||
Reference in New Issue
Block a user