diff --git a/pkg/agent/agent_init.go b/pkg/agent/agent_init.go index 76f12fa65..e95fbe7f8 100644 --- a/pkg/agent/agent_init.go +++ b/pkg/agent/agent_init.go @@ -337,5 +337,20 @@ func registerSharedTools( } else if (spawnEnabled || spawnStatusEnabled) && !cfg.Tools.IsToolEnabled("subagent") { logger.WarnCF("agent", "spawn/spawn_status tools require subagent to be enabled", nil) } + + // Register delegate tool for multi-agent setups. + // Auto-enabled when multiple agents exist. Delegation uses the SubTurn + // mechanism directly (not SubagentManager) and is independent of the + // subagent tool. + if len(registry.ListAgentIDs()) > 1 { + delegateTool := tools.NewDelegateTool() + delegateTool.SetSpawner(NewSubTurnSpawner(al)) + currentAgentID := agentID + delegateTool.SetSelfAgentID(currentAgentID) + delegateTool.SetAllowlistChecker(func(targetAgentID string) bool { + return registry.CanSpawnSubagent(currentAgentID, targetAgentID) + }) + agent.Tools.Register(delegateTool) + } } } diff --git a/pkg/agent/subturn.go b/pkg/agent/subturn.go index 483c72842..86617d02f 100644 --- a/pkg/agent/subturn.go +++ b/pkg/agent/subturn.go @@ -174,7 +174,10 @@ type SubTurnConfig struct { // Used by team tool to enforce token limits across all team members. InitialTokenBudget *atomic.Int64 - // Can be extended with temperature, topP, etc. + // TargetAgentID, when set, runs the sub-turn as the specified agent. + // The target agent's workspace, model, tools, and system prompt are used + // instead of the caller's. If empty, the sub-turn runs as the parent agent. + TargetAgentID string } // ====================== Context Keys ====================== @@ -232,6 +235,7 @@ func (s *AgentLoopSpawner) SpawnSubTurn( Critical: cfg.Critical, Timeout: cfg.Timeout, MaxContextRunes: cfg.MaxContextRunes, + TargetAgentID: cfg.TargetAgentID, } return spawnSubTurn(ctx, s.al, parentTS, agentCfg) @@ -314,8 +318,9 @@ func spawnSubTurn( return nil, ErrDepthLimitExceeded } - // 2. Config validation - if cfg.Model == "" { + // 2. Config validation: Model is required unless TargetAgentID is set + // (the target agent provides its own model). + if cfg.Model == "" && cfg.TargetAgentID == "" { return nil, ErrInvalidSubTurnConfig } @@ -333,12 +338,22 @@ func spawnSubTurn( childID := al.generateSubTurnID() - // Get the agent instance from parent, falling back to the default agent. - // Wrap it in a shallow copy that uses an ephemeral (in-memory only) session store - // so that child turns never pollute or persist to the parent's session history. - baseAgent := parentTS.agent - if baseAgent == nil { - baseAgent = al.registry.GetDefaultAgent() + // Resolve the agent instance for the child turn. + // When TargetAgentID is set, look up that agent from the registry so the + // child runs with the target's workspace, model, tools, and system prompt. + // Otherwise fall back to the parent's agent (existing behavior). + var baseAgent *AgentInstance + if cfg.TargetAgentID != "" { + var ok bool + baseAgent, ok = al.registry.GetAgent(cfg.TargetAgentID) + if !ok { + return nil, fmt.Errorf("target agent %q not found in registry", cfg.TargetAgentID) + } + } else { + baseAgent = parentTS.agent + if baseAgent == nil { + baseAgent = al.registry.GetDefaultAgent() + } } if baseAgent == nil { return nil, errors.New("parent turnState has no agent instance") diff --git a/pkg/agent/subturn_test.go b/pkg/agent/subturn_test.go index 6a33892db..e9f557c82 100644 --- a/pkg/agent/subturn_test.go +++ b/pkg/agent/subturn_test.go @@ -4,6 +4,9 @@ import ( "context" "errors" "fmt" + "os" + "path/filepath" + "strings" "sync" "testing" "time" @@ -2122,3 +2125,206 @@ func TestSubTurn_IndependentContext(t *testing.T) { t.Log("✓ SubTurn completed successfully (independent context)") } } + +// ====================== TargetAgentID Tests ====================== + +// modelRecordingProvider captures the model passed to Chat for test assertions. +type modelRecordingProvider struct { + mu sync.Mutex + lastModel string +} + +func (rp *modelRecordingProvider) Chat( + _ context.Context, + _ []providers.Message, + _ []providers.ToolDefinition, + model string, + _ map[string]any, +) (*providers.LLMResponse, error) { + rp.mu.Lock() + rp.lastModel = model + rp.mu.Unlock() + return &providers.LLMResponse{Content: "Mock response"}, nil +} + +func (rp *modelRecordingProvider) GetDefaultModel() string { return "mock-model" } + +func (rp *modelRecordingProvider) getLastModel() string { + rp.mu.Lock() + defer rp.mu.Unlock() + return rp.lastModel +} + +// newMultiAgentLoop creates an AgentLoop with two named agents for testing +// cross-agent delegation via TargetAgentID. +func newMultiAgentLoop(t *testing.T, provider providers.LLMProvider) (*AgentLoop, func()) { + t.Helper() + tmpDir, err := os.MkdirTemp("", "multiagent-test-*") + if err != nil { + t.Fatalf("create temp dir: %v", err) + } + + alphaDir := filepath.Join(tmpDir, "alpha") + betaDir := filepath.Join(tmpDir, "beta") + os.MkdirAll(alphaDir, 0o755) + os.MkdirAll(betaDir, 0o755) + + cfg := &config.Config{ + Agents: config.AgentsConfig{ + Defaults: config.AgentDefaults{ + Workspace: tmpDir, + ModelName: "default-model", + MaxTokens: 4096, + MaxToolIterations: 10, + }, + List: []config.AgentConfig{ + { + ID: "alpha", + Workspace: alphaDir, + Model: &config.AgentModelConfig{Primary: "model-alpha"}, + }, + { + ID: "beta", + Workspace: betaDir, + Model: &config.AgentModelConfig{Primary: "model-beta"}, + }, + }, + }, + } + + msgBus := bus.NewMessageBus() + al := NewAgentLoop(cfg, msgBus, provider) + + return al, func() { os.RemoveAll(tmpDir) } +} + +func TestSpawnSubTurn_TargetAgentID_UsesTargetAgent(t *testing.T) { + rp := &modelRecordingProvider{} + al, cleanup := newMultiAgentLoop(t, rp) + defer cleanup() + + alphaAgent, ok := al.registry.GetAgent("alpha") + if !ok { + t.Fatal("alpha agent not in registry") + } + + // Parent is alpha, target is beta + parent := &turnState{ + ctx: context.Background(), + turnID: "parent-alpha", + depth: 0, + childTurnIDs: []string{}, + pendingResults: make(chan *tools.ToolResult, 4), + concurrencySem: make(chan struct{}, testMaxConcurrentSubTurns), + session: &ephemeralSessionStore{}, + agent: alphaAgent, + } + + result, err := spawnSubTurn(context.Background(), al, parent, SubTurnConfig{ + TargetAgentID: "beta", + SystemPrompt: "task for beta", + }) + if err != nil { + t.Fatalf("spawnSubTurn failed: %v", err) + } + if result == nil { + t.Fatal("expected non-nil result") + } + + // The recording provider captures the model passed to Chat(). + // If TargetAgentID works correctly, the child turn should have + // used beta's model, not alpha's. + if got := rp.getLastModel(); got != "model-beta" { + t.Errorf("child turn used model %q, want %q", got, "model-beta") + } +} + +func TestSpawnSubTurn_TargetAgentID_NotFound(t *testing.T) { + al, cleanup := newMultiAgentLoop(t, &mockProvider{}) + defer cleanup() + + alphaAgent, _ := al.registry.GetAgent("alpha") + parent := &turnState{ + ctx: context.Background(), + turnID: "parent-alpha", + depth: 0, + childTurnIDs: []string{}, + pendingResults: make(chan *tools.ToolResult, 4), + concurrencySem: make(chan struct{}, testMaxConcurrentSubTurns), + session: &ephemeralSessionStore{}, + agent: alphaAgent, + } + + _, err := spawnSubTurn(context.Background(), al, parent, SubTurnConfig{ + TargetAgentID: "nonexistent", + SystemPrompt: "task", + }) + + if err == nil { + t.Fatal("expected error for nonexistent agent") + } + if !strings.Contains(err.Error(), "not found") { + t.Errorf("error should mention 'not found', got: %v", err) + } +} + +func TestSpawnSubTurn_TargetAgentID_EmptyModelAccepted(t *testing.T) { + al, cleanup := newMultiAgentLoop(t, &mockProvider{}) + defer cleanup() + + alphaAgent, _ := al.registry.GetAgent("alpha") + parent := &turnState{ + ctx: context.Background(), + turnID: "parent-alpha", + depth: 0, + childTurnIDs: []string{}, + pendingResults: make(chan *tools.ToolResult, 4), + concurrencySem: make(chan struct{}, testMaxConcurrentSubTurns), + session: &ephemeralSessionStore{}, + agent: alphaAgent, + } + + // Model is empty but TargetAgentID is set — should NOT fail validation + result, err := spawnSubTurn(context.Background(), al, parent, SubTurnConfig{ + Model: "", // intentionally empty + TargetAgentID: "beta", + SystemPrompt: "task for beta", + }) + if err != nil { + t.Fatalf("should accept empty Model when TargetAgentID is set, got: %v", err) + } + if result == nil { + t.Fatal("expected non-nil result") + } +} + +func TestDelegateToolNotRegistered_SingleAgent(t *testing.T) { + // Single-agent setup: delegate should not be registered + al, _, _, provider, cleanup := newTestAgentLoop(t) + _ = provider + defer cleanup() + + agent := al.registry.GetDefaultAgent() + if agent == nil { + t.Fatal("default agent should exist") + } + if _, has := agent.Tools.Get("delegate"); has { + t.Error("delegate tool should not be registered in single-agent setup") + } +} + +func TestDelegateToolRegistered_MultiAgent(t *testing.T) { + al, cleanup := newMultiAgentLoop(t, &mockProvider{}) + defer cleanup() + + // Both agents should have the delegate tool + for _, id := range []string{"alpha", "beta"} { + agent, ok := al.registry.GetAgent(id) + if !ok { + t.Fatalf("agent %q not found", id) + } + if _, has := agent.Tools.Get("delegate"); !has { + t.Errorf("agent %q should have delegate tool in multi-agent setup", id) + } + } +} diff --git a/pkg/tools/delegate.go b/pkg/tools/delegate.go new file mode 100644 index 000000000..dcde27718 --- /dev/null +++ b/pkg/tools/delegate.go @@ -0,0 +1,104 @@ +package tools + +import ( + "context" + "fmt" + "strings" + + "github.com/sipeed/picoclaw/pkg/routing" +) + +// DelegateTool delegates a task to a specific named agent and waits for +// the result. Unlike spawn (async, fire-and-forget) or subagent (sync but +// generic), delegate targets a named agent and runs the task using that +// agent's own workspace, model, and tools. +type DelegateTool struct { + spawner SubTurnSpawner + allowlistCheck func(targetAgentID string) bool + selfAgentID string +} + +func NewDelegateTool() *DelegateTool { + return &DelegateTool{} +} + +func (t *DelegateTool) SetSpawner(spawner SubTurnSpawner) { + t.spawner = spawner +} + +func (t *DelegateTool) SetAllowlistChecker(check func(targetAgentID string) bool) { + t.allowlistCheck = check +} + +func (t *DelegateTool) SetSelfAgentID(id string) { + t.selfAgentID = id +} + +func (t *DelegateTool) Name() string { + return "delegate" +} + +func (t *DelegateTool) Description() string { + return "Delegate a task to another agent and wait for the result. " + + "Use this when another agent is better suited to handle a specific task " + + "based on their capabilities. The target agent runs with its own workspace, " + + "model, and tools." +} + +func (t *DelegateTool) Parameters() map[string]any { + return map[string]any{ + "type": "object", + "properties": map[string]any{ + "agent_id": map[string]any{ + "type": "string", + "description": "The ID of the target agent to delegate the task to", + }, + "task": map[string]any{ + "type": "string", + "description": "Clear description of the task to delegate", + }, + }, + "required": []string{"agent_id", "task"}, + } +} + +func (t *DelegateTool) Execute(ctx context.Context, args map[string]any) *ToolResult { + rawAgentID, _ := args["agent_id"].(string) + if strings.TrimSpace(rawAgentID) == "" { + return ErrorResult("agent_id is required and must be a non-empty string") + } + agentID := routing.NormalizeAgentID(rawAgentID) + + task, _ := args["task"].(string) + if strings.TrimSpace(task) == "" { + return ErrorResult("task is required and must be a non-empty string") + } + + if t.selfAgentID != "" && agentID == t.selfAgentID { + return ErrorResult("cannot delegate to self") + } + + if t.allowlistCheck != nil && !t.allowlistCheck(agentID) { + return ErrorResult(fmt.Sprintf("not allowed to delegate to agent %q", agentID)) + } + + if t.spawner == nil { + return ErrorResult("delegate tool not configured") + } + + result, err := t.spawner.SpawnSubTurn(ctx, SubTurnConfig{ + TargetAgentID: agentID, + SystemPrompt: task, + Async: false, + }) + if err != nil { + return ErrorResult(fmt.Sprintf("delegation to agent %q failed: %v", agentID, err)).WithError(err) + } + if result == nil { + return ErrorResult(fmt.Sprintf("delegation to agent %q returned no result", agentID)) + } + + result.ForLLM = fmt.Sprintf("[Response from agent %q]\n%s", agentID, result.ForLLM) + + return result +} diff --git a/pkg/tools/delegate_test.go b/pkg/tools/delegate_test.go new file mode 100644 index 000000000..729c524a7 --- /dev/null +++ b/pkg/tools/delegate_test.go @@ -0,0 +1,300 @@ +package tools + +import ( + "context" + "fmt" + "strings" + "testing" +) + +// delegateMockSpawner records the config and returns a canned result. +type delegateMockSpawner struct { + lastCfg SubTurnConfig + result *ToolResult + err error +} + +func (m *delegateMockSpawner) SpawnSubTurn(_ context.Context, cfg SubTurnConfig) (*ToolResult, error) { + m.lastCfg = cfg + if m.err != nil { + return nil, m.err + } + if m.result != nil { + return m.result, nil + } + return &ToolResult{ + ForLLM: "completed: " + cfg.SystemPrompt, + ForUser: "completed", + }, nil +} + +func TestDelegateTool_Name(t *testing.T) { + tool := NewDelegateTool() + if tool.Name() != "delegate" { + t.Errorf("Name() = %q, want %q", tool.Name(), "delegate") + } +} + +func TestDelegateTool_Parameters(t *testing.T) { + tool := NewDelegateTool() + params := tool.Parameters() + + props, ok := params["properties"].(map[string]any) + if !ok { + t.Fatal("properties should be a map") + } + _, hasAgentID := props["agent_id"] + if !hasAgentID { + t.Error("agent_id parameter should exist") + } + _, hasTask := props["task"] + if !hasTask { + t.Error("task parameter should exist") + } + + required, ok := params["required"].([]string) + if !ok { + t.Fatal("required should be a string array") + } + if len(required) != 2 { + t.Fatalf("required should have 2 entries, got %d", len(required)) + } +} + +func TestDelegateTool_Execute_Success(t *testing.T) { + spawner := &delegateMockSpawner{} + tool := NewDelegateTool() + tool.SetSpawner(spawner) + + result := tool.Execute(context.Background(), map[string]any{ + "agent_id": "researcher", + "task": "summarize the logs", + }) + + if result.IsError { + t.Fatalf("expected success, got error: %s", result.ForLLM) + } + if !strings.Contains(result.ForLLM, `[Response from agent "researcher"]`) { + t.Errorf("result should contain attribution, got: %s", result.ForLLM) + } + if !strings.Contains(result.ForLLM, "summarize the logs") { + t.Errorf("result should contain task output, got: %s", result.ForLLM) + } + + // Verify spawner received correct config + if spawner.lastCfg.TargetAgentID != "researcher" { + t.Errorf("TargetAgentID = %q, want %q", spawner.lastCfg.TargetAgentID, "researcher") + } + if spawner.lastCfg.Async { + t.Error("delegate should be synchronous (Async=false)") + } + if spawner.lastCfg.SystemPrompt != "summarize the logs" { + t.Errorf("SystemPrompt = %q, want %q", spawner.lastCfg.SystemPrompt, "summarize the logs") + } +} + +func TestDelegateTool_Execute_EmptyAgentID(t *testing.T) { + tests := []struct { + name string + args map[string]any + }{ + {"missing", map[string]any{"task": "test"}}, + {"empty string", map[string]any{"agent_id": "", "task": "test"}}, + {"whitespace only", map[string]any{"agent_id": " ", "task": "test"}}, + {"wrong type", map[string]any{"agent_id": 123, "task": "test"}}, + } + + tool := NewDelegateTool() + tool.SetSpawner(&delegateMockSpawner{}) + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := tool.Execute(context.Background(), tt.args) + if !result.IsError { + t.Error("expected error for invalid agent_id") + } + if !strings.Contains(result.ForLLM, "agent_id is required") { + t.Errorf("error should mention agent_id, got: %s", result.ForLLM) + } + }) + } +} + +func TestDelegateTool_Execute_EmptyTask(t *testing.T) { + tests := []struct { + name string + args map[string]any + }{ + {"missing", map[string]any{"agent_id": "a"}}, + {"empty string", map[string]any{"agent_id": "a", "task": ""}}, + {"whitespace only", map[string]any{"agent_id": "a", "task": "\t\n"}}, + } + + tool := NewDelegateTool() + tool.SetSpawner(&delegateMockSpawner{}) + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := tool.Execute(context.Background(), tt.args) + if !result.IsError { + t.Error("expected error for invalid task") + } + if !strings.Contains(result.ForLLM, "task is required") { + t.Errorf("error should mention task, got: %s", result.ForLLM) + } + }) + } +} + +func TestDelegateTool_Execute_PermissionDenied(t *testing.T) { + tool := NewDelegateTool() + tool.SetSpawner(&delegateMockSpawner{}) + tool.SetAllowlistChecker(func(targetAgentID string) bool { + return targetAgentID == "allowed-agent" + }) + + result := tool.Execute(context.Background(), map[string]any{ + "agent_id": "forbidden-agent", + "task": "test", + }) + + if !result.IsError { + t.Error("expected error for denied agent") + } + if !strings.Contains(result.ForLLM, "not allowed to delegate") { + t.Errorf("error should mention permission, got: %s", result.ForLLM) + } +} + +func TestDelegateTool_Execute_PermissionAllowed(t *testing.T) { + tool := NewDelegateTool() + tool.SetSpawner(&delegateMockSpawner{}) + tool.SetAllowlistChecker(func(targetAgentID string) bool { + return targetAgentID == "allowed-agent" + }) + + result := tool.Execute(context.Background(), map[string]any{ + "agent_id": "allowed-agent", + "task": "test", + }) + + if result.IsError { + t.Errorf("expected success for allowed agent, got error: %s", result.ForLLM) + } +} + +func TestDelegateTool_Execute_NoSpawner(t *testing.T) { + tool := NewDelegateTool() + + result := tool.Execute(context.Background(), map[string]any{ + "agent_id": "a", + "task": "test", + }) + + if !result.IsError { + t.Error("expected error when spawner is nil") + } + if !strings.Contains(result.ForLLM, "not configured") { + t.Errorf("error should mention not configured, got: %s", result.ForLLM) + } +} + +func TestDelegateTool_Execute_SpawnerError(t *testing.T) { + spawner := &delegateMockSpawner{ + err: fmt.Errorf("context deadline exceeded"), + } + tool := NewDelegateTool() + tool.SetSpawner(spawner) + + result := tool.Execute(context.Background(), map[string]any{ + "agent_id": "researcher", + "task": "test", + }) + + if !result.IsError { + t.Error("expected error when spawner fails") + } + if !strings.Contains(result.ForLLM, "delegation to agent") { + t.Errorf("error should mention delegation failure, got: %s", result.ForLLM) + } + if !strings.Contains(result.ForLLM, "context deadline exceeded") { + t.Errorf("error should propagate cause, got: %s", result.ForLLM) + } +} + +func TestDelegateTool_Execute_NoAllowlistCheck(t *testing.T) { + // When no allowlist checker is set, all agents are allowed + tool := NewDelegateTool() + tool.SetSpawner(&delegateMockSpawner{}) + + result := tool.Execute(context.Background(), map[string]any{ + "agent_id": "any-agent", + "task": "test", + }) + + if result.IsError { + t.Errorf("expected success without allowlist, got error: %s", result.ForLLM) + } +} + +func TestDelegateTool_Execute_NilResult(t *testing.T) { + tool := NewDelegateTool() + tool.SetSpawner(&nilResultSpawner{}) + + result := tool.Execute(context.Background(), map[string]any{ + "agent_id": "researcher", + "task": "test", + }) + + if !result.IsError { + t.Error("expected error for nil result") + } + if !strings.Contains(result.ForLLM, "returned no result") { + t.Errorf("error should mention no result, got: %s", result.ForLLM) + } +} + +func TestDelegateTool_Execute_SelfDelegation(t *testing.T) { + tool := NewDelegateTool() + tool.SetSpawner(&delegateMockSpawner{}) + tool.SetSelfAgentID("alpha") + + result := tool.Execute(context.Background(), map[string]any{ + "agent_id": "alpha", + "task": "test", + }) + + if !result.IsError { + t.Error("expected error for self-delegation") + } + if !strings.Contains(result.ForLLM, "cannot delegate to self") { + t.Errorf("error should mention self-delegation, got: %s", result.ForLLM) + } +} + +func TestDelegateTool_Execute_SelfDelegation_Normalized(t *testing.T) { + tool := NewDelegateTool() + tool.SetSpawner(&delegateMockSpawner{}) + tool.SetSelfAgentID("alpha") // stored normalized + + // Case-insensitive and whitespace variants should still be caught + variants := []string{"ALPHA", " Alpha ", " alpha "} + for _, v := range variants { + t.Run(v, func(t *testing.T) { + result := tool.Execute(context.Background(), map[string]any{ + "agent_id": v, + "task": "test", + }) + if !result.IsError { + t.Errorf("agent_id=%q should be caught as self-delegation", v) + } + }) + } +} + +// nilResultSpawner always returns (nil, nil). +type nilResultSpawner struct{} + +func (m *nilResultSpawner) SpawnSubTurn(_ context.Context, _ SubTurnConfig) (*ToolResult, error) { + return nil, nil +} diff --git a/pkg/tools/subagent.go b/pkg/tools/subagent.go index ada89efb7..feeabe536 100644 --- a/pkg/tools/subagent.go +++ b/pkg/tools/subagent.go @@ -30,6 +30,7 @@ type SubTurnConfig struct { ActualSystemPrompt string InitialMessages []providers.Message InitialTokenBudget *atomic.Int64 // Shared token budget for team members; nil if no budget + TargetAgentID string // If set, run as this agent (its workspace, model, tools) } type SubagentTask struct {