diff --git a/pkg/agent/subturn.go b/pkg/agent/subturn.go index 058338fac..b1a2fd856 100644 --- a/pkg/agent/subturn.go +++ b/pkg/agent/subturn.go @@ -354,12 +354,12 @@ func spawnSubTurn( ephemeralStore := newEphemeralSession(nil) agent := *baseAgent // shallow copy agent.Sessions = ephemeralStore - if cfg.Tools != nil { - // Explicit tool slice from caller has highest priority for sub-turns. - agent.Tools = toolRegistryFromSlice(cfg.Tools) - } else if baseAgent.Tools != nil { - // Otherwise inherit the parent's tool snapshot. + if baseAgent.Tools != nil { + // Inherit the parent's tool registry snapshot so hidden/TTL metadata is preserved. agent.Tools = baseAgent.Tools.Clone() + } else if len(cfg.Tools) > 0 { + // Fallback path for callers that provide explicit tool slices without a parent registry. + agent.Tools = toolRegistryFromSlice(cfg.Tools) } else { agent.Tools = tools.NewToolRegistry() } diff --git a/pkg/agent/subturn_test.go b/pkg/agent/subturn_test.go index 24578ebc7..4b0d1882d 100644 --- a/pkg/agent/subturn_test.go +++ b/pkg/agent/subturn_test.go @@ -955,9 +955,11 @@ func (p *toolCaptureProvider) GetDefaultModel() string { return "test-model" } -type subturnProbeTool struct{} +type subturnProbeTool struct { + name string +} -func (t *subturnProbeTool) Name() string { return "subturn_probe_tool" } +func (t *subturnProbeTool) Name() string { return t.name } func (t *subturnProbeTool) Description() string { return "subturn probe tool" } @@ -971,7 +973,7 @@ func (t *subturnProbeTool) Execute(ctx context.Context, args map[string]any) *to return tools.SilentResult("ok") } -func TestSpawnSubTurn_UsesExplicitConfigTools(t *testing.T) { +func TestSpawnSubTurn_EmptyExplicitToolsStillInheritParentRegistry(t *testing.T) { provider := &toolCaptureProvider{} cfg := &config.Config{ Agents: config.AgentsConfig{ @@ -990,11 +992,12 @@ func TestSpawnSubTurn_UsesExplicitConfigTools(t *testing.T) { t.Fatal("expected default agent") } - // Keep parent tools empty so child tools can only come from cfg.Tools. parentAgent.Tools = tools.NewToolRegistry() + parentAgent.Tools.Register(&subturnProbeTool{name: "inherited_tool"}) + parent := &turnState{ ctx: context.Background(), - turnID: "parent-explicit-tools", + turnID: "parent-empty-explicit-tools", depth: 0, pendingResults: make(chan *tools.ToolResult, 1), concurrencySem: make(chan struct{}, testMaxConcurrentSubTurns), @@ -1005,14 +1008,164 @@ func TestSpawnSubTurn_UsesExplicitConfigTools(t *testing.T) { _, err := spawnSubTurn(context.Background(), al, parent, SubTurnConfig{ Model: "test-model", SystemPrompt: "run task", - Tools: []tools.Tool{&subturnProbeTool{}}, + Tools: []tools.Tool{}, }) if err != nil { t.Fatalf("spawnSubTurn returned error: %v", err) } - if len(provider.lastToolNames) != 1 || provider.lastToolNames[0] != "subturn_probe_tool" { - t.Fatalf("expected explicit cfg tool to be sent, got %v", provider.lastToolNames) + if len(provider.lastToolNames) != 1 || provider.lastToolNames[0] != "inherited_tool" { + t.Fatalf("expected inherited parent tools, got %v", provider.lastToolNames) + } +} + +func TestSpawnSubTurn_InheritsRuntimeAddedTools(t *testing.T) { + provider := &toolCaptureProvider{} + cfg := &config.Config{ + Agents: config.AgentsConfig{ + Defaults: config.AgentDefaults{ + Workspace: t.TempDir(), + ModelName: "test-model", + MaxTokens: 4096, + MaxToolIterations: 10, + }, + }, + } + al := NewAgentLoop(cfg, bus.NewMessageBus(), provider) + + parentAgent := al.registry.GetDefaultAgent() + if parentAgent == nil { + t.Fatal("expected default agent") + } + + parentAgent.Tools = tools.NewToolRegistry() + parentAgent.Tools.Register(&subturnProbeTool{name: "base_tool"}) + parent := &turnState{ + ctx: context.Background(), + turnID: "parent-runtime-tools", + depth: 0, + pendingResults: make(chan *tools.ToolResult, 1), + concurrencySem: make(chan struct{}, testMaxConcurrentSubTurns), + session: &ephemeralSessionStore{}, + agent: parentAgent, + } + + // Simulate runtime-added tools registered after initial manager setup. + parentAgent.Tools.Register(&subturnProbeTool{name: "runtime_tool"}) + + _, err := spawnSubTurn(context.Background(), al, parent, SubTurnConfig{ + Model: "test-model", + SystemPrompt: "run task", + }) + if err != nil { + t.Fatalf("spawnSubTurn returned error: %v", err) + } + + got := map[string]bool{} + for _, name := range provider.lastToolNames { + got[name] = true + } + if !got["base_tool"] || !got["runtime_tool"] { + t.Fatalf("expected runtime-added parent tools in provider defs, got %v", provider.lastToolNames) + } +} + +func TestSpawnSubTurn_PreservesHiddenTTLSemantics(t *testing.T) { + provider := &toolCaptureProvider{} + cfg := &config.Config{ + Agents: config.AgentsConfig{ + Defaults: config.AgentDefaults{ + Workspace: t.TempDir(), + ModelName: "test-model", + MaxTokens: 4096, + MaxToolIterations: 10, + }, + }, + } + al := NewAgentLoop(cfg, bus.NewMessageBus(), provider) + + parentAgent := al.registry.GetDefaultAgent() + if parentAgent == nil { + t.Fatal("expected default agent") + } + + parentAgent.Tools = tools.NewToolRegistry() + parentAgent.Tools.Register(&subturnProbeTool{name: "core_tool"}) + parentAgent.Tools.RegisterHidden(&subturnProbeTool{name: "hidden_active"}) + parentAgent.Tools.RegisterHidden(&subturnProbeTool{name: "hidden_inactive"}) + parentAgent.Tools.PromoteTools([]string{"hidden_active"}, 2) + + parent := &turnState{ + ctx: context.Background(), + turnID: "parent-hidden-ttl", + depth: 0, + pendingResults: make(chan *tools.ToolResult, 1), + concurrencySem: make(chan struct{}, testMaxConcurrentSubTurns), + session: &ephemeralSessionStore{}, + agent: parentAgent, + } + + _, err := spawnSubTurn(context.Background(), al, parent, SubTurnConfig{ + Model: "test-model", + SystemPrompt: "run task", + }) + if err != nil { + t.Fatalf("spawnSubTurn returned error: %v", err) + } + + got := map[string]bool{} + for _, name := range provider.lastToolNames { + got[name] = true + } + if !got["core_tool"] || !got["hidden_active"] { + t.Fatalf("expected core + active hidden tools, got %v", provider.lastToolNames) + } + if got["hidden_inactive"] { + t.Fatalf("inactive hidden tool leaked into provider defs: %v", provider.lastToolNames) + } +} + +func TestSpawnSubTurn_UsesExplicitToolsWhenParentRegistryUnavailable(t *testing.T) { + provider := &toolCaptureProvider{} + cfg := &config.Config{ + Agents: config.AgentsConfig{ + Defaults: config.AgentDefaults{ + Workspace: t.TempDir(), + ModelName: "test-model", + MaxTokens: 4096, + MaxToolIterations: 10, + }, + }, + } + al := NewAgentLoop(cfg, bus.NewMessageBus(), provider) + + parentAgent := al.registry.GetDefaultAgent() + if parentAgent == nil { + t.Fatal("expected default agent") + } + parentAgent.Tools = nil + + parent := &turnState{ + ctx: context.Background(), + turnID: "parent-explicit-fallback", + depth: 0, + pendingResults: make(chan *tools.ToolResult, 1), + concurrencySem: make(chan struct{}, testMaxConcurrentSubTurns), + session: &ephemeralSessionStore{}, + agent: parentAgent, + } + + _, err := spawnSubTurn(context.Background(), al, parent, SubTurnConfig{ + Model: "test-model", + SystemPrompt: "run task", + Tools: []tools.Tool{&subturnProbeTool{name: "explicit_tool"}}, + }) + if err != nil { + t.Fatalf("spawnSubTurn returned error: %v", err) + } + + if len(provider.lastToolNames) != 1 || provider.lastToolNames[0] != "explicit_tool" { + t.Fatalf("expected explicit fallback tools, got %v", provider.lastToolNames) } } diff --git a/pkg/tools/spawn.go b/pkg/tools/spawn.go index d2cecb88c..2929de4a0 100644 --- a/pkg/tools/spawn.go +++ b/pkg/tools/spawn.go @@ -11,7 +11,6 @@ type SpawnTool struct { defaultModel string maxTokens int temperature float64 - tools *ToolRegistry allowlistCheck func(targetAgentID string) bool } @@ -26,7 +25,6 @@ func NewSpawnTool(manager *SubagentManager) *SpawnTool { defaultModel: manager.defaultModel, maxTokens: manager.maxTokens, temperature: manager.temperature, - tools: manager.tools, } } @@ -126,7 +124,7 @@ Task: %s`, go func() { result, err := t.spawner.SpawnSubTurn(ctx, SubTurnConfig{ Model: t.defaultModel, - Tools: snapshotTools(t.tools), + Tools: nil, // Inherit from the parent turn registry at runtime. SystemPrompt: systemPrompt, MaxTokens: t.maxTokens, Temperature: t.temperature, diff --git a/pkg/tools/spawn_test.go b/pkg/tools/spawn_test.go index 198c479f3..e6ea75f58 100644 --- a/pkg/tools/spawn_test.go +++ b/pkg/tools/spawn_test.go @@ -49,10 +49,12 @@ func (t *managerSnapshotTool) Execute(ctx context.Context, args map[string]any) type recordingSpawner struct { toolNames []string + toolsNil bool done chan struct{} } func (s *recordingSpawner) SpawnSubTurn(ctx context.Context, cfg SubTurnConfig) (*ToolResult, error) { + s.toolsNil = cfg.Tools == nil for _, tool := range cfg.Tools { if tool != nil { s.toolNames = append(s.toolNames, tool.Name()) @@ -140,7 +142,7 @@ func TestSpawnTool_Execute_NilManager(t *testing.T) { } } -func TestSpawnTool_Execute_PassesManagerToolsToSubTurn(t *testing.T) { +func TestSpawnTool_Execute_LeavesToolsUnsetForRuntimeInheritance(t *testing.T) { provider := &MockLLMProvider{} manager := NewSubagentManager(provider, "test-model", "/tmp/test") manager.RegisterTool(&managerSnapshotTool{name: "snapshot_tool"}) @@ -166,7 +168,7 @@ func TestSpawnTool_Execute_PassesManagerToolsToSubTurn(t *testing.T) { t.Fatal("timed out waiting for async spawn execution") } - if len(spawner.toolNames) != 1 || spawner.toolNames[0] != "snapshot_tool" { - t.Fatalf("expected tool snapshot [snapshot_tool], got %v", spawner.toolNames) + if !spawner.toolsNil { + t.Fatalf("expected cfg.Tools to be nil for runtime inheritance, got %v", spawner.toolNames) } } diff --git a/pkg/tools/subagent.go b/pkg/tools/subagent.go index 280492e86..9e8b08dc1 100644 --- a/pkg/tools/subagent.go +++ b/pkg/tools/subagent.go @@ -115,21 +115,6 @@ func (sm *SubagentManager) RegisterTool(tool Tool) { sm.tools.Register(tool) } -func snapshotTools(toolsRegistry *ToolRegistry) []Tool { - if toolsRegistry == nil { - return nil - } - - names := toolsRegistry.List() - snapshot := make([]Tool, 0, len(names)) - for _, name := range names { - if tool, ok := toolsRegistry.Get(name); ok { - snapshot = append(snapshot, tool) - } - } - return snapshot -} - func (sm *SubagentManager) Spawn( ctx context.Context, task, label, agentID, originChannel, originChatID string, @@ -337,7 +322,6 @@ type SubagentTool struct { defaultModel string maxTokens int temperature float64 - tools *ToolRegistry } func NewSubagentTool(manager *SubagentManager) *SubagentTool { @@ -348,7 +332,6 @@ func NewSubagentTool(manager *SubagentManager) *SubagentTool { defaultModel: manager.defaultModel, maxTokens: manager.maxTokens, temperature: manager.temperature, - tools: manager.tools, } } @@ -412,7 +395,7 @@ Task: %s`, if t.spawner != nil { result, err := t.spawner.SpawnSubTurn(ctx, SubTurnConfig{ Model: t.defaultModel, - Tools: snapshotTools(t.tools), + Tools: nil, // Inherit from the parent turn registry at runtime. SystemPrompt: systemPrompt, MaxTokens: t.maxTokens, Temperature: t.temperature, diff --git a/pkg/tools/subagent_tool_test.go b/pkg/tools/subagent_tool_test.go index a13a13839..3c8ecf9ec 100644 --- a/pkg/tools/subagent_tool_test.go +++ b/pkg/tools/subagent_tool_test.go @@ -325,7 +325,7 @@ func TestSubagentTool_ForUserTruncation(t *testing.T) { } } -func TestSubagentTool_Execute_PassesManagerToolsToSubTurn(t *testing.T) { +func TestSubagentTool_Execute_LeavesToolsUnsetForRuntimeInheritance(t *testing.T) { provider := &MockLLMProvider{} manager := NewSubagentManager(provider, "test-model", "/tmp/test") manager.RegisterTool(&managerSnapshotTool{name: "snapshot_tool"}) @@ -344,7 +344,7 @@ func TestSubagentTool_Execute_PassesManagerToolsToSubTurn(t *testing.T) { t.Fatalf("Expected success, got error: %s", result.ForLLM) } - if len(spawner.toolNames) != 1 || spawner.toolNames[0] != "snapshot_tool" { - t.Fatalf("expected tool snapshot [snapshot_tool], got %v", spawner.toolNames) + if !spawner.toolsNil { + t.Fatalf("expected cfg.Tools to be nil for runtime inheritance, got %v", spawner.toolNames) } }