refactor(agent): extract reusable tool loop and make subagents independent

Extract core LLM tool loop logic into shared RunToolLoop function that can be used by both main agent and subagents. Subagents now run their own tool loop with dedicated tool registry, enabling full independence. Key changes: - New pkg/tools/toolloop.go with reusable tool execution logic - Subagents use message tool to communicate directly with users - Heartbeat processing is now stateless via ProcessHeartbeat - Simplified system message routing without result forwarding - Shared tool registry creation for consistency between agents This architecture follows openclaw's design where async tools notify via bus and subagents handle their own user communication.
2026-02-13 14:39:39 +08:00
parent 4dfa133cb8
commit 0cce9fc905
5 changed files with 319 additions and 57 deletions
--- a/pkg/agent/loop.go
+++ b/pkg/agent/loop.go
@@ -51,25 +51,31 @@ type processOptions struct {
 	DefaultResponse string // Response when LLM returns empty
 	EnableSummary   bool   // Whether to trigger summarization
 	SendResponse    bool   // Whether to send response via bus
+	NoHistory       bool   // If true, don't load session history (for heartbeat)
 }

-func NewAgentLoop(cfg *config.Config, msgBus *bus.MessageBus, provider providers.LLMProvider) *AgentLoop {
-	workspace := cfg.WorkspacePath()
-	os.MkdirAll(workspace, 0755)
+// createToolRegistry creates a tool registry with common tools.
+// This is shared between main agent and subagents.
+func createToolRegistry(workspace string, restrict bool, cfg *config.Config, msgBus *bus.MessageBus) *tools.ToolRegistry {
+	registry := tools.NewToolRegistry()

-	restrict := cfg.Agents.Defaults.RestrictToWorkspace
+	// File system tools
+	registry.Register(tools.NewReadFileTool(workspace, restrict))
+	registry.Register(tools.NewWriteFileTool(workspace, restrict))
+	registry.Register(tools.NewListDirTool(workspace, restrict))
+	registry.Register(tools.NewEditFileTool(workspace, restrict))
+	registry.Register(tools.NewAppendFileTool(workspace, restrict))

-	toolsRegistry := tools.NewToolRegistry()
-	toolsRegistry.Register(tools.NewReadFileTool(workspace, restrict))
-	toolsRegistry.Register(tools.NewWriteFileTool(workspace, restrict))
-	toolsRegistry.Register(tools.NewListDirTool(workspace, restrict))
-	toolsRegistry.Register(tools.NewExecTool(workspace, restrict))
+	// Shell execution
+	registry.Register(tools.NewExecTool(workspace, restrict))

+	// Web tools
 	braveAPIKey := cfg.Tools.Web.Search.APIKey
-	toolsRegistry.Register(tools.NewWebSearchTool(braveAPIKey, cfg.Tools.Web.Search.MaxResults))
-	toolsRegistry.Register(tools.NewWebFetchTool(50000))
+	registry.Register(tools.NewWebSearchTool(braveAPIKey, cfg.Tools.Web.Search.MaxResults))
+	registry.Register(tools.NewWebFetchTool(50000))

-	// Register message tool
+	// Message tool - available to both agent and subagent
+	// Subagent uses it to communicate directly with user
 	messageTool := tools.NewMessageTool()
 	messageTool.SetSendCallback(func(channel, chatID, content string) error {
 		msgBus.PublishOutbound(bus.OutboundMessage{
@@ -79,10 +85,27 @@ func NewAgentLoop(cfg *config.Config, msgBus *bus.MessageBus, provider providers
 		})
 		return nil
 	})
-	toolsRegistry.Register(messageTool)
+	registry.Register(messageTool)

-	// Register spawn tool
+	return registry
+}
+
+func NewAgentLoop(cfg *config.Config, msgBus *bus.MessageBus, provider providers.LLMProvider) *AgentLoop {
+	workspace := cfg.WorkspacePath()
+	os.MkdirAll(workspace, 0755)
+
+	restrict := cfg.Agents.Defaults.RestrictToWorkspace
+
+	// Create tool registry for main agent
+	toolsRegistry := createToolRegistry(workspace, restrict, cfg, msgBus)
+
+	// Create subagent manager with its own tool registry
 	subagentManager := tools.NewSubagentManager(provider, cfg.Agents.Defaults.Model, workspace, msgBus)
+	subagentTools := createToolRegistry(workspace, restrict, cfg, msgBus)
+	// Subagent doesn't need spawn/subagent tools to avoid recursion
+	subagentManager.SetTools(subagentTools)
+
+	// Register spawn tool (for main agent)
 	spawnTool := tools.NewSpawnTool(subagentManager)
 	toolsRegistry.Register(spawnTool)

@@ -90,11 +113,6 @@ func NewAgentLoop(cfg *config.Config, msgBus *bus.MessageBus, provider providers
 	subagentTool := tools.NewSubagentTool(subagentManager)
 	toolsRegistry.Register(subagentTool)

-	// Register edit file tool
-	editFileTool := tools.NewEditFileTool(workspace, restrict)
-	toolsRegistry.Register(editFileTool)
-	toolsRegistry.Register(tools.NewAppendFileTool(workspace, restrict))
-
 	sessionsManager := session.NewSessionManager(filepath.Join(workspace, "sessions"))

 	// Create state manager for atomic state persistence
@@ -186,6 +204,21 @@ func (al *AgentLoop) ProcessDirectWithChannel(ctx context.Context, content, sess
 	return al.processMessage(ctx, msg)
 }

+// ProcessHeartbeat processes a heartbeat request without session history.
+// Each heartbeat is independent and doesn't accumulate context.
+func (al *AgentLoop) ProcessHeartbeat(ctx context.Context, content, channel, chatID string) (string, error) {
+	return al.runAgentLoop(ctx, processOptions{
+		SessionKey:      "heartbeat",
+		Channel:         channel,
+		ChatID:          chatID,
+		UserMessage:     content,
+		DefaultResponse: "I've completed processing but have no response to give.",
+		EnableSummary:   false,
+		SendResponse:    false,
+		NoHistory:       true, // Don't load session history for heartbeat
+	})
+}
+
 func (al *AgentLoop) processMessage(ctx context.Context, msg bus.InboundMessage) (string, error) {
 	// Add message preview to log (show full content for error messages)
 	var logContent string
@@ -231,30 +264,45 @@ func (al *AgentLoop) processSystemMessage(ctx context.Context, msg bus.InboundMe
 			"chat_id":   msg.ChatID,
 		})

-	// Parse origin from chat_id (format: "channel:chat_id")
-	var originChannel, originChatID string
+	// Parse origin channel from chat_id (format: "channel:chat_id")
+	var originChannel string
 	if idx := strings.Index(msg.ChatID, ":"); idx > 0 {
 		originChannel = msg.ChatID[:idx]
-		originChatID = msg.ChatID[idx+1:]
 	} else {
 		// Fallback
 		originChannel = "cli"
-		originChatID = msg.ChatID
 	}

-	// Use the origin session for context
-	sessionKey := fmt.Sprintf("%s:%s", originChannel, originChatID)
+	// Extract subagent result from message content
+	// Format: "Task 'label' completed.\n\nResult:\n<actual content>"
+	content := msg.Content
+	if idx := strings.Index(content, "Result:\n"); idx >= 0 {
+		content = content[idx+8:] // Extract just the result part
+	}

-	// Process as system message with routing back to origin
-	return al.runAgentLoop(ctx, processOptions{
-		SessionKey:      sessionKey,
-		Channel:         originChannel,
-		ChatID:          originChatID,
-		UserMessage:     fmt.Sprintf("[System: %s] %s", msg.SenderID, msg.Content),
-		DefaultResponse: "Background task completed.",
-		EnableSummary:   false,
-		SendResponse:    true, // Send response back to original channel
-	})
+	// Skip internal channels - only log, don't send to user
+	internalChannels := map[string]bool{"cli": true, "system": true, "subagent": true}
+	if internalChannels[originChannel] {
+		logger.InfoCF("agent", "Subagent completed (internal channel)",
+			map[string]interface{}{
+				"sender_id":    msg.SenderID,
+				"content_len":  len(content),
+				"channel":      originChannel,
+			})
+		return "", nil
+	}
+
+	// Agent acts as dispatcher only - subagent handles user interaction via message tool
+	// Don't forward result here, subagent should use message tool to communicate with user
+	logger.InfoCF("agent", "Subagent completed",
+		map[string]interface{}{
+			"sender_id":    msg.SenderID,
+			"channel":      originChannel,
+			"content_len":  len(content),
+		})
+
+	// Agent only logs, does not respond to user
+	return "", nil
 }

 // runAgentLoop is the core message processing logic.
@@ -275,9 +323,13 @@ func (al *AgentLoop) runAgentLoop(ctx context.Context, opts processOptions) (str
 	// 1. Update tool contexts
 	al.updateToolContexts(opts.Channel, opts.ChatID)

-	// 2. Build messages
-	history := al.sessions.GetHistory(opts.SessionKey)
-	summary := al.sessions.GetSummary(opts.SessionKey)
+	// 2. Build messages (skip history for heartbeat)
+	var history []providers.Message
+	var summary string
+	if !opts.NoHistory {
+		history = al.sessions.GetHistory(opts.SessionKey)
+		summary = al.sessions.GetSummary(opts.SessionKey)
+	}
 	messages := al.contextBuilder.BuildMessages(
 		history,
 		summary,
@@ -454,16 +506,14 @@ func (al *AgentLoop) runLLMIteration(ctx context.Context, messages []providers.M
 				})

 			// Create async callback for tools that implement AsyncTool
-			// This callback sends async completion results to the user
+			// NOTE: Following openclaw's design, async tools do NOT send results directly to users.
+			// Instead, they notify the agent via PublishInbound, and the agent decides
+			// whether to forward the result to the user (in processSystemMessage).
 			asyncCallback := func(callbackCtx context.Context, result *tools.ToolResult) {
-				// Send ForUser content to user if not silent
+				// Log the async completion but don't send directly to user
+				// The agent will handle user notification via processSystemMessage
 				if !result.Silent && result.ForUser != "" {
-					al.bus.PublishOutbound(bus.OutboundMessage{
-						Channel: opts.Channel,
-						ChatID:  opts.ChatID,
-						Content: result.ForUser,
-					})
-					logger.InfoCF("agent", "Async tool result sent to user",
+					logger.InfoCF("agent", "Async tool completed, agent will handle notification",
 						map[string]interface{}{
 							"tool":        tc.Name,
 							"content_len": len(result.ForUser),
--- a/pkg/heartbeat/service.go
+++ b/pkg/heartbeat/service.go
@@ -173,6 +173,9 @@ func (hs *HeartbeatService) executeHeartbeat() {
 	lastChannel := hs.state.GetLastChannel()
 	channel, chatID := hs.parseLastChannel(lastChannel)

+	// Debug log for channel resolution
+	hs.logInfo("Resolved channel: %s, chatID: %s (from lastChannel: %s)", channel, chatID, lastChannel)
+
 	result := handler(prompt, channel, chatID)

 	if result == nil {
@@ -259,8 +262,12 @@ This file contains tasks for the heartbeat service to check periodically.

 ## Instructions

-If there's nothing that needs attention, respond with: HEARTBEAT_OK
-This ensures the heartbeat runs silently when everything is fine.
+- Execute ALL tasks listed below. Do NOT skip any task.
+- For simple tasks (e.g., report current time), respond directly.
+- For complex tasks that may take time, use the spawn tool to create a subagent.
+- The spawn tool is async - subagent results will be sent to the user automatically.
+- After spawning a subagent, CONTINUE to process remaining tasks.
+- Only respond with HEARTBEAT_OK when ALL tasks are done AND nothing needs attention.

 ---

--- a/pkg/tools/subagent.go
+++ b/pkg/tools/subagent.go
@@ -28,6 +28,8 @@ type SubagentManager struct {
 	defaultModel string
 	bus          *bus.MessageBus
 	workspace    string
+	tools        *ToolRegistry
+	maxIterations int
 	nextID       int
 }

@@ -38,10 +40,27 @@ func NewSubagentManager(provider providers.LLMProvider, defaultModel, workspace
 		defaultModel: defaultModel,
 		bus:          bus,
 		workspace:    workspace,
+		tools:        NewToolRegistry(),
+		maxIterations: 10,
 		nextID:       1,
 	}
 }

+// SetTools sets the tool registry for subagent execution.
+// If not set, subagent will have access to the provided tools.
+func (sm *SubagentManager) SetTools(tools *ToolRegistry) {
+	sm.mu.Lock()
+	defer sm.mu.Unlock()
+	sm.tools = tools
+}
+
+// RegisterTool registers a tool for subagent execution.
+func (sm *SubagentManager) RegisterTool(tool Tool) {
+	sm.mu.Lock()
+	defer sm.mu.Unlock()
+	sm.tools.Register(tool)
+}
+
 func (sm *SubagentManager) Spawn(ctx context.Context, task, label, originChannel, originChatID string, callback AsyncCallback) (string, error) {
 	sm.mu.Lock()
 	defer sm.mu.Unlock()
@@ -73,10 +92,15 @@ func (sm *SubagentManager) runTask(ctx context.Context, task *SubagentTask, call
 	task.Status = "running"
 	task.Created = time.Now().UnixMilli()

+	// Build system prompt for subagent
+	systemPrompt := `You are a subagent. Complete the given task independently and report the result.
+You have access to tools - use them as needed to complete your task.
+After completing the task, provide a clear summary of what was done.`
+
 	messages := []providers.Message{
 		{
 			Role:    "system",
-			Content: "You are a subagent. Complete the given task independently and report the result.",
+			Content: systemPrompt,
 		},
 		{
 			Role:    "user",
@@ -95,9 +119,22 @@ func (sm *SubagentManager) runTask(ctx context.Context, task *SubagentTask, call
 	default:
 	}

-	response, err := sm.provider.Chat(ctx, messages, nil, sm.defaultModel, map[string]interface{}{
-		"max_tokens": 4096,
-	})
+	// Run tool loop with access to tools
+	sm.mu.RLock()
+	tools := sm.tools
+	maxIter := sm.maxIterations
+	sm.mu.RUnlock()
+
+	loopResult, err := RunToolLoop(ctx, ToolLoopConfig{
+		Provider:      sm.provider,
+		Model:         sm.defaultModel,
+		Tools:         tools,
+		MaxIterations: maxIter,
+		LLMOptions: map[string]any{
+			"max_tokens":  4096,
+			"temperature": 0.7,
+		},
+	}, messages, task.OriginChannel, task.OriginChatID)

 	sm.mu.Lock()
 	var result *ToolResult
@@ -127,10 +164,10 @@ func (sm *SubagentManager) runTask(ctx context.Context, task *SubagentTask, call
 		}
 	} else {
 		task.Status = "completed"
-		task.Result = response.Content
+		task.Result = loopResult.Content
 		result = &ToolResult{
-			ForLLM:  fmt.Sprintf("Subagent '%s' completed: %s", task.Label, response.Content),
-			ForUser: response.Content,
+			ForLLM:  fmt.Sprintf("Subagent '%s' completed (iterations: %d): %s", task.Label, loopResult.Iterations, loopResult.Content),
+			ForUser: loopResult.Content,
 			Silent:  false,
 			IsError: false,
 			Async:   false,
--- a/pkg/tools/toolloop.go
+++ b/pkg/tools/toolloop.go
@@ -0,0 +1,165 @@
+// PicoClaw - Ultra-lightweight personal AI agent
+// Inspired by and based on nanobot: https://github.com/HKUDS/nanobot
+// License: MIT
+//
+// Copyright (c) 2026 PicoClaw contributors
+
+package tools
+
+import (
+	"context"
+	"encoding/json"
+	"fmt"
+
+	"github.com/sipeed/picoclaw/pkg/logger"
+	"github.com/sipeed/picoclaw/pkg/providers"
+	"github.com/sipeed/picoclaw/pkg/utils"
+)
+
+// ToolLoopConfig configures the tool execution loop.
+type ToolLoopConfig struct {
+	Provider      providers.LLMProvider
+	Model         string
+	Tools         *ToolRegistry
+	MaxIterations int
+	LLMOptions    map[string]any
+}
+
+// ToolLoopResult contains the result of running the tool loop.
+type ToolLoopResult struct {
+	Content    string
+	Iterations int
+}
+
+// RunToolLoop executes the LLM + tool call iteration loop.
+// This is the core agent logic that can be reused by both main agent and subagents.
+func RunToolLoop(ctx context.Context, config ToolLoopConfig, messages []providers.Message, channel, chatID string) (*ToolLoopResult, error) {
+	iteration := 0
+	var finalContent string
+
+	for iteration < config.MaxIterations {
+		iteration++
+
+		logger.DebugCF("toolloop", "LLM iteration",
+			map[string]any{
+				"iteration": iteration,
+				"max":       config.MaxIterations,
+			})
+
+		// 1. Build tool definitions
+		var providerToolDefs []providers.ToolDefinition
+		if config.Tools != nil {
+			toolDefs := config.Tools.GetDefinitions()
+			providerToolDefs = make([]providers.ToolDefinition, 0, len(toolDefs))
+			for _, td := range toolDefs {
+				providerToolDefs = append(providerToolDefs, providers.ToolDefinition{
+					Type: td["type"].(string),
+					Function: providers.ToolFunctionDefinition{
+						Name:        td["function"].(map[string]any)["name"].(string),
+						Description: td["function"].(map[string]any)["description"].(string),
+						Parameters:  td["function"].(map[string]any)["parameters"].(map[string]any),
+					},
+				})
+			}
+		}
+
+		// 2. Set default LLM options
+		llmOpts := config.LLMOptions
+		if llmOpts == nil {
+			llmOpts = map[string]any{
+				"max_tokens":  4096,
+				"temperature": 0.7,
+			}
+		}
+
+		// 3. Call LLM
+		response, err := config.Provider.Chat(ctx, messages, providerToolDefs, config.Model, llmOpts)
+		if err != nil {
+			logger.ErrorCF("toolloop", "LLM call failed",
+				map[string]any{
+					"iteration": iteration,
+					"error":     err.Error(),
+				})
+			return nil, fmt.Errorf("LLM call failed: %w", err)
+		}
+
+		// 4. If no tool calls, we're done
+		if len(response.ToolCalls) == 0 {
+			finalContent = response.Content
+			logger.InfoCF("toolloop", "LLM response without tool calls (direct answer)",
+				map[string]any{
+					"iteration":     iteration,
+					"content_chars": len(finalContent),
+				})
+			break
+		}
+
+		// 5. Log tool calls
+		toolNames := make([]string, 0, len(response.ToolCalls))
+		for _, tc := range response.ToolCalls {
+			toolNames = append(toolNames, tc.Name)
+		}
+		logger.InfoCF("toolloop", "LLM requested tool calls",
+			map[string]any{
+				"tools":     toolNames,
+				"count":     len(response.ToolCalls),
+				"iteration": iteration,
+			})
+
+		// 6. Build assistant message with tool calls
+		assistantMsg := providers.Message{
+			Role:    "assistant",
+			Content: response.Content,
+		}
+		for _, tc := range response.ToolCalls {
+			argumentsJSON, _ := json.Marshal(tc.Arguments)
+			assistantMsg.ToolCalls = append(assistantMsg.ToolCalls, providers.ToolCall{
+				ID:   tc.ID,
+				Type: "function",
+				Function: &providers.FunctionCall{
+					Name:      tc.Name,
+					Arguments: string(argumentsJSON),
+				},
+			})
+		}
+		messages = append(messages, assistantMsg)
+
+		// 7. Execute tool calls
+		for _, tc := range response.ToolCalls {
+			argsJSON, _ := json.Marshal(tc.Arguments)
+			argsPreview := utils.Truncate(string(argsJSON), 200)
+			logger.InfoCF("toolloop", fmt.Sprintf("Tool call: %s(%s)", tc.Name, argsPreview),
+				map[string]any{
+					"tool":      tc.Name,
+					"iteration": iteration,
+				})
+
+			// Execute tool (no async callback for subagents - they run independently)
+			var toolResult *ToolResult
+			if config.Tools != nil {
+				toolResult = config.Tools.ExecuteWithContext(ctx, tc.Name, tc.Arguments, channel, chatID, nil)
+			} else {
+				toolResult = ErrorResult("No tools available")
+			}
+
+			// Determine content for LLM
+			contentForLLM := toolResult.ForLLM
+			if contentForLLM == "" && toolResult.Err != nil {
+				contentForLLM = toolResult.Err.Error()
+			}
+
+			// Add tool result message
+			toolResultMsg := providers.Message{
+				Role:       "tool",
+				Content:    contentForLLM,
+				ToolCallID: tc.ID,
+			}
+			messages = append(messages, toolResultMsg)
+		}
+	}
+
+	return &ToolLoopResult{
+		Content:    finalContent,
+		Iterations: iteration,
+	}, nil
+}